Setting screen resolution to a multiple of 200 x 100 is not working reliably with overlaid toolbar buttons
There are cropping more and more issues up with the patch developed in #8478 (closed). We should investigate either a better approach or tune the existing one to fix the problem reliably.
Activity
Some random notes:
The currently known issues are:
- https://trac.torproject.org/projects/tor/ticket/8478#comment:25 ff.
- #9738 (closed)
- beginning with TBB 3.0b1 there are regressions visible on my Linux box and on my Windows test computer as well.
At least 1) and 3) could be avoided if we explicitly chose the small icons for the toolbar. It might be interesting to explore what caused the regression from TBB 3.0a4 to TBB 3.0b1, maybe that helps developing a better fix. Don't forget to think about #9268 (moved) when writing a next generation fix.
-
Replying to gk:
- beginning with TBB 3.0b1 there are regressions visible on my Linux box and on my Windows test computer as well. That's due to the fancy new HTTPS Everywhere toolbar button, sigh.
-
I have been looking into this for the last days and have not found a way to solve this issue properly with extension means. The problem we face is described pretty well by https://bugzilla.mozilla.org/show_bug.cgi?id=870346#c1: The outerWindow dimensions are set, then the window is made visible and then depending on other extensions overlaying stuff the size of the innerWindow is adapted. Thus, having a fixed innerWindow size regardless the overlayed content of other extensions AND having the resizing we do not visible to the user is currently not possible. If we want to keep our current model (resizing the inner dimensions to a multiple 200 x 100 and faking the outer dimensions accordingly) we have a bunch of ad-hoc solutions for this ticket. We could:
- Use only small icons
- Craft a specific HTTPS Everyhwere icon that does not enlarge the chrome area anymore
- We could resize the window additionally with setTimeout(0)/or after xul-overlay-merged got fired. Thus, if the innerwindow has no multiple of 200 x 100 anymore after other overlays got merged we would enlarge the window a tiny bit again which would minimize the usability impact of it (compared to the scenario where we would enlarge the window from its original size to, say, 1000 x 900 having the window already visible).
I am not happy with either of these stopgaps, though, for various reasons (they don't solve the underlying issue; they won't be acceptable by Mozilla; the user might shoot herself in the foot with installing just one further legitimate extension...).
-
Interesting things to note: If a modal dialog is shown on start-up the dimensions are correct. The width and height are correct as well if one presses the New Identity button (without "needing" a modal dialog, i.e. that works in the default bundles, too). And: I am probably confronted with #9268 (moved) on my machine again: I only get 899 as max height although the algorithm says "900" while setting the value manually to "800" results in 800px as innerHeight.
-
Inspired by my previous comment I tried to load a modal dialog during startup but without it being visible before it gtes closed again. This works pretty well on Linux (surprisingly) and solves this issue there (tested on different computers with different distros) but alas on Windows the modal dialog is shortly visible. But even if I could fix this we would still have the Mac where all the crazy stuff is happening. It seems the content of about:tor gets loaded into that modal dialog instead of the browser window. So, this road seems blocked. But maybe the New Identity hint helps, like "clicking" the New Identity button before the browser window gets visible on start-up? If not, then I fall back to 3).
I've scraped Panopticlick results for Tor Browser 3.5 requests with screens that are 1000 px wide and between 400 and 1400 px high. **The 9 px bug seems to affect many people! **
screen = value of the video variable in the POST request to Panopticlick requests = absolute number (calculated, Panopticlick itself shows relative numbers)
screen requests 1000x600x24 68 1000x591x24 24 1000x891x24 22 1000x700x24 17 1000x900x24 16 1000x691x24 11 1000x590x24 9 1000x800x24 8 1000x500x24 6 1000x789x24 5 1000x790x24 5 1000x791x24 5 1000x883x24 5 1000x1000x24 5 1000x991x24 4 1000x787x24 3 1000x400x24 2 1000x572x24 2 1000x598x24 2 1000x599x24 2 1000x673x24 2 1000x798x24 2 1000x888x24 2 1000x892x24 2 1000x898x24 2 1000x1300x24 2 1000x508x24 1 1000x557x24 1 1000x561x24 1 1000x563x24 1 1000x565x24 1 1000x567x24 1 1000x574x24 1 1000x585x24 1 1000x587x24 1 1000x588x24 1 1000x592x24 1 1000x594x24 1 1000x595x24 1 1000x608x24 1 1000x612x24 1 1000x627x24 1 1000x639x24 1 1000x651x24 1 1000x687x24 1 1000x690x24 1 1000x699x24 1 1000x714x24 1 1000x721x24 1 1000x724x24 1 1000x728x24 1 1000x731x24 1 1000x752x24 1 1000x763x24 1 1000x769x24 1 1000x831x24 1 1000x850x24 1 1000x887x24 1 1000x890x24 1 1000x908x24 1 1000x964x24 1 1000x968x24 1 1000x999x24 1 1000x1054x24 1 1000x1211x24 1 1000x1216x24 1 1000x1240x24 1 (ommitting screens with zero requests)
I've converted the bits/denominators into absolute numbers and updated the table and attachment above. It's pretty bad:
Only 45% of requests were correctly rounded to 100 px. This of course hurts even the people for which rounding does work by shrinking the anonymity set.
Looks like at the time the first window is resized, not all extensions' toolbar buttons have necessarily been drawn yet, varying from system to system. For example, 9 px missing height (24% of requests) happens like this I think:
-
TB starts up and for some users (including me) HTTPS Everywhere presumably hasn't fully drawn its button yet.
-
TB Button gets the new window notification, does the relative resize, and all is good until ...
-
... HTTPSE fully draws its button. With "Use Small Icons" disabled (= the default), the HTTPSE button + padding + margin + border is 9 px larger vertically than the other toolbar buttons. With "Use Small Icons" enabled, the the HTTPSE button is 1 px larger vertically.
-
So either 9 or 1 px is stolen vertically from the innerHeight, and e.g. 1000x600 becomes 1000x591 or 1000x599.
-
If you open a new window manually or through New Identity, HTTPSE is now fully initialized or something and TB Button will do a good stable resize.
Maybe just close the damned first window immediately and open a new one? And could someone ask upstream if there's some kind of "all extensions have initialized" notification?
-
-
Replying to cypherpunks:
Maybe just close the damned first window immediately and open a new one? And could someone ask upstream if there's some kind of "all extensions have initialized" notification?
The tricky thing is closing the first window before it is visible + getting the resizing done properly. I did not figure out a way to get that working on all supported platforms although I tried for quite a while. And your notification does not help as the window is shown before all extensions are initialized/the chrome is overlaid.
Maybe just close the damned first window immediately and open a new one? The tricky thing is closing the first window before it is visible
Is that really neccessary? If the window is closed after it's visible then from a user's perspective there'll just be a sort of flicker. Seems okay until a better solution comes along?
(I know next to nothing about JS and Firefox extensions, so my own attempt ended up with an endless loop of windows opening and closing.)
And could someone ask upstream if there's some kind of "all extensions have initialized" notification? And your notification does not help as the window is shown before all extensions are initialized/the chrome is overlaid.
Isn't that good as well? Window opens with a wrong size for a fraction of a second, we get the notification and do the resize, and it's stable because all extensions have been initialized.
The only case not completely fixed would be starting TB with an URL argument.
-
It was the suggestion of a Mozilla dev in https://bugzilla.mozilla.org/show_bug.cgi?id=870346 and should be a minimal annoying solution for the users (i.e. the time between having the window visible and resizing it (again) should ideally be non-existent). But it seems that road is blocked as well. Thanks for testing.
Trac:
Status: needs_review to needs_revision Inspired by my previous comment I tried to load a modal dialog during startup but without it being visible before it gtes closed again. This works pretty well on Linux (surprisingly) and solves this issue there (tested on different computers with different distros) but alas on Windows the modal dialog is shortly visible.
Could you upload that code to your repo? What you describe sounds pretty good now.
-
Replying to cypherpunks:
Ding dong, the witch is dead! :)
Nice! But it is a nasty witch which came back several times already. Thus, we need to be cautious here :)
Works perfectly for me, BTW it does so even without the setTimeout(0) wrapper.
Good. I probably like to keep that wrapper for a safety margin, though...
intrigeri: Could you test that patch within your/TAILS' environment?
Replying to gk:
intrigeri: Could you test that patch within your/TAILS' environment?
Sure.
- http://browserspy.dk/screen.php reports width = 1000, height = 599, AvailHeight = 599 and AvailWidth = 1000
- panopticlick reports screen size = 1000x599
-
Replying to cypherpunks:
Does the Tails browser "Use Small Icons"?
No. Once I enable "Use Small Icons", I get 1000x600 in both tests, woo! :)
Should I conclude that the proposed patch depends on this option to be set, and that we have to change it in Tails?
Should I conclude that the proposed patch depends on this option to be set, and that we have to change it in Tails?
No no, I just encountered missing 1px when that option is set.
Could you set extensions.torbutton.loglevel to 3 before startup, and show what's logged in these lines
- "About to resize new window:"
- "Got max dimensions:"
- "Resized new window from:"
- "Mutation observer: Window dimensions are:"
- "Mutation observer: Window dimensions are (after resizing again):"
one time with small and one time with regular sized icons (small icons would also have to be enabled before startup)?
-
Replying to intrigeri:
Replying to gk:
intrigeri: Could you test that patch within your/TAILS' environment?
Sure.
- http://browserspy.dk/screen.php reports width = 1000, height = 599, AvailHeight = 599 and AvailWidth = 1000
- panopticlick reports screen size = 1000x599
Works for me with TAILS (even without the new patch). Thus, I guess your issue might be #9268 (moved). Could that be the case? You could test whether the resizing logic works on your machine if you replace
height = Math.floor(maxHeight/100.0)*100;with
height = 500;in torbutton.js.
-
Replying to gk:
Works for me with TAILS (even without the new patch). Thus, I guess your issue might be #9268 (moved). Could that be the case?
I've no idea. Could #9268 (moved) account for a 1-pixel offset?
You could test whether the resizing logic works on your machine if you replace {{{ height = Math.floor(maxHeight/100.0)*100; }}} with {{{ height = 500; }}} in torbutton.js.
If I do this, then I get 1000x500 both with and without the new patch.
-
Replying to cypherpunks:
Wait a sec, Tails ships Adblock Plus, right?
Correct.
Maybe retry with that icon removed from the toolbar...
Tails hides the ABP icon with
extensions.adblockplus.showintoolbar=false. We also hide the HTTPS Everywhere and Foxyproxy toolbar icons viauserChrome.css.With normal sized icons:
- Tails 0.22.1~rc1 + the new patch: fail
- Tails 0.22.1~rc1 + the new patch; ABP, Cookie Monster and FoxyProxy disabled: fail
- Tails 0.22.1~rc1 + the new patch; NoScript and HTTPS Everywhere disabled: fail
- Tails 0.22.1~rc1 + the new patch; ABP, Cookie Monster, FoxyProxy, NoScript and HTTPS Everywhere disabled: success
So indeed, it seems that some extension of ours might be messing up with the toolbar size in a way that Torbutton does not detect.
I've not tried disabling add-ons in small icons mode (since it works fine with all extensions enabled + the new patch).
-
Replying to cypherpunks:
Could you set extensions.torbutton.loglevel to 3 before startup, and show what's logged in these lines [...]
If it is still useful that I do this, then I'm happy too. Is it?
Note that all my tests were done with Tails 0.22.1~rc1, in a VM that is given a 1280x768 monitor. My eyes think that the browser window takes all the available vertical space.
-
Replying to intrigeri:
Replying to gk:
Works for me with TAILS (even without the new patch). Thus, I guess your issue might be #9268 (moved). Could that be the case?
I've no idea. Could #9268 (moved) account for a 1-pixel offset?
Yes, happened to me. Could you remove the panel/toolbar at the bottom in order to check if that additional space "solves" the problem?
You could test whether the resizing logic works on your machine if you replace {{{ height = Math.floor(maxHeight/100.0)*100; }}} with {{{ height = 500; }}} in torbutton.js.
If I do this, then I get 1000x500 both with and without the new patch.
Good. So the code is generally working but not in your particular situation.
-
FWIW: This patch fixed the problem as well for TBB running on Windows in a Virtualbox environment. But the Ubuntu counterpart is horribly broken which I'll track in an other bug.
Trac:
Summary: Setting screen resolution to a multiple of 200 x 100 is not working reliably with patch in #8478 (closed) to Setting screen resolution to a multiple of 200 x 100 is not working reliably with overlaid toolbar buttons -
Replying to gk:
FWIW: This patch fixed the problem as well for TBB running on Windows in a Virtualbox environment. But the Ubuntu counterpart is horribly broken which I'll track in an other bug.
No need to do so. It seems missing/broken Guest Additions where the reason for this issue. After reinstalling them everything works as expected.
-
Replying to gk:
Replying to intrigeri:
Replying to gk:
Thus, I guess your issue might be #9268 (moved). Could that be the case?
I've no idea. Could #9268 (moved) account for a 1-pixel offset?
Yes, happened to me. Could you remove the panel/toolbar at the bottom in order to check if that additional space "solves" the problem?
It does solve the problem: both with and without the patch I get 1000x500.
-
Replying to intrigeri:
Replying to gk:
Replying to intrigeri:
Replying to gk:
Thus, I guess your issue might be #9268 (moved). Could that be the case?
I've no idea. Could #9268 (moved) account for a 1-pixel offset?
Yes, happened to me. Could you remove the panel/toolbar at the bottom in order to check if that additional space "solves" the problem?
It does solve the problem: both with and without the patch I get 1000x500.
You mean 1000x600? As this (and not the test with height = 500 which was working without removing toolbars) is the one I am interested in.
-
Replying to gk:
You mean 1000x600? As this (and not the test with height = 500 which was working without removing toolbars) is the one I am interested in.
OK, let me clarify. On Tails 0.22.1~rc1, without patching Torbutton at all, without changing the browser configuration (icons size, add-ons) at all, I get 1000x600 or 1000x500, depending on the size of the screen given to my VM.
-
Replying to intrigeri:
Replying to gk:
You mean 1000x600? As this (and not the test with height = 500 which was working without removing toolbars) is the one I am interested in.
OK, let me clarify. On Tails 0.22.1~rc1, without patching Torbutton at all, without changing the browser configuration (icons size, add-ons) at all, I get 1000x600 or 1000x500, depending on the size of the screen given to my VM.
Okay, thanks. Your problem seems then indeed to be caused by #9268 (moved).
So why not push the patch as is for TBB 3.5.1 / Tails 0.22.1 (and possibly set small icons on the latter as a workaround for the x768 resolution case)?
Some future logic to fully take toolbars into consideration would be independent of this.
A few weeks after it's integrated in both TBB and Tails, I'll run Panopticlick scrape no. 2, and then another few weeks later scrape no. 3, then subtract 2 from 3 to get some data on how the situation has (hopefully) improved.
-
Replying to cypherpunks:
So why not push the patch as is for TBB 3.5.1 / Tails 0.22.1 (and possibly set small icons on the latter as a workaround for the x768 resolution case)?
Agreed for Tails. I'm on it.
This is merged in 3.5.2. I still have issues on mac due to a large dock size, possibly also #9268 (moved).
Trac:
Status: needs_review to closed
Resolution: N/A to fixedReplying to cypherpunks:
A few weeks after it's integrated in both TBB and Tails, I'll run Panopticlick scrape no. 2, and then another few weeks later scrape no. 3, then subtract 2 from 3 to get some data on how the situation has (hopefully) improved.
Okay, two scrapes on 2014-03-02 and 2014-03-12, looking again at Tor-Browser-ish requests with screens between 1000x400x24 and 1000x1400x24.
Good news: **83% of that period's requests were rounded to 100px! ** (before: 45%)
screen requests 1000x600x24 103 1000x900x24 59 1000x700x24 30 1000x800x24 16 1000x1000x24 14 1000x400x24 7 1000x777x24 4 1000x878x24 3 1000x822x24 3 1000x792x24 2 1000x711x24 2 1000x611x24 2 1000x500x24 2 1000x687x24 1 1000x866x24 1 1000x783x24 1 1000x877x24 1 1000x738x24 1 1000x879x24 1 1000x702x24 1 1000x891x24 1 1000x697x24 1 1000x691x24 1 1000x808x24 1 1000x679x24 1 1000x1025x24 1 1000x643x24 1 1000x635x24 1 1000x895x24 1 1000x602x24 1 1000x938x24 1 1000x599x24 1 1000x594x24 1 1000x589x24 1 1000x584x24 1 1000x1035x24 1 1000x579x24 1 1000x578x24 1 1000x577x24 1 1000x543x24 1 1000x978x24 1 1000x992x24 1 1000x1400x24 1 1000x1238x24 1 1000x1044x24 1 1000x1223x24 1 1000x1173x24 1 mentioned in issue #10833 (closed)
