Automated rounding of content window dimensions
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.
Activity
Here is the patch: https://github.com/arthuredelstein/torbutton/commit/d5a337ac06433b06e8d9738a03803e1c86eca238
Trac:
Status: new to needs_review
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 (moved)) 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 (moved)) is an interesting approach which might get us around all of the rounding issue...
Trac:
Keywords: TorBrowserTeam201501R deleted, tbb-torbutton added
Status: needs_review to needs_revisionReplying 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 (moved)) 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 (moved)) 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.
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:
- Auto-trimming the window to hide gray areas after a user-requested resize.
- 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.
- Providing a whitelist of typical window sizes, such as maximized sizes on various screens.
- 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.)
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
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 2^k^ [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.
Trac:
Username: guywith2c- 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":
-
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 (moved)) 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 (moved) + 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 wantWe 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 (moved)) 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...
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 (moved), 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 (moved)).
-
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:
-
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 (closed)...
-
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.
Trac:
Cc: N/A to gacar -
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_windowspref, which is on by default:https://github.com/arthuredelstein/torbutton/commit/276b44bd28df432cdaa968156118d169c3a46484
-
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..
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...
-
Trac:
almost-not-quite.diff -
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?
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
shrinkwrapruns, 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 andshrinkwrapruns 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?
-
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
shrinkwrapruns, 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 andshrinkwrapruns 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?)
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.
Trac:
Status: needs_review to newToday 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
Trac:
Status: new to needs_reviewHere'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
-
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.
Trac:
Status: needs_review to needs_revision 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.
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.
-
I have so far 3-4 issues to report all on a normal Windows 7 box:
- I get (after maximizing) 1586X700 back (no DPI messing involved just the default 100% is used).
- 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.
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).
Trac:
Status: needs_review to needs_revision Replying to gk:
I have so far 3-4 issues to report all on a normal Windows 7 box:
Thanks for testing!
- 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?
- 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.
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.
-
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!
- 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.
- 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.
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.
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!
- 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.
- 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.
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.
Replying to gk:
I have so far 3-4 issues to report all on a normal Windows 7 box:
- 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.
- 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.
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 (closed) 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
-
Here are new testing results:
-
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)
-
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.
-
Setting DPI to 150% and maximizing on Windows leaves me with 900 x 450 (fullscreen stays the same after reloading).
-
Setting DPI to 125% and maximizing on Windows leaves me with 1250 x 500 (fullscreen is 1250 x 625 after reloading).
-
If I make the menu bar visible with DPI 125% on Windows, the height gets off by 1.
-
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 (moved) 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 (moved)). Having the fix for #7255 (moved) 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.
Trac:
Status: needs_review to needs_revision -
-
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.
Replying to gk:
- 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)
- 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 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 (moved)
Wrt the #7255 (moved) 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 (moved)). Having the fix for #7255 (moved) 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.
