Properly enable letterboxing (again)

It seems while rebasing in #30429 (moved) we did not realize that the letterboxing patches made it into ESR 68 yet the pref to enabled the functionality needs still to get set.

added TorBrowserTeam201909R actualpoints::0.2 component::applications/tor browser owner::tbb-team points::0.1 priority::medium resolution::fixed severity::normal status::closed tbb-9.0-must-alpha type::defect labels

Oh, and while we are at it: we should disable the notification bar in Torbutton that complains when the user tries to maximize their window.

Trac:
Summary: Properly enabled letterboxing (again) to Properly enable letterboxing (again)

Patches in https://github.com/acatarineu/tor-browser/commit/31598 and https://github.com/acatarineu/torbutton/commit/31598.

Trac:
Status: new to needs_review

Replying to acat:

Patches in https://github.com/acatarineu/tor-browser/commit/31598 and https://github.com/acatarineu/torbutton/commit/31598.

I think we want to bind the behavior to the letterboxing pref maybe? That is: if letterboxing is enabled don't show any notification if resizing is happening (but make sure that our window is still properly rounded when created) and if letterboxing is disabled (because folks don't like the new behavior) fall back to the current status quo)

Trac:
Status: needs_review to needs_revision

Yes, I think that's a good idea. Revised patch: https://github.com/acatarineu/torbutton/commit/31598+1

Trac:
Status: needs_revision to needs_review

Hm, so this leaves is in a weird situation, kind of: disabling/enabling letterboxing happens instantly. However, showing/not showing the notification box depends on the window creation state, which gives surprising results. You'd e.g. get the notification bar if you just enabled letterboxing but you started without it being on and vice versa. I think we should find a better way binding letterboxing to the notification bar.

If that's to tricky I am fine just disabling that part entirely. However, in that case we could just rip the resizelistener etc. out.

Trac:
Status: needs_review to needs_revision

Mmm, actually I thought about that, but not sure how I tested that I thought letterboxing was not being enabled instantly, just on a new window.

In any case, here is a revised patch: https://github.com/acatarineu/torbutton/commit/31598+2. Now the resize listener is always enabled and checking the pref when it's called. Being strict, this still has the issue that a user with the window already maximized changing letterboxing pref will not get any notification, since these are only shown on window resize right now. Do you think we should also handle those cases?

Trac:
Status: needs_revision to needs_review

Replying to acat:

Mmm, actually I thought about that, but not sure how I tested that I thought letterboxing was not being enabled instantly, just on a new window.

In any case, here is a revised patch: https://github.com/acatarineu/torbutton/commit/31598+2. Now the resize listener is always enabled and checking the pref when it's called. Being strict, this still has the issue that a user with the window already maximized changing letterboxing pref will not get any notification, since these are only shown on window resize right now. Do you think we should also handle those cases?

No, I think what we have now is fine. I merged your patches to torbutton (commit 1edfec57049a76052fda8eb17b6e3b8f281e3361) and tor-browser (commit f19b6946fcccb42b3b060c5ecf53ba8ec524b420), thanks!

Trac:
Status: needs_review to closed
Resolution: N/A to fixed
Keywords: TorBrowserTeam201909 deleted, TorBrowserTeam201909R added

Trac:
Actualpoints: N/A to 0.2

closed

changed time estimate to 48m

added 1h 36m of time spent

mentioned in issue #31887 (moved)

mentioned in issue #32324 (moved)

moved to tpo/applications/tor-browser#31598 (closed)

mentioned in issue tpo/applications/tor-browser#31887 (closed)