1943086 - [Toolbar redesign] Wrong toolbar values sent to APZ when exiting fullscreen

Status: RESOLVED FIXED

(Fenix :: Toolbar, defect, P2)

Description

[Petru-Mugurel Lingurar [:petru]]

Ticket opened as a followup to bug 1938966 comment 36

Steps to reproduce

1. Have the navbar feature enabled
2. Access any website showing a video which allows viewing it in fullscreen
3. Enter fullscreen
4. Exit fullscreen

Expected behavior

We inform APZ about the toolbar height and webpage being overlapped by it through setDynamicToolbarMaxHeight and setVerticalClipping .
The vertical clipping value is not bigger than the toolbar's height.

Actual behavior

For a short time we send unexpected values:

• setDynamicToolbarMaxHeight is initially called with the height of just the address bar
• setVerticalClipping is called with the height of both the addressbar and the navbar

When the initialization of the navbar finishes both calls receive the right values but might be too late for some flows as seen in bug 1938966 comment 33

Comment 1

[Petru-Mugurel Lingurar [:petru]]

Setting S2/P2 as this impacts a bigger bug / fix - bug 1938966.

Comment 2

[Petru-Mugurel Lingurar [:petru]]

An example of the data we send to APZ

09:42:29.941 exiting fullscreen
09:42:29.942 setDynamicToolbarMaxHeight: 158
09:42:29.942 setVerticalClipping: -158
09:42:29.943 setVerticalClipping: -296     <-- this is the issue. Vertical clipping is bigger than toolbar height.
09:42:29.949 setVerticalClipping: -296
09:42:29.949 setVerticalClipping: -296
09:42:29.955 setVerticalClipping: -296
09:42:29.955 setVerticalClipping: -296
09:42:29.958 setVerticalClipping: -296
09:42:29.959 setVerticalClipping: -296
09:42:29.969 setVerticalClipping: -296
09:42:29.969 setVerticalClipping: -296
09:42:29.979 setVerticalClipping: -241
09:42:29.979 setVerticalClipping: -177
09:42:29.984 setVerticalClipping: -153
09:42:29.984 setVerticalClipping: -132
09:42:29.985 setVerticalClipping: -132
09:42:29.986 setVerticalClipping: -132
09:42:29.988 setVerticalClipping: -132
09:42:29.988 setVerticalClipping: -132
09:42:30.070 setVerticalClipping: -63
09:42:30.070 setVerticalClipping: -3
09:42:30.071 setVerticalClipping: -3
09:42:30.071 setVerticalClipping: -3
09:42:30.090 setVerticalClipping: -3
09:42:30.091 setVerticalClipping: -3
09:42:30.173 setDynamicToolbarMaxHeight: 296     <-- after an updated (bigger) toolbar height everything should be good
09:42:30.216 setVerticalClipping: -2
09:42:30.216 setVerticalClipping: 0
09:42:30.220 setVerticalClipping: -2
09:42:30.220 setVerticalClipping: 0
09:42:30.311 setVerticalClipping: 0

Comment 3

[Petru-Mugurel Lingurar [:petru]]

The issue happens because we inform APZ of the toolbar height and set automatic updates of the toolbar offset in response to exiting fullscreen, but while still being in landscape so based on this check that includes a screen size also we infer that the navbar should not be shown.

Later we are informed of layout updates where both the top address bar and navbar are seen on the screen so we inform APZ of both the toolbar being hidden - two toolbars being hidden means a bigger offset value than the height of just the addressbar.

Later we reinitialize the toolbars after changing from landscape to portrait and now based on this check that includes a screen size also we infer that the navbar should be shown and so the values sent to APZ for the dynamic toolbars heights and offsets will be correct.

Comment 4

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1943086 - Coerce dynamic toolbars offset values to their heights before informing APZ r=#android-reviewers

This ensures that APZ will not be configured with a bigger toolbar offset
(a bigger dynamic toolbar translation) than it's set height.

Comment 5

[Petru-Mugurel Lingurar [:petru]]

Similar logs to those from comment 2 showing compatible values sent to APZ:

12:07:59.958 exiting fullscreen
12:07:59.959 setDynamicToolbarMaxHeight: 158
12:07:59.962 setVerticalClipping: -158
12:07:59.962 setVerticalClipping: -158
12:07:59.966 setVerticalClipping: -158
12:07:59.967 setVerticalClipping: -158
12:07:59.969 setVerticalClipping: -158
12:07:59.969 setVerticalClipping: -158
12:07:59.972 setVerticalClipping: -158
12:07:59.973 setVerticalClipping: -158
12:07:59.992 setVerticalClipping: -158
12:07:59.992 setVerticalClipping: -158
12:08:00.002 setVerticalClipping: -96
12:08:00.002 setVerticalClipping: -96
12:08:00.002 setVerticalClipping: -96
12:08:00.002 setVerticalClipping: -96
12:08:00.004 setVerticalClipping: -96
12:08:00.004 setVerticalClipping: -96
12:08:00.126 setVerticalClipping: -96
12:08:00.126 setVerticalClipping: -8
12:08:00.167 setDynamicToolbarMaxHeight: 296
12:08:00.198 setVerticalClipping: -8
12:08:00.199 setVerticalClipping: 0
12:08:00.202 setVerticalClipping: -8
12:08:00.202 setVerticalClipping: 0
12:08:00.229 setVerticalClipping: 0

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c77cb4efd79f
Coerce dynamic toolbars offset values to their heights before informing APZ r=android-reviewers,tchoh

Comment 7

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/c77cb4efd79f