Opened 3 years ago

Last modified 10 months ago

#14429 needs_revision defect

Automated rounding of content window dimensions

Reported by: arthuredelstein Owned by: arthuredelstein
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-fingerprinting-resolution, tbb-torbutton
Cc: gacar, brade, mcs, ln5 Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

I've written a small patch for torbutton that forces the content ("gBrowser") to have dimensions be a multiple of 200x200. In other words, window.innerWidth and window.innerHeight, and similar calls, always return a rounded number.

This should at least provide some protection to users who resize or maximize their Tor Browser window with JS activated.

I haven't dealt with the zooming issue here, but that would be an interesting next step.

Child Tickets

TicketStatusOwnerSummaryComponent
#15537newtbb-teamTor Browser 4.5a5 fails to maximizeApplications/Tor Browser
#15602assignedtbb-teamTorButton should contain an opt-out option for the enforced max screen size in TB4.5+Applications/Tor Browser

Attachments (12)

maximized_windows7.png (173.1 KB) - added by gk 3 years ago.
almost-not-quite.diff (1.4 KB) - added by mikeperry 3 years ago.
Attempted fix for resizing issue. Uses incorrect element for geometry.
ubuntuResizeMarginsBefore.png (128.0 KB) - added by arthuredelstein 3 years ago.
ubuntuResizeMarginsAfter.png (154.2 KB) - added by arthuredelstein 3 years ago.
rounding_win8.png (184.0 KB) - added by gk 3 years ago.
DPI_issue1.png (215.2 KB) - added by gk 3 years ago.
torbutton-1.9.1.0-4715ab2330-screenshot0.png (93.6 KB) - added by bernie.allen 3 years ago.
win7-1028x768-torbutton-1.9.1.0-4715ab2330-initial.png (95.0 KB) - added by bernie.allen 3 years ago.
win7-1028x768-torbutton-1.9.1.0-4715ab2330-maximized.png (67.5 KB) - added by bernie.allen 3 years ago.
debian7-xfce-1024x600-torbutton-1.9.1.0-ce23ee30ab_maximized_zoom_too_much.png (85.5 KB) - added by bernie.allen 3 years ago.
window7_fullscreen_150DPI.PNG (146.0 KB) - added by gk 2 years ago.
view_tickets_trac_zoom.png (3.3 KB) - added by gk 18 months ago.

Download all attachments as: .zip

Change History (148)

comment:1 Changed 3 years ago by arthuredelstein

Status: newneeds_review

comment:2 Changed 3 years ago by arthuredelstein

Keywords: TorBrowserTeam201501R added

comment:3 Changed 3 years ago by gk

Keywords: tbb-torbutton added; TorBrowserTeam201501R removed
Status: needs_reviewneeds_revision

Nice idea. But I think it needs revision due to usability issues. See the attached screenshot for an example: This is a maximized window where essentially 1/3 of the browser window is grey and unusable despite the user wanting to have the maximum browser window possible on her screen. Similar things happen in fullscreen mode, e.g. I think denying the user to give her Tor Browser in fullscreen/maximized mode is just leading to frustration and switching to a different browser.

The challenge here has never been to enforce rounding of a window by a certain multiple of, say 100. The challenge is to achieve that AND don't change how the browser looks like or behaves in important ways. I think a viable improvement would at least be if we would prompt a user *before* the window is actually maximized/in fullscreen mode (#7255) which we'd need anyway even if we don't take this patch but just make the user aware that she is about to shoot herself in the foot. And, yes, zooming (#7256) is an interesting approach which might get us around all of the rounding issue...

Changed 3 years ago by gk

Attachment: maximized_windows7.png added

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

Replying to gk:

Nice idea. But I think it needs revision due to usability issues. See the attached screenshot for an example: This is a maximized window where essentially 1/3 of the browser window is grey and unusable despite the user wanting to have the maximum browser window possible on her screen.

One possibility would be to take common set of maximized window dimensions and include these in a whitelist. Another thing we could add is that the window shrinks to the allowed dimensions after resizing finishes, to avoid wasting screen space with the dark gray background.

Similar things happen in fullscreen mode, e.g. I think denying the user to give her Tor Browser in fullscreen/maximized mode is just leading to frustration and switching to a different browser.

I don't think usability should trump an easy path for de-anonymization.

The challenge here has never been to enforce rounding of a window by a certain multiple of, say 100. The challenge is to achieve that AND don't change how the browser looks like or behaves in important ways. I think a viable improvement would at least be if we would prompt a user *before* the window is actually maximized/in fullscreen mode (#7255) which we'd need anyway even if we don't take this patch but just make the user aware that she is about to shoot herself in the foot.

I don't understand this approach. Why would we want to permit users to shoot themselves in the foot, even after a warning? Some users won't understand the warning and will get deanonymized. I think in practice, we should be protecting users from this easy and fatal mistake.

Of course, it's not just maximizing or fullscreen that is dangerous -- any resizing is dangerous.

And, yes, zooming (#7256) is an interesting approach which might get us around all of the rounding issue...

I agree that zooming could help cosmetically, but if the user resizes the window, trying to see more of a page, and the page zooms instead, that could be frustrating, too.

comment:5 in reply to:  3 Changed 3 years ago by arthuredelstein

Replying to gk:

Thanks very much for testing, by the way! I do agree the large gray areas are going to be a turn-off to users, so some improvements to this patch could be:

  1. Auto-trimming the window to hide gray areas after a user-requested resize.
  2. Decreasing the step-size from 200x200 to 100x100 or 50x50. This would buy some usability at the expense of 2 bits or 4 bits of entropy.
  3. Providing a whitelist of typical window sizes, such as maximized sizes on various screens.
  4. Zooming the window as the user adjusts the window size so that a gray bar is only present along one dimension or another ("letterbox" or "pillarbox").

Does this seem like a useful avenue to pursue?

(Of course this sort of auto-resizing patch could also be used in conjunction with a warning to users when they first attempt to resize a window.)

comment:6 Changed 3 years ago by arthuredelstein

Here's a new version of the patch that auto-trims the window after the user resizes it. That way we don't have ugly gray bars taking up a lot of screen space.

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

comment:7 Changed 3 years ago by mikeperry

Keywords: tbb-fingerprinting-resolution added; tbb-fingerprinting removed

comment:8 in reply to:  6 Changed 3 years ago by guywith2c

Replying to arthuredelstein:

Here's a new version of the patch that auto-trims the window after the user resizes it. That way we don't have ugly gray bars taking up a lot of screen space.

Seeing that screen-size is the SINGLE BIGGEST identifier (containing more bits than all the other identifying tokens combined together), I take here liberty to digress:

If I may, I vote about comment 5's improvements thusly:

  • against (1) - maximized window should stay maximized and keep the scrollbar on the right edge, if possible, AND the gray areas could/should! be used as interactive privacy area of "warning":
    • idea: when a user hovers over this "ugly grey area", she is informed :

"
In this tab you have Javascript enabled, thus making you be 2k [k == whatever panopticlick.eff.org suggests] times more easily identifiable (attacker will only have to find your browser in K [K == what panopticlick says] possible ones).

-- To increase privacy, disable javascript for this tab by clicking _here_, and your inner window will get maximized to size of outer maximized window to get rid of this "large gray(or warning-red) area".

-- To keep at current privacy level for this tab, keep this ?annoying? large gray area.

-- To decrease privacy for this tab, click _here_ and your inner window will be maximized, thus possibly identifying your browser as uniqe and so creating a very strong evidence of this browser's visit of this web page at this time and the following linked websites may be also aware of this evidence: google-ananlytics.com , google.com, adserve.net, ...

"

  • {200,100}x{200,100} or even {200,100,50}x{200,100,50} in (2). Going to 50 pixels might remove too much privacy - maybe in a year or two when more people use it, this could be stepped down to 50 pixels
  • for (3) - make this list accessible by the hover describe in my first vote above.
  • for (4) - ditto.

In other words, PLEASE do use these "large gray areas" to make people aware of the (HUGE!!!) implications of screen size uniqueness => de-privatness (whoa, I got a new word here ! :) ) And use it to let them / make them choose.

comment:9 in reply to:  4 Changed 3 years ago by gk

Replying to arthuredelstein:

Replying to gk:

The challenge here has never been to enforce rounding of a window by a certain multiple of, say 100. The challenge is to achieve that AND don't change how the browser looks like or behaves in important ways. I think a viable improvement would at least be if we would prompt a user *before* the window is actually maximized/in fullscreen mode (#7255) which we'd need anyway even if we don't take this patch but just make the user aware that she is about to shoot herself in the foot.

I don't understand this approach. Why would we want to permit users to shoot themselves in the foot, even after a warning? Some users won't understand the warning and will get deanonymized. I think in practice, we should be protecting users from this easy and fatal mistake.

Of course, it's not just maximizing or fullscreen that is dangerous -- any resizing is dangerous.

There are at least two things to consider IMO: 1) There are use cases where resizing/maximizing a window or making a window fullscreen is no shooting oneself in the foot at all. 2) If we make the usability burden too high, users are going away from Tor Browser and reducing the anonymity group for all other users as well.

Wrt to 2) remember we even allow the user to activate Flash in Tor Browser and allowing that may have much worse consequences than the screen resizing problem. But we do that for a reason while all the other plugins are not allowed to run at all. I think we should at least allow a user to opt out of the automatic rounding of browser windows, too. And not allowing 1) is even more problematic, I think (and I could come up ad hoc with a bunch of scenarios where resizing is no problem at all). So as I said: We should at least do #7255 + the fix here, trying to impact usability as little as possible even if we have the prompt in order to not make users upset to a point where they choose a different browser. The plan Mike had on IRC is a good one:

22:44 < mikeperry> so I think the best plan is to combine #14429 and #7255 together, 
                   so that we do the trim/forced window size by default, but then 
                   give users a "Don't do this in the future" confirm dialog or 
                   doorhanger?
22:44 < mikeperry> so they could opt out and actually resize their window freely if 
                   they want

We could even be smarter showing a (different?) dialog (additionally) warning a user who is resizing the browser window although it is already a rounded one (if the user did not opt out yet).

(Personally, and this is 3), users should have the right to shoot themselves in the foot if they want to but that is orthogonal to the much more important 1) and 2)).

And, yes, zooming (#7256) is an interesting approach which might get us around all of the rounding issue...

I agree that zooming could help cosmetically, but if the user resizes the window, trying to see more of a page, and the page zooms instead, that could be frustrating, too.

I did not say it is easy... ;)

That's the global view I have in mind. Hopefully, it makes at least a bit sense. :) I have to think about your patch and some really good options a bit more...

Last edited 3 years ago by gk (previous) (diff)

comment:10 Changed 3 years ago by mikeperry

Keywords: TorBrowserTeam201502 added

comment:11 Changed 3 years ago by mikeperry

In the interest of progress, I am also in favor of landing the patch for this resizing code before we complete the prompt/confirmation dialog in #7255, especially if that improves our chances of testing out the resizing code among a wider audience in 4.5a4.

Simply putting this code behind a pref seems good for now (especially if we can get this code to automatically solve weird things like #14098).

comment:12 in reply to:  6 Changed 3 years ago by gk

Cc: gacar added

Replying to arthuredelstein:

Here's a new version of the patch that auto-trims the window after the user resizes it. That way we don't have ugly gray bars taking up a lot of screen space.

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

Looks much better, thanks! Two comments:

1) I think we should keep the 200x100 rounding as we already do on start-up for now. Vertical space is precious (assuming we have a user with a horizontal display as we are currently doing). However, having some more data driven clues would be really nice. See: https://trac.torproject.org/13025#comment:17 for a beginning. As a side note: there has already been a wrestling in the past with rounding windows to 50px x 50 px: https://www.torproject.org/docs/torbutton/en/design/ (section Resize windows to multiples of 50px during Tor usage (recommended)) + see #3506...

2) Your patch re-rounded the already rounded window on start-up which is quite confusing: the properly rounded window shows up and then your logic kicks in making the window smaller. I think the resizing logic works on start-up in the most cases. Thus, what about only resizing again with your logic if there is no multiple of 200x100 after start-up? This should be easily doable and does not scare the user so much ("What the heck is going on with my browser windows here???") provided she already has rounded dimensions.

comment:13 Changed 3 years ago by arthuredelstein

Here's a new version with 200x100 quantization. I believe I have now ensured that resizing doesn't occur at startup unless the window dimensions are not correctly rounded. Rounding is dynamically bound to the (reintroduced) extensions.torbutton.resize_windows pref, which is on by default:

https://github.com/arthuredelstein/torbutton/commit/276b44bd28df432cdaa968156118d169c3a46484

comment:14 Changed 3 years ago by arthuredelstein

Status: needs_revisionneeds_review

comment:15 Changed 3 years ago by arthuredelstein

Keywords: TorBrowserTeam201502R added; TorBrowserTeam201502 removed

comment:16 Changed 3 years ago by mikeperry

What made you pick 1000ms? If we set this to 100ms or even 10ms, will there be issues on some systems? I will play with some timeouts on my systems and see if I notice anything, but I was curious if you had a reason for picking such a large delay.

I think there may also be a race/timer resolution issue with the ping() function. In one instance, my window was not resized. I'm not sure, but I suspect this may be because the if (now - lastPingTime >= timeout) check failed due to the timer actually firing slightly early.

Also as a general matter, I'm not sure we actually need the grey border and the content window quantization for drag-based resizing, but only for maximization or other window manager enforced sizing. In other words, it seems fine to reveal the intermediate sizes to the content window if the user is simply dragging, but it is not fine to reveal the intermediate size if the window manager was maximizing or tiling the window. I guess it may not be possible to tell the difference in practice, though?

My statements about trying to not display the grey border during drag hinge on how bad we think it is to keep reflowing as if we would allow the resize, only to snap back to a size the user wasn't expecting. Maybe I'm wrong, and it is actually better to give the user a hint with the grey background..

comment:17 in reply to:  16 Changed 3 years ago by arthuredelstein

Replying to mikeperry:

What made you pick 1000ms? If we set this to 100ms or even 10ms, will there be issues on some systems? I will play with some timeouts on my systems and see if I notice anything, but I was curious if you had a reason for picking such a large delay.

That's a good point. While the user is resizing the window (by dragging an edge or corner), the chrome window object fires many "resize" events. I couldn't find any nice way to detect if the user has stopped dragging the window (mousedown/up events don't fire), so I decided just to wait until we feel it is safe to assume that the user has finished dragging before calling shrinkwrap(). I'm not sure what this time is, but 250 ms seemed too short, at least with myself as the test subject. I think 500 ms seems OK. (If the user does pause in a drag, then the shrinkwrap occurs anyway, even though the mouse is still in "dragging" mode.) Let me know what you think.

I just measured the typical time interval between "resize" events and the interval is almost always 50 ms or less, except if I leave the mouse down and stop moving for a while. (This may be unusually easy for me because I am testing with a touchpad where a double-tap leaves the mouse down.)

I think there may also be a race/timer resolution issue with the ping() function. In one instance, my window was not resized. I'm not sure, but I suspect this may be because the if (now - lastPingTime >= timeout) check failed due to the timer actually firing slightly early.

Weird. Seems like the timer firing early should be considered a bug. In any case, I've written a new version of that patch with a simpler and hopefully more robust pinger function and I've reduced the timeout to 500 ms.

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

Also as a general matter, I'm not sure we actually need the grey border and the content window quantization for drag-based resizing, but only for maximization or other window manager enforced sizing. In other words, it seems fine to reveal the intermediate sizes to the content window if the user is simply dragging, but it is not fine to reveal the intermediate size if the window manager was maximizing or tiling the window. I guess it may not be possible to tell the difference in practice, though?

Yes, I couldn't find a way to tell immediately that the user has clicked the maximize button. In my observations, the "sizemodechange" event only occurs after maximization has completed, and it is preceded by a number of "resize" events. For window tiling, I'm not sure we would get any special events.

My statements about trying to not display the grey border during drag hinge on how bad we think it is to keep reflowing as if we would allow the resize, only to snap back to a size the user wasn't expecting. Maybe I'm wrong, and it is actually better to give the user a hint with the grey background..

I do like having some sort of hint what the final window size is going to look like. But I may be the only one...

Last edited 3 years ago by arthuredelstein (previous) (diff)

Changed 3 years ago by mikeperry

Attachment: almost-not-quite.diff added

Attempted fix for resizing issue. Uses incorrect element for geometry.

comment:18 Changed 3 years ago by mikeperry

Ok, I think you are right that we should leave the grey border in there as a hint to the user about what we're going to do to the window size.

I think also I understand why I was able to get the window to fail to resize now. I simply held on to the resize drag too long. I played around with trying to reschedule the ping() function if there was still discrepency in the gbrowser container vs content window box, but I wasn't sure which elements to use for sizing that.

If we can get this idea to work, perhaps we can set the timeout much quicker (ie 10ms). I attached a patch that attempts to do this (but leaves the timeout as-is, since it was easier to see the log messages arrive in the browser console). Unfortunately, I tried a few things and couldn't find the right element to use to check the actual size discrepancy... perhaps you have some ideas?

comment:19 in reply to:  18 ; Changed 3 years ago by arthuredelstein

Replying to mikeperry:

Ok, I think you are right that we should leave the grey border in there as a hint to the user about what we're going to do to the window size.

I think also I understand why I was able to get the window to fail to resize now. I simply held on to the resize drag too long.

I think maybe we are seeing different behaviors for some reason. In the latest version of my patch, after quite a bit of testing I haven't observed a resize failure (on OS X). Here's a recording of a few tests: https://www.youtube.com/watch?v=TQxuuFTgz7M

One relatively minor issue shown at the end of the movie is that, when I hold the mouse button down during a resize drag, but I stop moving the mouse, after a timeout I see shrinkwrap runs, meaning the window shrinks so that there is no grey margin around the gBrowser (web content) element. Then, if I move the mouse a little, holding the mouse button down, the window border jumps back to follow my mouse cursor again. It's an odd looking behavior, but I don't see how to avoid it without hacking on Firefox. In any case, whenever I finally release the mouse button, the timeout expires and shrinkwrap runs a final time.

Most of the time, I hope users will simply drag the window to approximately the size they want, and then release the mouse. Then the window would resize once, 500 ms later.

I played around with trying to reschedule the ping() function if there was still discrepency in the gbrowser container vs content window box, but I wasn't sure which elements to use for sizing that.

I'm not sure why you are getting a discrepancy -- on my machine, 500 ms after dragging ends, the grey margins invariably disappear.

If we can get this idea to work, perhaps we can set the timeout much quicker (ie 10ms). I attached a patch that attempts to do this (but leaves the timeout as-is, since it was easier to see the log messages arrive in the browser console).

I fear a timeout of 10 ms would be problematic, because then shrinkwrap would often be called between "resize" events (which are often 30 ms apart during a resize drag).

Unfortunately, I tried a few things and couldn't find the right element to use to check the actual size discrepancy... perhaps you have some ideas?

Maybe I'm not clear on the size discrepancy you're observing -- can you post a screenshot?

comment:20 in reply to:  19 Changed 3 years ago by mikeperry

Replying to arthuredelstein:

Replying to mikeperry:

Ok, I think you are right that we should leave the grey border in there as a hint to the user about what we're going to do to the window size.

I think also I understand why I was able to get the window to fail to resize now. I simply held on to the resize drag too long.

I think maybe we are seeing different behaviors for some reason. In the latest version of my patch, after quite a bit of testing I haven't observed a resize failure (on OS X). Here's a recording of a few tests: https://www.youtube.com/watch?v=TQxuuFTgz7M

One relatively minor issue shown at the end of the movie is that, when I hold the mouse button down during a resize drag, but I stop moving the mouse, after a timeout I see shrinkwrap runs, meaning the window shrinks so that there is no grey margin around the gBrowser (web content) element. Then, if I move the mouse a little, holding the mouse button down, the window border jumps back to follow my mouse cursor again. It's an odd looking behavior, but I don't see how to avoid it without hacking on Firefox. In any case, whenever I finally release the mouse button, the timeout expires and shrinkwrap runs a final time.

Most of the time, I hope users will simply drag the window to approximately the size they want, and then release the mouse. Then the window would resize once, 500 ms later.

I played around with trying to reschedule the ping() function if there was still discrepency in the gbrowser container vs content window box, but I wasn't sure which elements to use for sizing that.

I'm not sure why you are getting a discrepancy -- on my machine, 500 ms after dragging ends, the grey margins invariably disappear.

This is the difference. On Linux+Gnome desktop, if I hold the mouse button down and stop dragging like you did in the video, the grey margin does not disappear, nor does the outer window actually resize if I release the button without a further drag movement. The window remains stuck at the held size with the grey margins without a resize. This is why I added the return value after the check for the deltas in shrinkWrap(), and the subsequent invocation of ping() inside the timeout handler.

On my system, Firefox thinks that the gBrowser's parent element *was* resized, but it also thinks the window element's outerWidth and outerHeight were not resized. Unfortunately, the window outer sizes also contain the toolbar and the title bar heights, and so the resize keeps happening as if there were still a discrepency, no matter what. Hence my suggestion that we try to find an element that is not actually resized, or figure out some other way to infer this information (perhaps by computing the size of these elements?)

comment:21 Changed 3 years ago by arthuredelstein

Thanks -- I can now reproduce this on Ubuntu. Looking into it.

Changed 3 years ago by arthuredelstein

Changed 3 years ago by arthuredelstein

comment:22 Changed 3 years ago by arthuredelstein

Status: needs_reviewnew

Replying to mikeperry:

This is the difference. On Linux+Gnome desktop, if I hold the mouse button down and stop dragging like you did in the video, the grey margin does not disappear, nor does the outer window actually resize if I release the button without a further drag movement. The window remains stuck at the held size with the grey margins without a resize. This is why I added the return value after the check for the deltas in shrinkWrap(), and the subsequent invocation of ping() inside the timeout handler.

I see an even worse result on default Ubuntu. If I hold the mouse button down and stop dragging, after the timeout, the grey margin disappears:


The toolbar shrinks to the gBrowser element size, but the outer window itself does not shrink, and a white margin appears around both gBrowser and toolbar. Then when I release the mouse button, the window remains stuck.

All calls to window.outerWidth, etc, seem not to be aware of the window's white margin. So there isn't even a way to detect this problem. The only solution I see is to call window.resizeBy(0,0) periodically in the indefinite future, but that would be an ugly hack.

This is looking difficult to fix -- I wonder if it means we will have to patch Firefox. One possibility would be to add a "resizeend" event, the fires after the user stops dragging or the window has fully maximized. That would be a much cleaner option than these various timing-based hacks.

comment:23 Changed 3 years ago by arthuredelstein

Status: newneeds_review

Today I was able to modify the patch to behave better in Ubuntu. The white margin still appears if I hold onto the dragging handle for too long, but if I release the mouse button, the white margin now disappears. Here's a video of the new version: https://www.youtube.com/watch?v=5oV1VhRCa_U&feature=youtu.be

The patch is here:
https://github.com/arthuredelstein/torbutton/commit/56981b29518de7b943b870e80765e9b3ddc1f1d5

comment:24 Changed 3 years ago by mikeperry

Keywords: tbb-4.5-alpha added

comment:25 Changed 3 years ago by mikeperry

Keywords: TorBrowserTeam201503R added; TorBrowserTeam201502R removed

comment:26 Changed 3 years ago by gk

Keywords: GeorgKoppen201503R added

I'll look at it although we probably want to have more eyes reviewing the code and above all testing it on all systems we can get our hands on.

comment:27 Changed 3 years ago by mikeperry

Owner: changed from tbb-team to arthuredelstein
Status: needs_reviewassigned

comment:28 Changed 3 years ago by arthuredelstein

Here's a new version of the patch with very minor improvements, rebased on top of the latest master:

https://github.com/arthuredelstein/torbutton/commit/51e8c6d6c5fc266b9cad8dfb4f748b3be538cf71

comment:29 Changed 3 years ago by gk

Status: assignedneeds_review

comment:30 in reply to:  28 ; Changed 3 years ago by gk

Status: needs_reviewneeds_revision

Replying to arthuredelstein:

Here's a new version of the patch with very minor improvements, rebased on top of the latest master:

https://github.com/arthuredelstein/torbutton/commit/51e8c6d6c5fc266b9cad8dfb4f748b3be538cf71

Applying this to a clean 4.5a4 on Windows 8 and maximizing the windows leads
a) to the browser window moving slightly out of the screen and
b) to the height not being rounded anymore.

See attached image.

Changed 3 years ago by gk

Attachment: rounding_win8.png added

comment:31 in reply to:  30 Changed 3 years ago by arthuredelstein

Replying to gk:

Replying to arthuredelstein:

Here's a new version of the patch with very minor improvements, rebased on top of the latest master:

https://github.com/arthuredelstein/torbutton/commit/51e8c6d6c5fc266b9cad8dfb4f748b3be538cf71

Applying this to a clean 4.5a4 on Windows 8 and maximizing the windows leads
a) to the browser window moving slightly out of the screen and
b) to the height not being rounded anymore.

Thanks for finding this. I believe I have fixed it, and made some other improvements for maximizing in Linux as well.

https://github.com/arthuredelstein/torbutton/commit/5faa9b02d4f12d6959723e17870571cc15b3d468

On this version of the patch, I've done manual tests on Windows 7, OS X, and Ubuntu to confirm that maximizing and dragging look OK to me.

Last edited 3 years ago by arthuredelstein (previous) (diff)

comment:32 Changed 3 years ago by arthuredelstein

Status: needs_revisionneeds_review

(Just posted a corrected link.)

comment:33 Changed 3 years ago by arthuredelstein

I should note on this latest patch, I added code to ensure that fullscreen mode has black margins instead of gray, and is horizontally centered. So a fullscreen video, for example, gets a sort of "windowboxed" effect.

comment:34 Changed 3 years ago by gk

Status: needs_reviewneeds_revision

I have so far 3-4 issues to report all on a normal Windows 7 box:

1) I get (after maximizing) 1586X700 back (no DPI messing involved just the default 100% is used).
2) If I play with my DPI value (a lot of complaints about the current resize code running during start-up are related due to non-default DPI values) and increase the size of text etc. to 150% and start Tor Browser, I can see how the resizing is step-wise ongoing until my height is only 100 and the browser is basically not usable for me. I end up with DPI_issue1.png which is attached.
3)
a) If I enlarge the text etc. to 125%, I start with 1000x500 which is good but maximizing leads sometimes to 1200x400
b) If a) is the case and I drag the window out to the left it gets resized to 600x500 (which is okay). But if I maximize the window now, I am left with 1200x499 (which happens as well after resizing if a) is not happening).

Last edited 3 years ago by gk (previous) (diff)

Changed 3 years ago by gk

Attachment: DPI_issue1.png added

comment:35 in reply to:  34 ; Changed 3 years ago by arthuredelstein

Replying to gk:

I have so far 3-4 issues to report all on a normal Windows 7 box:

Thanks for testing!

1) I get (after maximizing) 1586X700 back (no DPI messing involved just the default 100% is used).

That is interesting. I always see properly quantized window.innerWidth/innerHeight on my Windows machine after maximizing. Are there any error messages being printed in the browser console?

2) If I play with my DPI value (a lot of complains about the current resize code running during start-up are related due to non-default DPI values) and increase the size of text etc. to 150% and start Tor Browser, I can see how the resizing is step-wise ongoing until my height is only 100 and the browser is basically not usable for me. I end up with DPI_issue1.png which is attached.
3)
a) If I enlarge the text etc. to 125%, I start with 1000x500 which is good but maximizing leads sometimes to 1200x400
b) If a) is the case and I drag the window out to the left it gets resized to 600x500 (which is okay). But if I maximize the window now, I am left with 1200x499 (which happens as well after resizing if a) is not happening).

How are you measuring these dimensions? Are you using window.innerWidth/innerHeight in the web console? At this point, zoomed pages should not be giving properly quantized values at all. As I have mentioned above, I haven't made any attempt to deal with zooming at this stage. For the case where the user zooms one tab to 125% and leaves the other at 100%, we will need to introduce a mechanism that leaves at least one tab with empty margins. My thinking was that it was more important to get it working on various platforms at 100% zoom.

comment:36 in reply to:  35 ; Changed 3 years ago by gk

Replying to arthuredelstein:

Replying to gk:

I have so far 3-4 issues to report all on a normal Windows 7 box:

Thanks for testing!

1) I get (after maximizing) 1586X700 back (no DPI messing involved just the default 100% is used).

That is interesting. I always see properly quantized window.innerWidth/innerHeight on my Windows machine after maximizing. Are there any error messages being printed in the browser console?

No, alas not.

2) If I play with my DPI value (a lot of complains about the current resize code running during start-up are related due to non-default DPI values) and increase the size of text etc. to 150% and start Tor Browser, I can see how the resizing is step-wise ongoing until my height is only 100 and the browser is basically not usable for me. I end up with DPI_issue1.png which is attached.
3)
a) If I enlarge the text etc. to 125%, I start with 1000x500 which is good but maximizing leads sometimes to 1200x400
b) If a) is the case and I drag the window out to the left it gets resized to 600x500 (which is okay). But if I maximize the window now, I am left with 1200x499 (which happens as well after resizing if a) is not happening).

How are you measuring these dimensions? Are you using window.innerWidth/innerHeight in the web console?

I visit browserspy.dk/screen.php

At this point, zoomed pages should not be giving properly quantized values at all. As I have mentioned above, I haven't made any attempt to deal with zooming at this stage. For the case where the user zooms one tab to 125% and leaves the other at 100%, we will need to introduce a mechanism that leaves at least one tab with empty margins. My thinking was that it was more important to get it working on various platforms at 100% zoom.

I think you misunderstood me. I did not zoom in the browser. I left it totally untouched in this regard and took a clean, unmodified Tor Browser. All I was doing is changing the DPI settings system-wide which quite some Windows users at least are doing it seems. See: http://www.techrepublic.com/blog/windows-and-office/get-a-better-view-in-windows-7-by-adjusting-dpi-scaling/ for what I meant.

comment:37 in reply to:  36 Changed 3 years ago by arthuredelstein

Replying to gk:

Replying to arthuredelstein:

Replying to gk:

I have so far 3-4 issues to report all on a normal Windows 7 box:

Thanks for testing!

1) I get (after maximizing) 1586X700 back (no DPI messing involved just the default 100% is used).

That is interesting. I always see properly quantized window.innerWidth/innerHeight on my Windows machine after maximizing. Are there any error messages being printed in the browser console?

No, alas not.

Hmm, I wonder what is going on here.

2) If I play with my DPI value (a lot of complains about the current resize code running during start-up are related due to non-default DPI values) and increase the size of text etc. to 150% and start Tor Browser, I can see how the resizing is step-wise ongoing until my height is only 100 and the browser is basically not usable for me. I end up with DPI_issue1.png which is attached.
3)
a) If I enlarge the text etc. to 125%, I start with 1000x500 which is good but maximizing leads sometimes to 1200x400
b) If a) is the case and I drag the window out to the left it gets resized to 600x500 (which is okay). But if I maximize the window now, I am left with 1200x499 (which happens as well after resizing if a) is not happening).

How are you measuring these dimensions? Are you using window.innerWidth/innerHeight in the web console?

I visit browserspy.dk/screen.php

At this point, zoomed pages should not be giving properly quantized values at all. As I have mentioned above, I haven't made any attempt to deal with zooming at this stage. For the case where the user zooms one tab to 125% and leaves the other at 100%, we will need to introduce a mechanism that leaves at least one tab with empty margins. My thinking was that it was more important to get it working on various platforms at 100% zoom.

I think you misunderstood me. I did not zoom in the browser. I left it totally untouched in this regard and took a clean, unmodified Tor Browser. All I was doing is changing the DPI settings system-wide which quite some Windows users at least are doing it seems. See: http://www.techrepublic.com/blog/windows-and-office/get-a-better-view-in-windows-7-by-adjusting-dpi-scaling/ for what I meant.

I did indeed misunderstand. Sorry about that -- I wasn't aware of the DPI settings issue. I'll try experimenting with DPI the next time I have a chance.

comment:38 in reply to:  34 Changed 3 years ago by arthuredelstein

Replying to gk:

I have so far 3-4 issues to report all on a normal Windows 7 box:

1) I get (after maximizing) 1586X700 back (no DPI messing involved just the default 100% is used).

My guess is this was caused by a failure of the gBrowser.contentWindow.innerWidth to update properly, which I have observed occasionally. Momentarily increasing gBrowser.width by 1 seems to fix this.

2) If I play with my DPI value (a lot of complains about the current resize code running during start-up are related due to non-default DPI values) and increase the size of text etc. to 150% and start Tor Browser, I can see how the resizing is step-wise ongoing until my height is only 100 and the browser is basically not usable for me. I end up with DPI_issue1.png which is attached.

DPI seems to cause some weird rounding errors when we call window.resizeTo(...). The shrinking window overshoots and causes the content window to further shrink, which causes the window to shrink more, etc. So I added 2-pixel margin of safety so that the window can't overshoot.

3)
a) If I enlarge the text etc. to 125%, I start with 1000x500 which is good but maximizing leads sometimes to 1200x400

Same solution as (2).

b) If a) is the case and I drag the window out to the left it gets resized to 600x500 (which is okay). But if I maximize the window now, I am left with 1200x499 (which happens as well after resizing if a) is not happening).

I hope this is fixed by the same trick in (1).

I have also made some improvements which gets zoomed windows to behave correctly (occasionally inner width/height are off by 1, such as 199 instead of 200, which has to do with rounding error by Firefox, and doesn't seem to be fixable).

I did a number of tests on Windows at multiple DPIs, and it seems to be in better shape. I also tested on OS X, and Ubuntu with Gnome, and KDE. (An extra fix was introduced to get KDE to work properly.) As far as I can tell, the response to the user resizing or maximizing the Window works consistently in all OSs.

One new problem I ran into, is that when I rebased my patch on top of the latest master, I found that the new anti-maximizing notice from #7225 actually changes the content window inner width/height! I'm not sure what the best way to deal with this is.

Here's my patch. In testing it may be useful to set torbutton loglevel to 3, to see the window innerWidth/Height in real time as you resize/maximize the window.

https://github.com/arthuredelstein/torbutton/commit/1295a13778b9b46d0a819b8ad5cfc355b1a16332

comment:39 Changed 3 years ago by arthuredelstein

Status: needs_revisionneeds_review

comment:40 Changed 3 years ago by gk

Status: needs_reviewneeds_revision

Here are new testing results:

1) If you enter fullscreen mode and reload the page the window height is not rounded anymore (I get this on Linux OSX and Windows; basically the height is always off by one)

2) On Ubuntu with Unity if I move the browser window out of the screen I get to a point where the window gets resized if I am just dropping it. If I do that I get a quite small window (which is okay) that has always the same size even if I try to maximize it again (which is not okay); while I have a similar functionality on Windows I don't have that problem there.

3) Setting DPI to 150% and maximizing on Windows leaves me with 900 x 450 (fullscreen stays the same after reloading).

4) Setting DPI to 125% and maximizing on Windows leaves me with 1250 x 500 (fullscreen is 1250 x 625 after reloading).

5) If I make the menu bar visible with DPI 125% on Windows, the height gets off by 1.

6) I did not test the zoom behavior extensively. But just shrinking the window with every zoom step (regardless whether the text gets smaller or larger) seems strange to me. If you zoom a couple of times there is almost no window left. I think we should think a bit harder about that case, preferably in a different ticket.

Wrt the #7255 patch: I think you can just revert it and implement a hint for the user that explains what happened after the resize (as we discussed in #7255). Having the fix for #7255 with the patch for this ticket makes not much sense.

Needs_revision is mainly for the latter. I think we could ship the patch in the next alpha (maybe modulo the zoom part) with the user notification changes. I am interested in getting feedback from other setups.

I am starting with the code review now.

Last edited 3 years ago by gk (previous) (diff)

comment:41 in reply to:  40 Changed 3 years ago by gk

Replying to gk:

I am starting with the code review now.

I think the code is fine. One nit:

        // If the user has stopped resizing the window after 1 second, then we can resize
        // the window so no gray margin is visible.

does not make sense where it currently is.

comment:42 in reply to:  40 ; Changed 3 years ago by arthuredelstein

Replying to gk:

1) If you enter fullscreen mode and reload the page the window height is not rounded anymore (I get this on Linux OSX and Windows; basically the height is always off by one)

5) If I make the menu bar visible with DPI 125% on Windows, the height gets off by 1.

I only seem to see this off-by-one effect when zoom is other than 1x. Do you see this with 1x zoom?

6) I did not test the zoom behavior extensively. But just shrinking the window with every zoom step (regardless whether the text gets smaller or larger) seems strange to me. If you zoom a couple of times there is almost no window left. I think we should think a bit harder about that case, preferably in a different ticket.

New ticket: #15474

Wrt the #7255 patch: I think you can just revert it and implement a hint for the user that explains what happened after the resize (as we discussed in #7255). Having the fix for #7255 with the patch for this ticket makes not much sense.

Needs_revision is mainly for the latter. I think we could ship the patch in the next alpha (maybe modulo the zoom part) with the user notification changes. I am interested in getting feedback from other setups.

I'm working on adding a user notification and disabling the zoom.

comment:43 in reply to:  42 Changed 3 years ago by gk

Replying to arthuredelstein:

Replying to gk:

1) If you enter fullscreen mode and reload the page the window height is not rounded anymore (I get this on Linux OSX and Windows; basically the height is always off by one)

5) If I make the menu bar visible with DPI 125% on Windows, the height gets off by 1.

I only seem to see this off-by-one effect when zoom is other than 1x. Do you see this with 1x zoom?

I did not play with zoom and just used a default Tor Browser. What I did was loading browserspy.dk/screen.php -> go into fullscreen -> reload with Ctrl + Shift + R. This caused some reflow and the result was a height off by 1. EDIT: This was meant to belong to 1). Wrt 5) the default Tor Browser thing holds as well but there I just enabled the the menu bar and reloaded browserspy.dk/screen.php.

Last edited 3 years ago by gk (previous) (diff)

comment:44 Changed 3 years ago by arthuredelstein

We have committed a version of this patch to torbutton where zooming has been disabled:
https://gitweb.torproject.org/torbutton.git/commit/?id=ddd8a6a78c56feeba2c8d5493edb6970ec148588

Let's keep this ticket open while we work on fixing the problems identified by gk and any other issues yet to be discovered.

comment:45 Changed 3 years ago by gk

When starting or, sometimes, when loading an other site I get tons of:

[03-28 21:42:07] Torbutton INFO: zoom 1X chromeWin 1000x972 container 1000x901 gBrowser 1000x900 content 1000x900
[03-28 21:42:07] Torbutton INFO: zoom 1X chromeWin 1000x971 container 1000x900 gBrowser 1000x900 content 1000x900

in my console. This can go on for seconds until it magically stops. I think there are two things that come to mind here:

1) There is no need for starting the resizing dance at all if the browser window is already properly rounded after start-up (as in my case)
2) Once a proper width + height is reached stop we are fine and done

comment:46 in reply to:  45 ; Changed 3 years ago by gk

Replying to gk:

When starting or, sometimes, when loading an other site I get tons of:

[03-28 21:42:07] Torbutton INFO: zoom 1X chromeWin 1000x972 container 1000x901 gBrowser 1000x900 content 1000x900
[03-28 21:42:07] Torbutton INFO: zoom 1X chromeWin 1000x971 container 1000x900 gBrowser 1000x900 content 1000x900

And now i seem to be stuck in a loop that already lasts for minutes:

[03-28 22:11:20] Torbutton INFO: zoom 1X chromeWin 1000x973 container 1000x902 gBrowser 1000x900 content 1000x900
[03-28 22:11:20] Torbutton INFO: zoom 1X chromeWin 1000x972 container 1000x901 gBrowser 1000x900 content 1000x900

comment:47 in reply to:  46 ; Changed 3 years ago by arthuredelstein

Replying to gk:

Replying to gk:

When starting or, sometimes, when loading an other site I get tons of:

[03-28 21:42:07] Torbutton INFO: zoom 1X chromeWin 1000x972 container 1000x901 gBrowser 1000x900 content 1000x900
[03-28 21:42:07] Torbutton INFO: zoom 1X chromeWin 1000x971 container 1000x900 gBrowser 1000x900 content 1000x900

And now i seem to be stuck in a loop that already lasts for minutes:

[03-28 22:11:20] Torbutton INFO: zoom 1X chromeWin 1000x973 container 1000x902 gBrowser 1000x900 content 1000x900
[03-28 22:11:20] Torbutton INFO: zoom 1X chromeWin 1000x972 container 1000x901 gBrowser 1000x900 content 1000x900

This loop should stop as soon as you move your mouse over the browser window. In fact you are seeing the "rebuild" function which "jogs" the window size.

The reason for this jog (on linux) is because of the followng scenario:

  1. The user drags the window
  2. The user holds the mouse down, but stops dragging
  3. "shrinkwrap" is called and the browser attempts to resize the window to rounded dimensions
  4. The window manager (such as GNOME or KDE) doesn't allow resizing because as far as it is concerned, the "dragging" event is continuing
  5. The browser draws the window contents as shrunk to 200i x 100j, but the window manager does not. So a large white space appears.

There is no way for the browser to know about this discrepancy. We can easily fix the discrepancy with a single call to rebuild, *unless* the user is still holding the mouse down. So we need to keep calling rebuild periodically, until we can be sure the user is no longer holding on to the drag event. The only way to be sure of this is if a mouseover is detected. So we keep calling rebuild every 250 ms until a mouseover happens.

comment:48 in reply to:  47 ; Changed 3 years ago by gk

Replying to arthuredelstein:

There is no way for the browser to know about this discrepancy. We can easily fix the discrepancy with a single call to rebuild, *unless* the user is still holding the mouse down. So we need to keep calling rebuild periodically, until we can be sure the user is no longer holding on to the drag event. The only way to be sure of this is if a mouseover is detected. So we keep calling rebuild every 250 ms until a mouseover happens.

But what if I use the browser mainly via keyboard (as I often do e.g.) or if someone is just loading a page and leaving the computer for quite a while? This behavior made my log grow quite quickly. I wonder even if scripts could detect this additional continuing load under certain circumstances given high precision JS timers. How expensive is this operation?

comment:49 Changed 3 years ago by gk

Another thing I noticed: Pressing the Alt-key alone makes my menu bar visible/hidden. Alas, this happens quite often if I am trying to switch windows with Alt + TAB or using some browser shortcuts and am failing in both cases. The side-effect is that my window is getting smaller every time which is quite confusing.

comment:50 in reply to:  48 ; Changed 3 years ago by gk

Replying to gk:

Replying to arthuredelstein:

There is no way for the browser to know about this discrepancy. We can easily fix the discrepancy with a single call to rebuild, *unless* the user is still holding the mouse down. So we need to keep calling rebuild periodically, until we can be sure the user is no longer holding on to the drag event. The only way to be sure of this is if a mouseover is detected. So we keep calling rebuild every 250 ms until a mouseover happens.

But what if I use the browser mainly via keyboard (as I often do e.g.) or if someone is just loading a page and leaving the computer for quite a while? This behavior made my log grow quite quickly. I wonder even if scripts could detect this additional continuing load under certain circumstances given high precision JS timers. How expensive is this operation?

Just to give some data: I can easily see that updateDimensions() is called 50 times a second. Oh, and moving the mouse on the window does not always stop that problem . Not sure in which cases this happens, though.

comment:51 Changed 3 years ago by gk

Two other things I noticed: On my Linux box with XFCE if I maximize the window it gets maximized then resized back to my old size and then maximized again and the properly rounded. The second maximizing is sometimes not happening (I observed that while loading pages with lots of resources, like browserspy.dk/screen.php) and I am stuck with my old window size despite me wanting to maximize the window.

comment:52 in reply to:  50 ; Changed 3 years ago by arthuredelstein

Replying to gk:

Replying to gk:

Replying to arthuredelstein:

There is no way for the browser to know about this discrepancy. We can easily fix the discrepancy with a single call to rebuild, *unless* the user is still holding the mouse down. So we need to keep calling rebuild periodically, until we can be sure the user is no longer holding on to the drag event. The only way to be sure of this is if a mouseover is detected. So we keep calling rebuild every 250 ms until a mouseover happens.

But what if I use the browser mainly via keyboard (as I often do e.g.) or if someone is just loading a page and leaving the computer for quite a while? This behavior made my log grow quite quickly. I wonder even if scripts could detect this additional continuing load under certain circumstances given high precision JS timers. How expensive is this operation?

Just to give some data: I can easily see that updateDimensions() is called 50 times a second. Oh, and moving the mouse on the window does not always stop that problem . Not sure in which cases this happens, though.

Phooey, that is not supposed to happen. I discovered a mistake in my code that loads the resizing code once per tab instead of once per window. Here's my patch for review:
https://github.com/arthuredelstein/torbutton/commit/e083a635ee8e4287ee8b5df1f4c548f041baa4db

comment:53 Changed 3 years ago by arthuredelstein

Status: needs_revisionneeds_review

comment:54 Changed 3 years ago by mcs

Cc: brade mcs added

comment:55 Changed 3 years ago by ln5

Here's what happened to me when upgrading (automatic upgrade) 4.5a4 -> 4.5a5 on a system running a tiling window manager:

  1. after restart, the tb window had a constantly flickering grey border
  2. switching to another window not possible -- "it switched back"
  3. setting extensions.torbutton.resize_windows -> false fixed the issue -- browser now behaves like before

comment:56 Changed 3 years ago by gk

Another thing I encountered while disabling the resizing by flipping extensions.torbutton.resize_windows is that this is actually resizing (i.e. shrinking the width at least as much as possible) the window a last time. The expected behavior would be to do nothing to the window at all in this case.

comment:57 Changed 3 years ago by gk

Keywords: tbb-4.5-alpha TorBrowserTeam201503R GeorgKoppen201503R removed
Status: needs_reviewneeds_revision

The patch looks good to me. Given the numerous issues that are still open it won't make it into the 4.5 stable, though, as we need some more iterations to get things right.

comment:58 Changed 3 years ago by gk

See #15535 and #15537 for some related bug reports.

comment:59 in reply to:  57 Changed 3 years ago by arthuredelstein

Replying to gk:

The patch looks good to me. Given the numerous issues that are still open it won't make it into the 4.5 stable, though, as we need some more iterations to get things right.

Sounds reasonable. Unfortunately, on Twitter and in comments on the blog, I'm seeing a lot of complaints about autoresizing. (On #7256 I've posted a version of the patch that uses zooming rather than autoresizing.)

comment:60 Changed 3 years ago by mikeperry

FWIW, I think the right thing to do is to continue to merge fixes to this for the various edge cases for a while. We can always switch the pref off by default for the release if it still seems that many remain.

I think we should also post dev snapshot XPIs (with sha256sums and detatched gpg sigs) to this bug and to related bugs as we fix things, with the pref still on. People will continue to provide feedback if we make it easy for them to test fixes.

comment:61 in reply to:  52 Changed 3 years ago by gk

Replying to arthuredelstein:

Phooey, that is not supposed to happen. I discovered a mistake in my code that loads the resizing code once per tab instead of once per window. Here's my patch for review:
https://github.com/arthuredelstein/torbutton/commit/e083a635ee8e4287ee8b5df1f4c548f041baa4db

I cherry-picked that one as it looked good to me and merged it as commit 046bce076aea058e41dfc05e7949429bdf800a2b. Not sure what else was ready for review/merge. Note: https://lists.torproject.org/pipermail/tor-qa/2015-April/000586.html has some hints that there are really performance impacts with the Linux workaround.

comment:62 in reply to:  58 ; Changed 3 years ago by gk

Replying to gk:

See #15535 and #15537 for some related bug reports.

+ #15560

comment:63 in reply to:  62 Changed 3 years ago by mikeperry

Replying to gk:

Replying to gk:

See #15535 and #15537 for some related bug reports.

+ #15560

Likely also +#15608

comment:64 Changed 3 years ago by arthuredelstein

Here's a new patch for improving the window dimension-rounding behavior:

https://github.com/arthuredelstein/torbutton/commits/da4f7262ee4794a9aa2e751256fda56dd920f7f8

If anyone would like to test this, you can download the torbutton XPI here:
https://people.torproject.org/~arthuredelstein/downloads/torbutton-1.9.1.0.xpi
Signed by me:
https://people.torproject.org/~arthuredelstein/downloads/torbutton-1.9.1.0.xpi.asc

The sha256sum of the XPI is:
f2c1a7a3606538ecaa77397d98bc27a42ff331dbd86c917f1762c20c53d39f74

To test it, after verifying, open the torbutton XPI in Tor Browser and restart. (You'll need to make sure that the extensions.torbutton.resize_windows pref is set to true.)

The main changes are:

  • Windows are now being quantized to 100x100 because I found that interval to be substantially less annoying than 200x100. Question: can we afford that 1 bit of extra entropy?
  • Non-maximized windows stay at 100% zoom and "shrink" to the nearest multiple of 100x100, so that no margins are visible.
  • Shrinking now only happens after the user actively resizes the window. This change in policy gets rid of the problem where pressing "ALT" to show the menu bar, or zooming, causing an additional window shrink.
  • Maximized windows don't shrink (because the user intends to fill the screen), but the content window is zoomed to minimize the margin at the bottom of the window. Margins remain at left and right. I took this approach because most pages wrap their text horizontally and scroll downwards.
  • The contents of "docked" windows (which fill the screen vertically but not horizontally) are zoomed to fill the available height to minimize the margin at the bottom of the window (similar to maximized windows).
  • On systems with tiling window managers, the window is never resized, but the contents are zoomed to best fit the tile aspect ratio and minimize margins.
  • In fullscreen mode, the contents are shaped and zoomed to best fit the screen aspect ratio and minimize margins.
  • The background "multiple resizing attempts" on linux have been removed, because these caused many problems. Instead, in the (hopefully) unusual case where a resize may have failed, the window waits for a mouseover or keypress to definitively shrink the margins.
  • Windows "DPI" settings are now accounted for.
  • Window zoom is reset whenever the user browses to a new domain.
  • The "resize new window" code in torbutton.js has been modified slightly so that the "content-sizer.js" code doesn't interfere with it.
Last edited 3 years ago by arthuredelstein (previous) (diff)

comment:65 in reply to:  64 Changed 3 years ago by bernie.allen

Replying to arthuredelstein:

Working well for me so far. Fixes the problems I reported in #15535.

Other feedback:

  • One thing that feels a little awkward to me is the shrinking happens about a second after resize. I do not know if that is due to the "ALT" problem you mention. I would prefer if it shrunk instantly.
  • After re-enabling resize_windows, installing the XPI, and restarting, the window had a very small and bad size. I do not know if this would be the case for a first-time Tor Browser run.
  • Sometimes after maximizing, the margin is only on the right. Sometimes, it appears on both the left and right. I am not sure why. I prefer the latter.

Very cool feature.

Last edited 3 years ago by bernie.allen (previous) (diff)

comment:66 Changed 3 years ago by bernie.allen

Tor Browser is always starting with a too-small window for me using the XPI from comment 64--even if I close the browser with a larger window size. Would be glad to provide more details on my system/configuration if it might be helpful.

comment:67 Changed 3 years ago by bernie.allen

Also experiencing uncommanded zooming sometimes using the XPI from comment 64. I focus another window (perhaps on another desktop), then refocus the Tor Browser window, and the content area is zoomed to 114%.

Last edited 3 years ago by bernie.allen (previous) (diff)

comment:68 Changed 3 years ago by gk

Without looking at the code and without testing all the new things: During start-up and after pressing "New Identity" weird things happen now: on start-up the window is good then gets pretty small then gets larger as needed and then resized again to the already good dimensions I had which is pretty confusing. "New Identity" does not give me a new window I can work with in the proper dimensions immediately (as it was up to now) but rather I just see a very small window in the upper left with the about:tor page in it which expands after several seconds [sic] to the expected size.

comment:69 in reply to:  66 ; Changed 3 years ago by arthuredelstein

Replying to bernie.allen:

Tor Browser is always starting with a too-small window for me using the XPI from comment 64--even if I close the browser with a larger window size. Would be glad to provide more details on my system/configuration if it might be helpful.

Thanks for all this testing! Yes, it would be very helpful if you can mention your OS version and desktop manager or window manager. Also, were the windows maximized when you saw uncommanded zooming?

comment:70 in reply to:  68 Changed 3 years ago by arthuredelstein

Replying to gk:

Without looking at the code and without testing all the new things: During start-up and after pressing "New Identity" weird things happen now: on start-up the window is good then gets pretty small then gets larger as needed and then resized again to the already good dimensions I had which is pretty confusing. "New Identity" does not give me a new window I can work with in the proper dimensions immediately (as it was up to now) but rather I just see a very small window in the upper left with the about:tor page in it which expands after several seconds [sic] to the expected size.

Wow, I have no idea where several seconds are coming from. What OS and desktop environment did you observe these things on?

comment:71 in reply to:  69 Changed 3 years ago by bernie.allen

Replying to arthuredelstein:

Replying to bernie.allen:

Tor Browser is always starting with a too-small window for me using the XPI from comment 64--even if I close the browser with a larger window size. Would be glad to provide more details on my system/configuration if it might be helpful.

Thanks for all this testing! Yes, it would be very helpful if you can mention your OS version and desktop manager or window manager.

Debian 7 + XFCE.

Also, were the windows maximized when you saw uncommanded zooming?

Yes, I think so. Will try to pin down the exact circumstances.

comment:72 in reply to:  68 Changed 3 years ago by bernie.allen

Replying to gk:

Without looking at the code and without testing all the new things: During start-up and after pressing "New Identity" weird things happen now: on start-up the window is good then gets pretty small then gets larger as needed and then resized again to the already good dimensions I had which is pretty confusing. "New Identity" does not give me a new window I can work with in the proper dimensions immediately (as it was up to now) but rather I just see a very small window in the upper left with the about:tor page in it which expands after several seconds [sic] to the expected size.

I observe both these behaviors. From comment 51 it looks like you are using XFCE too.

Last edited 3 years ago by bernie.allen (previous) (diff)

comment:73 Changed 3 years ago by arthuredelstein

I think I have fixed most of the issues observed by Georg and Bernie.

  • I moved the quantizeBrowser(...) call to run after the resize_new_window stuff in torbutton.js, so that there is no mutual interference at all now. Formerly, interference was causing weird window sizes on startup and New Identity in linux.
  • A resize event was being accidentally dropped on linux, which probably resulted in the late zooming and expansion and inconsistent centering effects observed.
  • I sped up the shrinking to 0.5 sec on linux. Unfortunately, it's not possible to make the shrinking instantaneous because there is no signal available to Tor Browser (or Firefox) indicating when the user has stopped drag-resizing the window. It's possible that the user has only paused for 0.5 sec, in which case attempting to shrink the window fails and a badly drawn window needs to be "rebuilt". We rebuild the window at the point when the user starts to use the browser (such as a mouseover or keypress). So a longer wait decreases the likelihood that we will need to rebuild the window, and a shorter wait is more appealing during normal use. Hopefully 500 ms is a reasonable compromise.

Here's the new patch:
https://github.com/arthuredelstein/torbutton/commit/26660a0e87
To try this out, you can download the torbutton XPI here:
https://people.torproject.org/~arthuredelstein/downloads/torbutton-1.9.1.0-26660a0e87.xpi
Signed by me:
https://people.torproject.org/~arthuredelstein/downloads/torbutton-1.9.1.0-26660a0e87.xpi.asc
The sha256sum of the XPI is:
54da4364d961195a470d059126071d4e98b6ab3dc22d12cf29da72c920b28a05

comment:74 in reply to:  73 Changed 3 years ago by bernie.allen

Replying to arthuredelstein:

How can one find your signing key? I am sorry to admit I skipped checking the signature and only checked the sha256sum last time.

comment:75 Changed 3 years ago by bernie.allen

Verified the signature now. I was using gpg incorrectly. Sorry for the noise.

I found the key ID of the signing key from trying to verify without it. Then I downloaded that key ID from a key server.

Last edited 3 years ago by bernie.allen (previous) (diff)

comment:76 Changed 3 years ago by bernie.allen

With XPI in comment 73 there is always a grey border of 2-3 pixels at the bottom of the window and 1 pixel at the right of the window. The grey border on the right only appears after the first resize. It is very much a tolerable imperfection to me but I thought I should mention it. Again I am using Debian 7 + XFCE.

I do not observe any of the previous problems (yet). Will try to test on some more common system configurations.

Last edited 3 years ago by bernie.allen (previous) (diff)

comment:77 Changed 3 years ago by bernie.allen

There are common display resolutions and OS configurations, so there are probably some common sizes for the content area when the window is maximized, right. For instance, 1920x1080 with a default Windows 7 install. I am wondering if it might be a future improvement on this feature to quantize to those sizes when maximized. Then the content area would be fully maximized without grey borders for users with the most common configurations--users who are perhaps more "normal" and more likely to be annoyed by this feature. Users with the same display resolutions but different configurations would probably not be far off from the common content area sizes.

Last edited 3 years ago by bernie.allen (previous) (diff)

comment:78 in reply to:  76 ; Changed 3 years ago by arthuredelstein

Replying to bernie.allen:

With XPI in comment 73 there is always a grey border of 2-3 pixels at the bottom of the window and 1 pixel at the right of the window. The grey border on the right only appears after the first resize. It is very much a tolerable imperfection to me but I thought I should mention it. Again I am using Debian 7 + XFCE.

Thanks again for the detailed testing!

I have now gotten rid of those borders. I also fixed a bug where activation/deactivation by the pref wasn't working properly.

Here's the new version:

https://github.com/arthuredelstein/torbutton/commit/4715ab2330
https://people.torproject.org/~arthuredelstein/downloads/torbutton-1.9.1.0-4715ab2330.xpi
https://people.torproject.org/~arthuredelstein/downloads/torbutton-1.9.1.0-4715ab2330.xpi.asc
hash: 3f53e183cee4aee19a4fe91ebcd4f6b7c92707972a52c0667db43467b3a7b010

comment:79 in reply to:  77 Changed 3 years ago by arthuredelstein

Replying to bernie.allen:

There are common display resolutions and OS configurations, so there are probably some common sizes for the content area when the window is maximized, right. For instance, 1920x1080 with a default Windows 7 install. I am wondering if it might be a future improvement on this feature to quantize to those sizes when maximized. Then the content area would be fully maximized without grey borders for users with the most common configurations--users who are perhaps more "normal" and more likely to be annoyed by this feature. Users with the same display resolutions but different configurations would probably not be far off from the common content area sizes.

I have been considering variations on this idea. The problem is that toolbars, menus, and so on can result in many different maximized content sizes. So it's not clear to me what the best preferred size would be.

One possible future improvement would be a C++ patch for Firefox that allows more granular zooming levels. Then we could more precisely fill the height of the screen when the window is maximized.

Changed 3 years ago by bernie.allen

comment:80 in reply to:  78 ; Changed 3 years ago by bernie.allen

Replying to arthuredelstein:

I have now gotten rid of those borders.

Thanks for the new build to try. The border on the right is gone, but there is still always a 2-3 pixel border on the bottom for me, which becomes larger on maximization. I attached a screenshot.

Also, things go haywire if you enable a bottom toolbar, like the search toolbar or developer toolbar, then maximize.

Last edited 3 years ago by bernie.allen (previous) (diff)

comment:81 in reply to:  80 ; Changed 3 years ago by arthuredelstein

Replying to bernie.allen:

Replying to arthuredelstein:

I have now gotten rid of those borders.

Thanks for the new build to try. The border on the right is gone, but there is still always a 2-3 pixel border on the bottom for me, which becomes larger on maximization. I attached a screenshot.

Thanks. The bottom 2-pixel border is apparently built-in to Firefox on XFCE. I can't seem to get rid of it. Unmaximized windows get no pixels at the bottom of GNOME or OS X.

Maximizing adds a small margin to the bottom because the are only a limited set of allowed zoom sizes. Possibly a C++ patch to Firefox could fix that.

Also, things go haywire if you enable a bottom toolbar, like the search toolbar or developer toolbar, then maximize.

Ugh, I see what you mean. I'll work on that tomorrow.

comment:82 Changed 3 years ago by bernie.allen

I tried XPI from comment 78 in a Windows 7 VM at 1028x768:

  • Window cannot be restored if you minimize from maximized. It disappears permanently.
  • Displays warning to not maximize when you maximize. (This warning is no longer relevant, right?)
  • Displays large grey borders and zooms too large when maximized.
  • Stretching window to maximum height causes grey border at bottom. (Is this "docking"?)

Would like to try to debug some of these things, but my Firefox/JS development experience is almost nil so best I can do currently is report.

Last edited 3 years ago by bernie.allen (previous) (diff)

comment:83 in reply to:  81 Changed 3 years ago by bernie.allen

Replying to arthuredelstein:

Thanks. The bottom 2-pixel border is apparently built-in to Firefox on XFCE. I can't seem to get rid of it. Unmaximized windows get no pixels at the bottom of GNOME or OS X.

I confirmed this border is also present in Iceweasel without Torbutton. My mistake.

Last edited 3 years ago by bernie.allen (previous) (diff)

comment:84 in reply to:  82 ; Changed 3 years ago by arthuredelstein

Replying to bernie.allen:

I tried XPI from comment 78 in a Windows 7 VM at 1028x768:

  • Displays warning to not maximize when you maximize. (This warning is no longer relevant, right?)
  • Displays large grey borders and zooms too large when maximized.

Could you send a screenshot, by any chance? Also, what is the zoom level?

  • Stretching window to maximum height causes grey border at bottom. (Is this "docking"?)

Yes, it's supposed to be "docking" where, instead of shrinking the window, we zoom to fill height (given available allowed zooming levels). How large is the grey margin at the bottom?

I also managed to get the Tor Browser window to disappear from the desktop entirely once, but I am not sure how.

Oh no. If you set the pref "extensions.torbutton.loglevel" to 3, and you open the Browser Console, you should be able to see a log of useful parameters after each resize event. I wonder if there might be any clues there.

Would like to try to debug some of these things, but my Firefox/JS development experience is almost nil so best I can do currently is report.

This is all super helpful -- thanks again. I'm hoping to squash all serious bugs in this feature this week.

comment:85 in reply to:  84 ; Changed 3 years ago by bernie.allen

Replying to arthuredelstein:

I also managed to get the Tor Browser window to disappear from the desktop entirely once, but I am not sure how.

Oh no. If you set the pref "extensions.torbutton.loglevel" to 3, and you open the Browser Console, you should be able to see a log of useful parameters after each resize event. I wonder if there might be any clues there.

It occurs when you minimize from maximized. The window disappears and cannot be restored.

I see the resize event messages in the browser console, but no messages when I maximize then minimize.

comment:86 in reply to:  84 ; Changed 3 years ago by bernie.allen

Replying to arthuredelstein:

Replying to bernie.allen:

I tried XPI from comment 78 in a Windows 7 VM at 1028x768:

  • Displays large grey borders and zooms too large when maximized.

Could you send a screenshot, by any chance? Also, what is the zoom level?

I now think this is expected behavior, just the display resolution is so low at 1024x768 that the zooming to 125% makes the content too large. If I increase the display resolution it looks OK when maximized.

Will attach screenshots anyway. The see-through part of the window in the first screenshot is seen in 4.0.8, and is not due to this change.

Edit: I misnamed the screenshots, the display resolution used is 1024x768.

Last edited 3 years ago by bernie.allen (previous) (diff)

comment:87 in reply to:  84 Changed 3 years ago by bernie.allen

Replying to arthuredelstein:

Replying to bernie.allen:

  • Stretching window to maximum height causes grey border at bottom. (Is this "docking"?)

Yes, it's supposed to be "docking" where, instead of shrinking the window, we zoom to fill height (given available allowed zooming levels). How large is the grey margin at the bottom?

I'd guess 40-50 pixels. It only seems to occur if you are already zoomed before docking. So if you maximize (which zooms to 125% for me) then dock to the left, you get the 40- to 50-pixel grey margin at the bottom.

If you dock when zoomed at 100%, it zooms to fill the area without grey margin.

comment:88 in reply to:  86 Changed 3 years ago by arthuredelstein

Replying to bernie.allen:

Replying to arthuredelstein:

Replying to bernie.allen:

I tried XPI from comment 78 in a Windows 7 VM at 1028x768:

  • Displays large grey borders and zooms too large when maximized.

Could you send a screenshot, by any chance? Also, what is the zoom level?

I now think this is expected behavior, just the display resolution is so low at 1024x768 that the zooming to 125% makes the content too large. If I increase the display resolution it looks OK when maximized.

I think 125% is higher than it's supposed to be and I think I know what caused this bug. I've made a fix here:

https://github.com/arthuredelstein/torbutton/commit/cb741c1ff3
https://people.torproject.org/~arthuredelstein/downloads/torbutton-1.9.1.0-cb741c1ff3.xpi
https://people.torproject.org/~arthuredelstein/downloads/torbutton-1.9.1.0-cb741c1ff3.xpi.asc
hash: e11764e727ca49e2bc953bf813095667544890c1415c8294f810c2b9afe5551c

Will attach screenshots anyway. The see-through part of the window in the first screenshot is seen in 4.0.8, and is not due to this change.

Something we should try to fix, but perhaps not on this ticket.

comment:89 in reply to:  85 ; Changed 3 years ago by arthuredelstein

Replying to bernie.allen:

Replying to arthuredelstein:

I also managed to get the Tor Browser window to disappear from the desktop entirely once, but I am not sure how.

Oh no. If you set the pref "extensions.torbutton.loglevel" to 3, and you open the Browser Console, you should be able to see a log of useful parameters after each resize event. I wonder if there might be any clues there.

It occurs when you minimize from maximized. The window disappears and cannot be restored.

Can you see the window on your system toolbar? Or if you press ALT+TAB can you see it in the list of available windows?

comment:90 in reply to:  89 ; Changed 3 years ago by bernie.allen

Replying to arthuredelstein:

It occurs when you minimize from maximized. The window disappears and cannot be restored.

Can you see the window on your system toolbar? Or if you press ALT+TAB can you see it in the list of available windows?

Yes, it's still in the taskbar, and it is in the ALT-TAB list. However, clicking/selecting it does nothing. I can right click on the Tor Button icon in the task bar and choose "Open New Window", but after several seconds it says "Tor Browser is already running, but is not responding". (This is with the XPI in 78. I haven't installed the new one yet.)

After minimizing from maximized I see an empty white rectangle or two (window outlines, I think) flash in the top left corner of the screen before they disappear.

Do you have a Windows 7 installation to reproduce this on? You probably already know, but https://www.modern.ie/ has free Windows VMs.

Last edited 3 years ago by bernie.allen (previous) (diff)

comment:91 in reply to:  90 ; Changed 3 years ago by arthuredelstein

Replying to bernie.allen:

Replying to arthuredelstein:

It occurs when you minimize from maximized. The window disappears and cannot be restored.

Can you see the window on your system toolbar? Or if you press ALT+TAB can you see it in the list of available windows?

Yes, it's still in the taskbar, and it is in the ALT-TAB list. However, clicking/selecting it does nothing. I can right click on the Tor Button icon in the task bar and choose "Open New Window", but after several seconds it says "Tor Browser is already running, but is not responding". (This is with the XPI in 78. I haven't installed the new one yet.)

After minimizing from maximized I see an empty white rectangle or two (window outlines, I think) flash in the top left corner of the screen before they disappear.

Do you have a Windows 7 installation to reproduce this on? You probably already know, but https://www.modern.ie/ has free Windows VMs.

I didn't know about that! Here's a new revision that (I think) fixes the Tor Browser hang on minimizing the window in Windows 7:

​​https://github.com/arthuredelstein/torbutton/commit/ce23ee30ab
​​https://people.torproject.org/~arthuredelstein/downloads/torbutton-1.9.1.0-ce23ee30ab.xpi
​​https://people.torproject.org/~arthuredelstein/downloads/torbutton-1.9.1.0-ce23ee30ab.xpi.asc
hash: 0ffe6fc939414d41a84e048beca920c2d126f17e7e13a73b79330676b0eb2ae3

Last edited 3 years ago by arthuredelstein (previous) (diff)

comment:94 Changed 3 years ago by bernie.allen

Thanks, I confirmed that the XPI in comment 91 fixes the minimizing hang.

There is still the warning that appears when maximizing the window, which does not seem like it would be relevant with this feature turned on?

comment:95 in reply to:  94 Changed 3 years ago by arthuredelstein

Replying to bernie.allen:

Thanks, I confirmed that the XPI in comment 91 fixes the minimizing hang.

There is still the warning that appears when maximizing the window, which does not seem like it would be relevant with this feature turned on?

Yes, I think we will likely remove the warning if and when this feature is turned on by default.

comment:96 Changed 3 years ago by bernie.allen

The hash in comment 91 is incorrect, by the way.

As long as the signature verifies, the risk is that it is something else signed with your key, and not what is intended? This did not seem like a big risk so I installed anyway.

comment:97 Changed 3 years ago by bernie.allen

For what it's worth, I installed on a small Debian 7 XFCE laptop, display resolution 1024x600, and the zoom when maximized is too much. Screenshot to be attached. It would seem better without zooming.

comment:98 in reply to:  97 ; Changed 3 years ago by arthuredelstein

Replying to bernie.allen:

For what it's worth, I installed on a small Debian 7 XFCE laptop, display resolution 1024x600, and the zoom when maximized is too much. Screenshot to be attached. It would seem better without zooming.

Thanks. Sorry for pasting the wrong hash. What is the zoom level?

The reason for zooming is that, if we don't zoom at all, then there will be a large grey margin at the bottom. Zooming out in this case be nicer, but it's impossible to get precise multiples of 100 when we zoom out.

comment:99 in reply to:  98 ; Changed 3 years ago by bernie.allen

Replying to arthuredelstein:

What is the zoom level?

119%

The reason for zooming is that, if we don't zoom at all, then there will be a large grey margin at the bottom. Zooming out in this case be nicer, but it's impossible to get precise multiples of 100 when we zoom out.

That makes sense.

The zoom ratio is potentially higher for lower display resolutions because 100 pixels is a greater proportion of the total height. In my case I think I am just short of the next 100 pixel cutoff when maximized, which causes a large zoom. So I reconfigured the Tor Browser UI to give more vertical space, and now maximizing only causes a 101% zoom, which looks great.

1024x600 is probably an unusually low resolution, and reconfiguring the UI is a fine solution to me. Two other possible solutions though:

  1. Zoom out instead of in when you are close to the next multiple. For instance, if you are at 499 pixels you only need the tiniest zoom out to make it to 500. But you need a large zoom in to remove a 99-pixel border. I think the zoom out should be small enough that it does not cause an accessibility problem.
  2. Make the quantization interval smaller for smaller sizes (e.g., 50 pixels), so the maximum zoom is less. But I understand there is already concern that 100 pixels adds too much entropy.

Edit: Nevermind on 1, I see you already said that won't work.

Last edited 3 years ago by bernie.allen (previous) (diff)

comment:100 in reply to:  99 ; Changed 3 years ago by arthuredelstein

Replying to bernie.allen:

Replying to arthuredelstein:

What is the zoom level?

119%

The reason for zooming is that, if we don't zoom at all, then there will be a large grey margin at the bottom. Zooming out in this case be nicer, but it's impossible to get precise multiples of 100 when we zoom out.

That makes sense.

The zoom ratio is potentially higher for lower display resolutions because 100 pixels is a greater proportion of the total height. In my case I think I am just short of the next 100 pixel cutoff when maximized, which causes a large zoom. So I reconfigured the Tor Browser UI to give more vertical space, and now maximizing only causes a 101% zoom, which looks great.

What kind of reconfiguring did you do?

1024x600 is probably an unusually low resolution, and reconfiguring the UI is a fine solution to me. Two other possible solutions though:

  1. Zoom out instead of in when you are close to the next multiple. For instance, if you are at 499 pixels you only need the tiniest zoom out to make it to 500. But you need a large zoom in to remove a 99-pixel border. I think the zoom out should be small enough that it does not cause an accessibility problem.
  2. Make the quantization interval smaller for smaller sizes (e.g., 50 pixels), so the maximum zoom is less. But I understand there is already concern that 100 pixels adds too much entropy.

Edit: Nevermind on 1, I see you already said that won't work.

Well, it might work sometimes. Basically if you zoom out, it's a little unpredictable whether an exact multiple of 100px can be obtained. (Zooming in always works.) We could use code that under, certain circumstances, invisibly tries zooming out and falls back to zooming in in the case of failure. The code gets more and more complex, though, so I'm hesitant to do it. I'll need to think about it more.

Last edited 3 years ago by arthuredelstein (previous) (diff)

comment:101 Changed 3 years ago by gk

Ubuntu 12.04 with XFCE/Unity: If I resize a window by grabbing and dragging it (or just try to resize it ending up with the same size) and want to enter fullscreen mode the window size stays the same while the remaining parts of the screen get filled with black. I think the browser window should get as large as possible in this case (as usual) and not stay the same size. This does work properly if I don't try to resize first.

comment:102 in reply to:  101 ; Changed 3 years ago by arthuredelstein

Replying to gk:

Ubuntu 12.04 with XFCE/Unity: If I resize a window by grabbing and dragging it (or just try to resize it ending up with the same size) and want to enter fullscreen mode the window size stays the same while the remaining parts of the screen get filled with black. I think the browser window should get as large as possible in this case (as usual) and not stay the same size. This does work properly if I don't try to resize first.

Here's a fix that at least works for me. The symptom was intermittent on my machine, so it's hard to be sure I have fully fixed the bug:

​​​https://github.com/arthuredelstein/torbutton/commit/48668d46e7
​​​https://people.torproject.org/~arthuredelstein/downloads/torbutton-1.9.1.0-48668d46e7.xpi
​​​https://people.torproject.org/~arthuredelstein/downloads/torbutton-1.9.1.0-48668d46e7.xpi.asc
hash: 523099294e5ddce68c4eb46845aa3f37e4d214822b764cd3b9bc02b727ef5127

comment:103 in reply to:  102 ; Changed 3 years ago by bernie.allen

Replying to arthuredelstein:

Replying to gk:

Ubuntu 12.04 with XFCE/Unity: If I resize a window by grabbing and dragging it (or just try to resize it ending up with the same size) and want to enter fullscreen mode the window size stays the same while the remaining parts of the screen get filled with black. I think the browser window should get as large as possible in this case (as usual) and not stay the same size. This does work properly if I don't try to resize first.

Here's a fix that at least works for me. The symptom was intermittent on my machine, so it's hard to be sure I have fully fixed the bug

Was able to reproduce this in Debian 7 + XFCE (unsurprisingly), and this XPI seemed to fix it.

A nonideality is when you go full screen it zooms according to the space left below the navigation bar and title bar, but after about a second those disappear, so you are left with a large black margin at the bottom of the screen.

Also, an XFCE edge case (that is probably not worth worrying about): If you shade a window, resize the shaded window, then unshade, there is a grey margin at the right of the window. (I personally never use window shading except by accident, and this seems like an odd thing to do.)

Last edited 3 years ago by bernie.allen (previous) (diff)

comment:104 in reply to:  100 Changed 3 years ago by bernie.allen

Replying to arthuredelstein:

Replying to bernie.allen:

So I reconfigured the Tor Browser UI to give more vertical space, and now maximizing only causes a 101% zoom, which looks great.

What kind of reconfiguring did you do?

Installed an extension that hides the tab bar when there is only one tab. Since 600 vertical pixels is so limited I usually install the extension on this particular machine. Hopefully it is safe to do.

comment:105 in reply to:  103 Changed 3 years ago by bernie.allen

Replying to bernie.allen:

A nonideality is when you go full screen it zooms according to the space left below the navigation bar and title bar, but after about a second those disappear, so you are left with a large black margin at the bottom of the screen.

Setting browser.fullscreen.animateUp to 0 seems to fix this, but only sometimes: When you press F11 you seem to randomly get either full screen with or without the navigation/title bars.

comment:106 Changed 2 years ago by gk

Keywords: GeorgKoppen201505R added

comment:107 Changed 2 years ago by arthuredelstein

Status: needs_revisionneeds_review

Here's the latest version of the patch. I made some minor simplifications and rebased on top of torbutton's master:
https://github.com/arthuredelstein/torbutton/commit/79437c47bb
​​​​https://people.torproject.org/~arthuredelstein/downloads/torbutton-1.9.2.2-79437c47bb.xpi
​​​​https://people.torproject.org/~arthuredelstein/downloads/torbutton-1.9.2.2-79437c47bb.xpi.asc
sha256: 3ddda47c64f1aab3dd86f76869bb3a61fa6d9972332cd29ce18c30759c3f0a02

Note that unfortunately, I needed to make a correction to my patch for #13875, and that patch needs to be applied to tor-browser.git in order for this patch to work correctly.

comment:108 Changed 2 years ago by gk

Keywords: TorBrowserTeam201505R added

comment:109 Changed 2 years ago by gk

Some Windows experiences with the latest round of patches:

If I set the DPI to 150%

1) the height/availHeight is always off by one on start-up.
2) the height/availHeight is always off by one if I resize manually + maximize the window + shrinking the window with the button next to the "x".
3) making the window fullscreen loses around a third of the available space. I wonder if there would be something better we could do here as it seems that there is enough screen height available to get the fullscreen a bit more fullscreen-like (see attached screenshot)

If I set the DPI to 125% then I only get the off-by-one problem very seldom after resizing manually but I can't reproduce it reliably.

Another thing I found confusing was that while resizing the width (just dragging the window) my height gets adjusted as well. There seems to be only a particular step when this is happening (if I am dragging the window and my mouse cursor is between approx. 75% and 90% of my screen width). Oh, and this is only happening if the window has max height. This only happens if I touch the DPI settings it seems.

Changed 2 years ago by gk

comment:110 Changed 2 years ago by gk

Part 1

The needs_revision is for the following parts

  if (w !== window.outerWidth || h !== window.outerWidth) {

I guess you want h !== window.outerHeight.

Looking at largestMultipleLessThan() I was wondering why you can't just use Math.floor(max / factor) * factor? Apart from that: where did you find that Math.floor() can take two parameters?
The function name might be misleading as the result can be equal to max as well.

In computeTargetZoom() could you explain why you need Math.min() there? Why not just w - xStep and h- yStep given that they are always smaller than w and y and thus the former function as a boundary. (The second Math.min() seems to be missing h as the first parameter, no?)

Last edited 2 years ago by gk (previous) (diff)

comment:111 Changed 2 years ago by gk

Status: needs_reviewneeds_revision

Part 2

Minor things and general thoughts

-s/linux/Linux
-The comment in sortBy() is confusing as least *is* best in this case if I understand that right
-if (!stop) on line 443 is redundant as the while clause is only running if !stop
-Why is the loop in updateDimensions() running 8 times? Does this just happen to work?
-There is something wrong with the Even more unfortunately sentence on line 407ff.
-The comment for canBeResized() should mention "not minimized" for completeness sake as well
-the }; on line 499 is wrongly indented.

I am wondering about usability issues for people that need to zoom certain websites to be able to read them better/at all. Just leaving the site to search something and coming back is destroying the previous zoom and they have to start over.

And what I mentioned in an earlier comment. Have you thought about possible timing side-channels given that the code is quite resource hungry. Could an attacker induce the resizing logic and get information out of it?

Could we avoid roping the zoom in to fix corner cases (and it seems given the DPI on Windows issues) and think about fixes in native code instead? Does that make sense?

And, finally: This is really nice work, Arthur, really appreciated! Just in case my comments and corner cases I find/found seem to be overly negative.

comment:112 Changed 2 years ago by gk

FWIW I think we can ship that patch (with the review modifications) pref'ed off in stable TBs as well.

comment:113 Changed 2 years ago by mikeperry

Ok, I fixed the h !== window.outerWidth issue and pushed the branch onto origin/master, so we can deploy it in 5.0a1. The other issues are not fixed yet.

We need to wait a bit for transifex to pick up the new property string and copy it to all locales, though.

We'll also need to figure out how to manage the pref differences for this for 4.5.x and 5.0..

comment:114 in reply to:  113 Changed 2 years ago by gk

Replying to mikeperry:

We need to wait a bit for transifex to pick up the new property string and copy it to all locales, though.

I picked that up in 1.9.2.5. There are already locales where it is translated. Should be okay for an alpha.

We'll also need to figure out how to manage the pref differences for this for 4.5.x and 5.0..

Given that we, ideally, have only one additional release where we have to think about that just switching preferences should be enough.

comment:115 in reply to:  111 Changed 2 years ago by arthuredelstein

Replying to gk:

Part 2

Minor things and general thoughts

-s/linux/Linux
-The comment in sortBy() is confusing as least *is* best in this case if I understand that right
-if (!stop) on line 443 is redundant as the while clause is only running if !stop
-Why is the loop in updateDimensions() running 8 times? Does this just happen to work?

It's only supposed to run once, except in some special situations, such as when DPI != 100%. Possibly there is a bug where it is running in other situations.

-There is something wrong with the Even more unfortunately sentence on line 407ff.
-The comment for canBeResized() should mention "not minimized" for completeness sake as well
-the }; on line 499 is wrongly indented.

I am wondering about usability issues for people that need to zoom certain websites to be able to read them better/at all. Just leaving the site to search something and coming back is destroying the previous zoom and they have to start over.

That's a good point. Possibly the native fix you suggested can deal with this, or we could try to store a zoom level per site.

And what I mentioned in an earlier comment. Have you thought about possible timing side-channels given that the code is quite resource hungry. Could an attacker induce the resizing logic and get information out of it?

Now that I've removed periodic updates, I don't think this code is consuming any resources, except when the user is actively resizing the window. But you have a specific attack in mind that I'm not thinking of?

Could we avoid roping the zoom in to fix corner cases (and it seems given the DPI on Windows issues) and think about fixes in native code instead? Does that make sense?

I think it does make sense and it should probably be what I look at next.

And, finally: This is really nice work, Arthur, really appreciated! Just in case my comments and corner cases I find/found seem to be overly negative.

Thanks, Georg! I didn't see it as negative -- I really appreciate your careful bug-finding and comments. If you hadn't found these problems now, the reaction from users would be much more painful! :P

I'm going through each point carefully and working on fixes.

comment:116 Changed 2 years ago by gk

This is with 5.0a1:

10:54 < ln5> ouch. latest tb "did something" with resizing of the window
10:54 < ln5> in a tiling window manager, there are pretty massive grey boarders 
             bottom and right
11:19 < ln5> GeKo: i do see the wm i'm using in that list (and am also impressed 
             that stumpwm is listed!)
11:20 < GeKo> the text gets bigger/smaller but the window dimensions stay the same
11:20 < ln5> oh, $GDMSESSION is not set on this machine
11:20 < ln5> this gdm thing seems nifty. but complicated!
11:20 < GeKo> I see
11:20 < ln5> i like things simple :)
11:20 < ln5> maybe too simple
11:21 < GeKo> no, I actually share your view
11:21 < GeKo> :)
11:22 < ln5> setting GDMSESSION before starting tb didn't change anything
11:23 < ln5> GeKo: i do spot some behaviour that could be described as zooming though
11:23 < ln5> which btw is friggin frustrating! :)
11:23 < ln5> bc switching between full screen and half screen shouldn't mess with 
             the size of text IMO
Last edited 2 years ago by gk (previous) (diff)

comment:117 in reply to:  116 Changed 2 years ago by arthuredelstein

Cc: ln5 added

Replying to gk:

This is with 5.0a1:

10:54 < ln5> ouch. latest tb "did something" with resizing of the window
10:54 < ln5> in a tiling window manager, there are pretty massive grey boarders 
             bottom and right
11:19 < ln5> GeKo: i do see the wm i'm using in that list (and am also impressed 
             that stumpwm is listed!)
11:20 < GeKo> the text gets bigger/smaller but the window dimensions stay the same
11:20 < ln5> oh, $GDMSESSION is not set on this machine
11:20 < ln5> this gdm thing seems nifty. but complicated!
11:20 < GeKo> I see
11:20 < ln5> i like things simple :)
11:20 < ln5> maybe too simple
11:21 < GeKo> no, I actually share your view
11:21 < GeKo> :)
11:22 < ln5> setting GDMSESSION before starting tb didn't change anything

Unfortunately, GDMSESSION is not really a universal way to detect a tiling window manager because not everyone uses gdm. Any suggestions on how to detect that stumpwm is in use?

11:23 < ln5> GeKo: i do spot some behaviour that could be described as zooming though
11:23 < ln5> which btw is friggin frustrating! :)
11:23 < ln5> bc switching between full screen and half screen shouldn't mess with 
             the size of text IMO

If stumpwm had been properly detected, then the zoom level would (I believe) have stayed constant if you switch from full screen to a half-screen tile. The zooming is designed to get rid of the grey borders.

comment:118 Changed 2 years ago by Diapolo

This "feature" needs a disable button or at least a working way of disabling it via about:config! When I try to disable it, I set these to false:

extensions.torbutton.startup_resize_period
extensions.torbutton.resize_windows
extensions.torbutton.resize_on_toggle
extensions.torbutton.resize_new_windows

This is working for the current browser session, but when the browser is restarted (talking about 5.0a1 here) two of them are ALWAYS set to true (extensions.torbutton.startup_resize_period and extensions.torbutton.resize_new_windows), which really sucks. Please fix this!

comment:119 Changed 2 years ago by gk

Keywords: TorBrowserTeam201505 added; TorBrowserTeam201505R GeorgKoppen201505R removed

comment:120 Changed 2 years ago by mikeperry

Keywords: TorBrowserTeam201506 added; TorBrowserTeam201505 removed

comment:121 Changed 2 years ago by Diapolo

I'm still unable to deactivate this "feature" with 5.0a2 AND don't even get any info or answer what I can do to bring in some progress here -_-.

comment:122 Changed 2 years ago by Diapolo

Ping, who is able to sort this out? Really this needs an UI option to turn it off.

comment:123 in reply to:  122 Changed 2 years ago by arthuredelstein

Replying to Diapolo:

Ping, who is able to sort this out? Really this needs an UI option to turn it off.

Sorry for not answering before. I just tried with Tor Browser 5.0a2:

  1. Started Tor Browser and confirmed windows were getting auto-resized
  2. Set "extensions.torbutton.resize_windows" pref to false
  3. Quit and restarted Tor Browser
  4. Confirmed that "extensions.torbutton.resize_windows" was still false
  5. Confirmed that windows were no longer getting resized

So I'm not sure why this isn't working for you. Can you confirm that it's still not working, and whether you are able to see any error messages in the browser console window?

Also, if you can briefly mention why you need to turn off auto-resizing, that might also be helpful. We're hoping to protect users from being fingerprinted by their window size, without negatively affecting usability (much).

comment:124 Changed 2 years ago by Diapolo

I did the following and it is NOT working, I can't disable it:

  1. Started Tor Browser and confirmed windows were getting auto-resized
  2. Set "extensions.torbutton.resize_windows" pref to false
  3. Quit and restarted Tor Browser
  4. Confirmed that "extensions.torbutton.resize_windows" was still false
  5. Windows are STILL getting resized (even if 4. does apply)

Why I don't want this is easy, at least I want to opt-out. It looks ugly as hell to see the desktop surrounding the browser window. UX wise this is THE fingerprinting protection that makes me feel upset!

comment:125 Changed 2 years ago by Diapolo

Also I'm only able to DISABLE this functionality for the current session, if I set "extensions.torbutton.startup_resize_period" to false. After a browser restart this is set to enabled again!

comment:126 in reply to:  124 Changed 2 years ago by gk

Replying to Diapolo:

I did the following and it is NOT working, I can't disable it:

  1. Started Tor Browser and confirmed windows were getting auto-resized
  2. Set "extensions.torbutton.resize_windows" pref to false
  3. Quit and restarted Tor Browser
  4. Confirmed that "extensions.torbutton.resize_windows" was still false
  5. Windows are STILL getting resized (even if 4. does apply)

Just in case we are talking past each other: this ticket has nothing to do with the resizing that happens once if you start up Tor Browser.

comment:127 Changed 2 years ago by Diapolo

Again, I can't persistently disable this (even if the resize on startup is another feature, I'm not talking about startup).

Last edited 2 years ago by Diapolo (previous) (diff)

comment:128 Changed 2 years ago by Diapolo

Once more, to clarify:

  • "extensions.torbutton.resize_windows" IS false (and persistent)
  • as long as "extensions.torbutton.startup_resize_period" is set to true, every browser window is getting resized (not just on startup)
  • if I set "extensions.torbutton.startup_resize_period" to false, NONE of the browser windows is getting resized (but that setting is reset to true on a browser restart)

comment:129 Changed 2 years ago by mikeperry

Keywords: TorBrowserTeam201507 added; TorBrowserTeam201506 removed

Move over remaining June items to July

comment:130 in reply to:  128 Changed 2 years ago by arthuredelstein

Replying to Diapolo:

Once more, to clarify:

  • "extensions.torbutton.resize_windows" IS false (and persistent)
  • as long as "extensions.torbutton.startup_resize_period" is set to true, every browser window is getting resized (not just on startup)
  • if I set "extensions.torbutton.startup_resize_period" to false, NONE of the browser windows is getting resized (but that setting is reset to true on a browser restart)

Let's move your issue to a new ticket: #16465. Can you describe there in detail what kind of resizing behavior you see, and what OS/desktop manager you are using? Thanks.

comment:131 Changed 2 years ago by mikeperry

Keywords: TorBrowserTeam201507 removed

comment:132 Changed 2 years ago by gk

Ha, I played with my Alt-key per chance (I think I was bored a bit :) ) and my window started shrinking again reliably after I resized it once. Not sure whether that's the old bug (comment:49) or a new one with the same symptoms. That happens with 5.5a1.

Last edited 2 years ago by gk (previous) (diff)

comment:133 Changed 18 months ago by gk

Severity: Normal

Attached is a screen shot of what the "View Tickets" button in out trac looks like after one switches to a maximized window in our alphas/nightlies.

Changed 18 months ago by gk

Attachment: view_tickets_trac_zoom.png added

comment:134 Changed 16 months ago by gk

At least maximizing the screen seems to be broken after the switch to ESR 45. I am getting now resolutions like 940 x 627 while this was fine with ESR 38 based Tor Browsers. STR:

1) Start Tor Browser
2) Go to browserspy.dk/screen.php
3) Maximize window
4) Reload browserspy.dk/screen.php

comment:135 Changed 16 months ago by gk

As the question came up yesterday, yes, fullscreen mode is broken similarly. Some ideas Arthur tossed around were:

On #14429: I think one approach is to only attempt to round the dimensions on non-maximized screens
I think the rescaling stuff we attempted is probably not practical.
One possible way to improve #14429 is to create some kind of resizeend listener (in C++)
But that probably needs a separate implementation for each platform, so it's probably quite a bit of effort.

comment:136 Changed 10 months ago by gk

See: #20941 for a possible issue.

Note: See TracTickets for help on using tickets.