Opened 2 months ago

Closed 2 weeks ago

#22542 closed defect (fixed)

Tor Browser 7.0 Security Settings Window too small on macOS 10.12

Reported by: teor Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-7.0-issues, tbb-regression, tbb-security-slider, TorBrowserTeam201707R
Cc: arthuredelstein, mcs, brade Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

When I run Tor Browser 7.0 on macOS 10.12, and open the security settings window, the last line of text is off the page. This is probably because the UI font changed.

Child Tickets

Attachments (2)

SecurityBefore.png (35.4 KB) - added by teor 3 weeks ago.
SecurityAfter.png (44.5 KB) - added by teor 3 weeks ago.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 2 months ago by gk

Cc: arthuredelstein added
Keywords: tbb-7.0-issues tbb-regression tbb-security-slider added
Status: newneeds_information

So, you don't get a scrollbar and the text is off the page? Or is the issue that you are already getting a scrollbar on the level with the least amount of text?

comment:2 in reply to:  1 ; Changed 2 months ago by teor

Status: needs_informationnew

Replying to gk:

So, you don't get a scrollbar and the text is off the page? Or is the issue that you are already getting a scrollbar on the level with the least amount of text?

What I see:

There is no visual indication that there is any extra text in the window that can be scrolled to.

There is a scrollbar, but on macOS, it is invisible until the user starts scrolling.
If a user just happens to start scrolling, then the final line becomes visible, but the first line is hidden.

The scrolling also makes it hard to tell exactly which lines apply to Low, Medium, and High.

What I expect:

All the text is shown on the screen when the user opens the window.

comment:3 in reply to:  2 Changed 2 months ago by teor

Replying to teor:

Replying to gk:

So, you don't get a scrollbar and the text is off the page? Or is the issue that you are already getting a scrollbar on the level with the least amount of text?

This only happens when I am in High Security Mode.
The scrollbar appears for a few seconds when I first switch to High Security Mode, then disappears. (I didn't see the scrollbar to start with, because I was in High Security Mode before I upgraded.)

What I see:

There is no visual indication that there is any extra text in the window that can be scrolled to.

There is a scrollbar, but on macOS, it is invisible until the user starts scrolling.
If a user just happens to start scrolling, then the final line becomes visible, but the first line is hidden.

The scrolling also makes it hard to tell exactly which lines apply to Low, Medium, and High.

What I expect:

All the text is shown on the screen when the user opens the window.

comment:4 Changed 2 months ago by mcs

Keywords: TorBrowserTeam201706R added
Status: newneeds_review

Here is a simple fix (increase a hard-coded height):
https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug22542-01
I assume this won't look too bad on the other platforms.

comment:5 Changed 2 months ago by cypherpunks

UI font changed

Spacing between lines changed.

comment:6 Changed 2 months ago by gk

Resolution: fixed
Status: needs_reviewclosed

Looks good to me. Merged to master (commit 6febc86aaca26984131b541e185323070de88ea7).

comment:7 Changed 7 weeks ago by teor

Resolution: fixed
Status: closedreopened

This works for me using macOS 10.12.5 outside a VM (retina), but doesn't work inside a VM (non-retina).

I am running macOS 10.12.5 with Tor Browser 7.0.1 and Tor Button 1.9.7.4 in both environments.

I think it's better than it used to be, but there are still 2 lines of text missing at the bottom on non-retina. (And on retina, there are a few pixels still hidden at the bottom, which makes it possible to trigger the scroll bar.)

comment:8 Changed 6 weeks ago by gk

Keywords: TorBrowserTeam201707 added; TorBrowserTeam201706R removed

comment:9 Changed 6 weeks ago by gk

Cc: mcs brade added

comment:10 Changed 5 weeks ago by mcs

Keywords: TorBrowserTeam201707R added; TorBrowserTeam201707 removed
Status: reopenedneeds_review

What we really want to do is adjust the height of the window so that no scrollbar is necessary. Here is a patch:
https://oniongit.eu/brade/torbutton/merge_requests/1

We tested this on a couple of OSX systems, on a Linux system, and on Windows 10. Here is a pre-built xpi for testing:
https://people.torproject.org/~brade/builds/bug22542/torbutton@torproject.org.xpi
teor: if you have time please test this on your OSX systems.

As an aside, Kathy and I don't know what the best approach is for using oniongit; maybe we should create a torbutton project under the tor-browser group and submit merge requests against it?

comment:11 in reply to:  10 Changed 5 weeks ago by teor

Replying to mcs:

...
We tested this on a couple of OSX systems, on a Linux system, and on Windows 10. Here is a pre-built xpi for testing:
https://people.torproject.org/~brade/builds/bug22542/torbutton@torproject.org.xpi
teor: if you have time please test this on your OSX systems.

I'm sorry, I am not sure I will have time to do this, I have some tight deadlines at the moment.

comment:12 Changed 3 weeks ago by teor

Status: needs_reviewmerge_ready

Thanks, this works on macOS 10.12.5/6 in both my VM (non-retina) and standard (retina) environments. I'll attach before and after screenshots for the VM case.

Changed 3 weeks ago by teor

Attachment: SecurityBefore.png added

Changed 3 weeks ago by teor

Attachment: SecurityAfter.png added

comment:13 in reply to:  10 Changed 3 weeks ago by gk

Replying to mcs:

What we really want to do is adjust the height of the window so that no scrollbar is necessary. Here is a patch:
https://oniongit.eu/brade/torbutton/merge_requests/1

Okay, I commented.

We tested this on a couple of OSX systems, on a Linux system, and on Windows 10. Here is a pre-built xpi for testing:
https://people.torproject.org/~brade/builds/bug22542/torbutton@torproject.org.xpi
teor: if you have time please test this on your OSX systems.

As an aside, Kathy and I don't know what the best approach is for using oniongit; maybe we should create a torbutton project under the tor-browser group and submit merge requests against it?

Dunno. I think for now just having a user repo there with an up-to-date master branch for merge requests and using the code review features Gitlab provides is the way to go. At least that's what I am currently doing. If there is something else/better I should set up and test I am all ears.

comment:14 Changed 3 weeks ago by brade

I replied to the comment in oniongit.

comment:15 in reply to:  14 ; Changed 3 weeks ago by cypherpunks

Status: merge_readyneeds_review

Replying to gk:

Okay, I commented.

Replying to brade:

I replied to the comment in oniongit.

Of course, it's good to know that you commented. But don't you think that openness and clearness of the project suffer? Especially, because oniongit without JS does nothing, except loading CPU. What was the reason to move the discussion to oniongit?

comment:16 in reply to:  15 Changed 3 weeks ago by gk

Replying to cypherpunks:

Replying to gk:

Okay, I commented.

Replying to brade:

I replied to the comment in oniongit.

Of course, it's good to know that you commented. But don't you think that openness and clearness of the project suffer? Especially, because oniongit without JS does nothing, except loading CPU. What was the reason to move the discussion to oniongit?

The reason is our testing of oniongit as a trac replacement (at least regarding code review). Thanks for raising the JS issue. teor went already ahead and pointed it out to a wider audience (https://lists.torproject.org/pipermail/tor-project/2017-July/001328.html). We'll see where the discussion goes.

comment:17 Changed 2 weeks ago by gk

Resolution: fixed
Status: needs_reviewclosed

Looks good to me. Let's test it in an alpha, though, first. I pushed the code to master (commit 8fbc96cab7ed9816da68d070b7cb01548a19a25a).

Note: See TracTickets for help on using tickets.