1918757 - Blank space in fullscreen mode with navigation toolbar off and address bar on top

Status: RESOLVED FIXED

(Fenix :: Toolbar, defect)

Description

[Markus Stange [:mstange]]

Attachment: screen recording

[Tracking Requested - why for this release]: Regression in release configuration that's currently mostly untested on Nightly

Steps to reproduce:

1. Go to the settings, enable debug settings by tapping the Firefox logo in the about section, and turn off the navigation toolbar in the secret settings.
2. Under Customize, pick "Top" for the address bar location
3. Go to YouTube.
4. Play any regular video (not a "short") in fullscreen mode.

Expected results:
The video should cover the entire screen.

Actual results:
There is a toolbar-sized blank space at the top.

I see this both on a Samsung Galaxy S21 running Android 14 and on Pixel 6 running Android 13.

This is a recent regression. Using mozregression:

12:15.80 INFO: Narrowed nightly regression window from [2024-09-10, 2024-09-12] (2 days) to [2024-09-10, 2024-09-11] (1 days) (~0 steps left)
12:15.81 INFO: Got as far as we can go bisecting nightlies...
12:15.81 ERROR: Sorry, but mozregression was unable to get a repository - no pushlog url available.
12:15.81 INFO: Newest known good nightly: 2024-09-10
12:15.81 INFO: Oldest known bad nightly: 2024-09-11

Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ebf0e33ba93e&tochange=169a59fe35f8
Bug 1908639 seems like a good candidate. Petru, can you take a look?

Comment 1

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1918757 - part 1 - Don't update toolbars when configuration change if in fullscreen r=#android-reviewers

If in fullscreen the toolbars should not be shown so there's no point in updating them.

Comment 2

[Petru-Mugurel Lingurar [:petru]]

Removing my NI, indeed, I've introduced the regression, sorry for it and thanks for the provided details @Markus!

Comment 3

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1918757 - part 3 - Update toolbars and the engine view when exiting fullscreen r=#android-reviewers

We were already doing part of this.
Now we also reinitialize the toolbars to account for their different configurations when
the device is in portrait or landscape upon exiting fullscreen.

Comment 4

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1918757 - part 2 - Update FullScreenFeature to more eagerly update it's `isFullscreen` property r=#android-reviewers

Previously if evaluating isFullscreen inside FullScreenFeature's fullScreenChanged callback
we would get the old value and not the current status of the tab.
This change fixes that.

Comment 5

[Petru-Mugurel Lingurar [:petru]]

Talked with Mike about fixing this, he has a better approach that would fix another silent bug.

Comment 6

[Phabricator Automation]

Comment on attachment 9425484 [details]
Bug 1918757 - part 2 - Update FullScreenFeature to more eagerly update it's isFullscreen property r=#android-reviewers

Revision D222587 was moved to bug 1919706. Setting attachment 9425484 [details] to obsolete.

Comment 7

[Mike a [:mavduevskiy]]

Attachment: Bug 1918757 - Apply height params passed to initializeEngineView within the function

Hidden in if's, there was a case where the function was not propagating height params to the EngineClippingBehaviour, but calculating toolbar height values itself. The problem was, that code didn't consider the fullscreen state. The height values passed to initiazlieEngineView functions are calculated with fullscreen state and tabstrip in mind.

The bug was hidden because onConfigChange didn't reinitialize engine view, but recently it started doing that.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1d19595d9791
Apply height params passed to initializeEngineView within the function r=android-reviewers,petru

Comment 9

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/1d19595d9791