1914035 - [toolbar redesign] When the toolbar disappears, the cookies border does not remain stable on the screen and an empty space remains

Status: RESOLVED FIXED

(Firefox for Android :: Toolbar, defect)

Description

[eclaudiu64]

Attachment: mobizen_20240820_222813.mp4

User Agent: Mozilla/5.0 (Android 11; Mobile; rv:131.0) Gecko/131.0 Firefox/131.0

Steps to reproduce:

When the toolbar disappears, the cookies border does not remain stable on the screen and an empty space remains. It is best observed when the toolbar is placed at the bottom, then we make small movements and you can see empty space between the cookie border and the bottom of the screen. It can be reproduced in the new toolbar in Nightly and in the old toolbar. It can also happen when there is a cookie border at the top of the screen.

Actual results:

Empty space between the cookie border and the bottom of the screen.

Expected results:

Let that empty space not exist.

Comment 1

[eclaudiu64]

Can you take a look?

Comment 2

[Chris Peterson [:cpeterson]]

Thanks! This bug might be related to cookie banner bug 1908028 or blank space bug 1908639 or bug 1907751.

Comment 3

[Petru-Mugurel Lingurar [:petru]]

Tested on https://www.meteoromania.ro and https://www.meteoromania.ro/avertizari and I see the websites cookies banner being correctly placed in the latest Nightly.
@eclaudiu64 Can you confirm the issue seems fixed for you also?

Comment 4

[eclaudiu64]

I can reproduce this in Firefox Nightly 132. In addition, I noticed that the scroll bar, when we zoom in on a site and scroll left - right, the scroll bar is located higher, when the toolbar disappears, I will also attach a video.

Comment 5

[eclaudiu64]

Attachment: Screenshot_20240903-114301_Firefox Nightly.jpg

It happens that the cookies dialog is hidden behind the toolbar.

Comment 6

[Petru-Mugurel Lingurar [:petru]]

(In reply to eclaudiu64 from comment #5)

Created attachment 9422164 [details]
Screenshot_20240903-114301_Firefox Nightly.jpg

It happens that the cookies dialog is hidden behind the toolbar.

Thank you for reporting this!

I can't reproduce it though, curious if the QA team can offer more details about the conditions for seeing this, particularly on https://www.meteoromania.ro or other websites with fixed bottom elements, like youtube.

Comment 7

[eclaudiu64]

The fact is that it is not correctly attached to the toolbar and moves incorrectly, when moving and hiding the toolbar bar. I tested on many phones, the same behavior and with fresh installation of the app the error happens.

Comment 8

[Delia Pop (Android QA)]

Attachment: QA (3).mp4

I was able to reproduce both issues mentioned, on multiple websites that have a dynamic cookie banner:

• while the nav bar and address bar are hidden, slowly scrolling up then quickly lifting the finger from the screen will cause the empty space between the cookie banner and the bottom of the screen.
• with the empty space displayed, slowly scrolling up will move the cookie banner at the bottom of the screen > slowly scrolling up + lifting the finger from the screen, will then cause the cookie banner to become partially overlapped by the address bar.
  Tested with Google Pixel 8 Pro (Android 14) on the latest Nightly 132.0a1 from 09/04.

Comment 9

[Petru-Mugurel Lingurar [:petru]]

Thank you Delia!
Tried on 3 devices but I still can't reproduce on any of the indicated websites
But there is a related issue specific to when using the navbar which I fixed recently - bug 1912988 which solves some other issues also.
Would wait to see if after landing that anything changed.

Comment 10

[Delia Pop (Android QA)]

Attachment: media_20240912_201731_871381414956780334.mp4

I can no longer reproduce this issue on the latest Nightly 132.0a1 from 09/12.
Although the cookie banner does not entirely stick to the top of the screen (it can be seen slightly moving) while scrolling, I believe this will be solved by ticket 1917978.
I'll close this as fixed.

Comment 11

[eclaudiu64]

Attachment: VID_20240912205756128.mp4

I can still reproduce with the latest version of Firefox Nightly, the error is still there, just light taps and it reproduces.

Comment 12

[Delia Pop (Android QA)]

Thanks, will keep investigating.

Comment 13

[Petru-Mugurel Lingurar [:petru]]

I still can't reproduce but if the issue is that the fixed element does not follow the dynamic toolbar then this seems like a variation of bug 1917978.

Comment 14

[eclaudiu64]

Try enabling the Remove animations setting in your phone settings and retest.

Comment 15

[Petru-Mugurel Lingurar [:petru]]

(In reply to eclaudiu64 from comment #14)

Try enabling the Remove animations setting in your phone settings and retest.

Thank you!
Seems like this is the detail I was missing.
I can reproduce with both the navbar enabled or disabled and reproduces in the current Release (130) also.

Comment 16

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1914035 - part 1 - Update engine view clipping behavior when using the navbar r=#android-reviewers

With animations disabled onDependentViewChanged might not be called when the toolbar is
snapped to a new position.
layoutDependsOn is called though, with the right values which can be used to also update
the engine view based on the new values.

Comment 17

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1914035 - part 2 - Update engine view clipping behavior when using the old toolbar r=#android-reviewers

With animations disabled onDependentViewChanged might not be called when the toolbar is
snapped to a new position.
layoutDependsOn is called though, with the right values which can be used to also update
the engine view's vertical clipping value.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0c9787ceeda9
part 1 - Update engine view clipping behavior when using the navbar r=android-reviewers,skhan
https://hg.mozilla.org/integration/autoland/rev/0f1427140c18
part 2 - Update engine view clipping behavior when using the old toolbar r=android-reviewers,skhan

Comment 19

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/0c9787ceeda9
https://hg.mozilla.org/mozilla-central/rev/0f1427140c18

Comment 20

[Zac McKenney [:zmckenney]]

Attachment: portrait_toolbar_translationY.png

Comment 21

[Zac McKenney [:zmckenney]]

Attachment: landscape_toolbar_translationY.png

Comment 22

[Zac McKenney [:zmckenney]]

Hey Petru, I bisected a problem we were seeing in fullscreen video and these 2 commits were the cause. When going fullscreen I think we want to keep the engine view with translationY of 0 since we are removing the toolbar but it appears in EngineViewClippingBehavior.kt we are now translating it by the size of the toolbar still. I've attached screenshots but we should probably consider backing these out until we make changes.

Comment 23

[Petru-Mugurel Lingurar [:petru]]

Thanks for the ping, was able to reproduce the issue.

Seems like a conflict between us trying to manually setup the engineView and toolbars when entering fullscreen and then the layout behaviour being now more quick and constant in organizing the dependent views.
Conceptually we don't need the behaviour to be active while in fullscreen and we already reinitialize it when exiting fullscreen so a fix/improvement here is simple: disable the behaviour while in fullscreen.

Will work out a patch,
Removing the ask for QE to verify this until that lands also.

Comment 24

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1914035 - Avoid dynamic behavior for the engine view while in fullscreen r=#android-reviewers

If the toolbar is dynamic we use a CoordinatorLayout behavior to synchronize the
toolbar and the engine view.
In fullscreen the toolbars are hidden so there is no need to continue observing
layout updates and try to synchronize the two views.
The proper behavior is already set again when exiting fullscreen.

Comment 25

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:petru, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox132 to wontfix.

For more information, please visit BugBot documentation.

Comment 26

[BugBot [:suhaib / :marco/ :calixte]]

A patch has been attached on this bug, which was already closed. Filing a separate bug will ensure better tracking. If this was not by mistake and further action is needed, please alert the appropriate party. (Or: if the patch doesn't change behavior -- e.g. landing a test case, or fixing a typo -- then feel free to disregard this message)

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9260f85e046d
Avoid dynamic behavior for the engine view while in fullscreen r=android-reviewers,skhan

Comment 28

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/9260f85e046d

Comment 29

[Delia Pop (Android QA)]

Attachment: Samsung Galaxy S22 Ultra_Nightly133.mp4

I can no longer reproduce the issue initially reported, where the cookie banner was partially overlapped by the address bar in the latest Nightly 133.0a1 from 10/03 with Google Pixel 8 Pro (Android 14) and Samsung Galaxy S22 Ultra (Android 14).
Additionally, the issue with a empty toolbar space displayed on a full screen video (shown in Comments 20 and 21) is also no longer reproducible.

I've also tested this with the "Remove animations" option enabled and apart from the issue mentioned in Comment 10, with the cookie banner not sticking to the top of the screen (it can be seen slightly moving) while scrolling, no other issues were observed.