1594455 - Change the letterboxing background to match the theme, and reposition the content

Status: NEW

(Core :: Window Management, enhancement, P3)

Description

[Tom Ritter [:tjr] (OOTO until mid-August)]

This patch uplift's Tor's work in https://trac.torproject.org/projects/tor/ticket/32220

Comment 1

[Tom Ritter [:tjr] (OOTO until mid-August)]

Attachment: Bug 1594455 - Change letterboxing background to match theme, snap browser content to top

Comment 2

[richard (Tor Project)]

Attachment: Bug 1594455 - Change the letterboxing background to match the theme, and reposition the content

Second revision of letterboxing UX improvement patch

Comment 3

[Thorin [:thorin]]

STR

• use Tor Browser with the 32220 patch (I'm using 9.5a3)
• make sure LB is on and make sure the bookmarks toolbar is off. It's important that when you start TB, that the first inner window is 1000x1000 (or 1000x900 etc)
• close TB so we start clean
• open Tor Browser, the actual/real inner window is 1000x1000
• open a "new window" from the hamburger menu = the actual/real inner window is 1002x1001
• close the "new window"
• from the original window, click new identity - the subsequent actual/real inner window is 1002x1001
• there is something different about the very first window upon firefox.exe starting, and subsequent ones (where firefox.exe is not closed)
• of course, LB will protect end users with web content

AFAICT what happens is the "new window" or "new ID window" opens at 1000x1000 and 1ms later it changes to 1002x1001 : LBing kicks in much later, see Bug 1556016 ). This has something to do with the borders (1px each to left/right = 2pixels width, and 1px bottom = 1px in height).

---

Note: all of this requires user interaction, but that's not hard to do IMO. But it's also nothing compared to the entropy that can arise in Bug 1556016

I have a PoC for all of this, which displays the results (and debugs some info to the console as well) which I will attach later (on TZP) to the new window test. I'll let you know when it's all done - I need to do some more testing. Interestingly enough, some other things to mention

• with new windows set to open as new tabs (browser.link.open_newwindow.restriction = 0 = TB default), i.e we are not measuring an actual new window, I can still detect the 2px/1px change
• I can also detect the 2px/1px change without opening a new window/tab: when the chrome is resized
• the 2px/1px change indicates if LB is enabled or not
  • note: I can also detect when a 2px/1px change is NOT from this patch (e.g. TB8.5 where the LB margins are 2px/1px)
• I can use the same technique to bypass the window.open() clamping in TB8 series
• the technique is simply to constantly ask the new window about it's sizes and record a history of changes: from pre-render, to borders applied, to LB kicking in

I don't think there's anything to do here except clamp window.open (tor bug 31821, Bug #1556016), which would stop off all of the above in my PoC

Unless you also want to fix the new-win/new-ID difference in the STR. I think by matching the theme, people won't even notice it's there.

Comment 4

[Thorin [:thorin]]

The new window test at https://ghacksuserjs.github.io/TorZillaPrint/TorZillaPrint.html#screen is up, and will output the history of changes for you

(In reply to Simon Mainey from comment #3)

• I can also detect the 2px/1px change without opening a new window/tab: when the chrome is resized

Actually, I can only detect this when entering full screen, and it's almost instantaneous (so sometimes it fails; anecdotally 20% of the time) and instead of e.g. 2560 x 1440 the first measurement is 2558 x 1439

Comment 5

[sworddragon2]

A few days ago I did enable the letterboxing feature to make a look how far it currently is and noticed that there is no separation line between the letterboxed chrome area and the content window. This did lead to the issue I noticed in bug #1595394 comment #3 and probably the bug as a whole. So it would make sense to follow Firefox's theme here and also add the same separation line between the letterboxed chrome area and the content window that Firefox uses between its chrome area and the content window (in non-resistFingerprinting mode).

Comment 6

[sworddragon2]

Edit: It seems that Bugzilla does link to wrong comments (it exposes the linked comment as #2 while I point with my text to #3). The comment I wanted to point to is then bug #1595394 comment #4.

Comment 7

[sworddragon2]

Edit2: Nevermind, I just made a stupid mistake (sometimes I wished you could just edit your posts).

Comment 8

[Thorin [:thorin]]

Attachment: FWIW: aligning content to the top solves findbar results (highlight all is not necessary, just used to show the results) being vertically misaligned. Tested with TB alpha. Pic is FF stable

Comment 10

[Thorin [:thorin]]

comment #8 no longer seems to be an issue

Bug 1591054 and Bug 1593585 are also solved by .. see Bug 1593585#c1 Tor Browser patches

Other Tor Browser patches also fixed

• https://gitlab.torproject.org/tpo/applications/tor-browser/-/issues/41434 - inner measurements leaked in secondary new tabs
• https://gitlab.torproject.org/tpo/applications/tor-browser/-/issues/40081 - fixes an issue with the patch that added LBing borders (this one), but the fix also tightens up newwin size issues
• https://gitlab.torproject.org/tpo/applications/tor-browser/-/issues/41433 - LB exemptions - see https://gitlab.torproject.org/tpo/applications/tor-browser/-/merge_requests/440
• 1px white matter in full screen videos (can't find issue)
• maybe something about pdfs in FS ?

Awaiting upstreaming ... the code is becoming a bit divergent ... reminding Richard :)

Comment 11

[richard (Tor Project)]

ma1 has actually taken over letterboxing improvements on the tor-browser side (some time ago actually) so I will assign this over to him.