Opened 4 years ago

Closed 3 years ago

#17904 closed defect (fixed)

Use sufficient window dimensions in Privacy and Security Settings

Reported by: cypherpunks Owned by: tbb-team
Priority: Low Milestone:
Component: Applications/Tor Browser Version:
Severity: Minor Keywords: tbb-security-slider, TorBrowserTeam201609R
Cc: mcs, gk, arthuredelstein Actual Points:
Parent ID: Points:
Reviewer: Sponsor: SponsorU

Description

The Privacy and Security Settings window dimensions change when the Security Level slider is moved to a higher level to accommodate for the additional information on the right.

The changing of window dimensions distracts from the additional information that is changed because two things happen at the same time.

A solution would be to make the window have sufficient dimensions so it does not need to resize.

Child Tickets

Change History (14)

comment:1 Changed 4 years ago by teor

I see this happen in OS X when I change from medium-high to high: the window becomes slightly longer, and therefore the slider and window content move a little.

comment:2 Changed 4 years ago by mcs

Cc: mcs added

comment:3 Changed 4 years ago by gk

Cc: gk added
Keywords: tbb-security-slider added

comment:4 Changed 4 years ago by cypherpunks

Why not to allow the user to scale the window as he wants but conserving aspect ratio and internally transform the values visible to webpage (a multiplication and a rounding) in the way it should be indistinguishable from unscaled window.

comment:5 Changed 3 years ago by gk

Keywords: TorBrowserTeam201609 added
Sponsor: SponsorU

Getting this on our radar for September.

comment:6 Changed 3 years ago by arthuredelstein

Cc: arthuredelstein added

comment:7 Changed 3 years ago by arthuredelstein

See also #16211.

comment:8 Changed 3 years ago by arthuredelstein

Keywords: TorBrowserTeam201609R added; TorBrowserTeam201609 removed
Status: newneeds_review

Here's a patch that adds a scrollbar when the descriptions are too long for the window size. Resizing the window will cause the scrollbar to disappear.

https://github.com/arthuredelstein/torbutton/commit/17904

comment:9 Changed 3 years ago by mcs

Kathy and I reviewed this and it looks good. We have just a couple of comments:

  1. You added ids to many XUL elements, but we do not see where they are used. Are they needed? For example, you added id="privacy_checkboxes" Are these IDs needed?
  2. The preferences.xul file has no tabs except for one (which should probably be removed). Please use spaces instead of tabs to stay consistent with the rest of the file.

comment:10 in reply to:  9 ; Changed 3 years ago by arthuredelstein

Status: needs_reviewneeds_information

Replying to mcs:

Kathy and I reviewed this and it looks good. We have just a couple of comments:

Thanks for reviewing this!

  1. You added ids to many XUL elements, but we do not see where they are used. Are they needed? For example, you added id="privacy_checkboxes" Are these IDs needed?

I actually added them because I was having trouble keeping track of nested XUL elements. Would you prefer if I turn them into comments?

  1. The preferences.xul file has no tabs except for one (which should probably be removed). Please use spaces instead of tabs to stay consistent with the rest of the file.

Not sure how I ended up with tabs -- I will fix that.

comment:11 in reply to:  10 ; Changed 3 years ago by mcs

Replying to arthuredelstein:

I actually added them because I was having trouble keeping track of nested XUL elements. Would you prefer if I turn them into comments?

Yes, only because when I see an id I think "some code or CSS must reference this element by ID." But if you really like having them I can modify my thinking.

comment:12 in reply to:  11 ; Changed 3 years ago by arthuredelstein

Status: needs_informationneeds_review

Replying to mcs:

Replying to arthuredelstein:

I actually added them because I was having trouble keeping track of nested XUL elements. Would you prefer if I turn them into comments?

Yes, only because when I see an id I think "some code or CSS must reference this element by ID."

That makes sense. I've dropped them and also got rid of the tabs:

https://github.com/arthuredelstein/torbutton/commit/17904+1

comment:13 in reply to:  12 Changed 3 years ago by mcs

Replying to arthuredelstein:

That makes sense. I've dropped them and also got rid of the tabs:

https://github.com/arthuredelstein/torbutton/commit/17904+1

r=brade, r=mcs
This looks good to us.

comment:14 Changed 3 years ago by gk

Resolution: fixed
Status: needs_reviewclosed

Applied to master (commit b1adcba17a96a925345001d4da69cb71b469e3ed), thanks.

Note: See TracTickets for help on using tickets.