1912854 - www.vox.com - fixed-position header-bar misaligns when scrolling down

Status: VERIFIED FIXED

(Core :: Panning and Zooming, defect, P3)

Description

[Calin Tanase]

Environment:
Operating system: Android 10
Firefox version: Firefox Nightly 131.0a1-20240811212519 / Firefox Release 129.0-20240801122119

Steps to reproduce:

1. Go to https://www.vox.com/business-and-finance/365892/elon-musk-x-lawsuit-advertisers-antitrust
2. Dismiss the cookie banner.
3. Scroll down the page until the top menu bar becomes sticky.
4. Scroll up and down.

Expected Behavior:
The top menu bar remain sticky at the top of the screen.

Actual Behavior:
The top menu bar misaligns while scrolling down.

Notes:

• Reproduces regardless of the status of ETP
• Reproduces in Firefox Nightly
• Does not reproduce in Firefox Release, and Chrome

Created from https://github.com/webcompat/web-bugs/issues/140307

Comment 1

[Calin Tanase]

Attachment: Nightly vs Release vs Chrome.mp4

Comment 2

[Kiril Isakov]

@Calin Tanase Thank you for transferring my issue here from GitHub.

There is another example of the same vertical misalignment issue that I think needs to be reported here:

1. go to https://www.france24.com/en/live-news/20240813-olympic-flag-arrival-kicks-off-2028-pressure-for-los-angeles and dismiss the cookie banner, if any;

2. you'll notice vertical scroll is blocked beyond some point — the user is expected to tap on the “Read more” DOM element (or the surrounding area) to be able to read further;

3. now, the issue is that a tap on “Read more” actually results in a tap on a lower DOM element (most likely the “Watch live” pictogram of the sticky bottom menu bar), and you'll need to tap somewhere above “Read more” in order to actually tap on it.

(you may want to turn on the “Show touches” option in Android developer options in order to see where exactly your taps land on the screen)

Comment 3

[Calin Tanase]

Please file a new report for that website (on bugzilla) under the same Product/Component and we will look into it.

Comment 4

[Donal Meehan [:dmeehan]]

Setting 129 to unaffected since Comment 0 mentions this does not reproduce in release

Comment 5

[Daniel Holbert [:dholbert]]

Note, this bug only repro's with the dynamic toolbar enabled (as it is by default) -- i.e. "scroll to hide address bar and toolbar" turned on in settings|customize.

I ran mozregression and got this regression range for the vox top-toolbar positioning issue:
INFO: Newest known good nightly: 2024-08-04
INFO: Oldest known bad nightly: 2024-08-05

The "good" nightlies before this^ range have the toolbar snapped to the top when I scroll down, as expected. (It sometimes shifts down briefly while the dynamic toolbar is in-the-process-of-hiding, but it snaps back to the top when the toolbar becomes fully hidden.)

Pushlog for regression range (using hg revisions shown in these^ Nightlies' settings|about-firefox pages):
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f19c35b800f5811afa4b529f7821b073b2f10590&tochange=b7131a95dd254929f23ef852f5e34ae7cd67e37f

I'm not sure offhand what in there would've caused this. Maybe one of these ones that are Android/toolbar-related?

https://hg.mozilla.org/mozilla-central/rev/0bef67f98d4c09d89d6198246ba6a5e6060d0807
Bug 1907879 - Removed the toolbar top divider when the microsurvey prompt is displayed r=android-reviewers,amejiamarmol

https://hg.mozilla.org/mozilla-central/rev/d23ec83bdceefceb57dfb94b2ec09374ada631c5
Bug 1911085 - Avoid anchoring the snackbar to top toolbars r=android-reviewers,rsainani

Comment 6

[Daniel Holbert [:dholbert]]

(In reply to Donal Meehan [:dmeehan] from comment #4)

Setting 129 to unaffected since Comment 0 mentions this does not reproduce in release

Setting 130 to unaffected since I can't repro in Firefox beta 130 (vs. I can repro in Nightly 131). Not too surprising that beta130 is unaffected since the pushlog in comment 5 is in the version-131 Nightly timeframe. Also, we haven't enabled the new navbar beyond Nightly yet, and it seems possible this might require the navbar to trigger the issue.

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: testcase 1 (reduced)

Comment 8

[Daniel Holbert [:dholbert]]

This seems to affect any fixed-position element that has the top and bottom properties set.

testcase 1 sets top, bottom, and height to emulate a sticky top toolbar.
testcase 2 (upcoming) just sets top andbottom, leaving the default height so that the element should stretch to fill the viewport.

When I scroll down the testcases to make the dynamic toolbar hide:

• In "bad" Nightly 2024-08-05 and later, I only get "bad" behavior where the fixed-pos element detaches from the edge of the screen.
• In "good" Nightly 2024-08-04 and earlier, I almost-always get "good" behavior with the fixed-pos snapped to the edge of the screen (though I occasionally get "bad" behavior in these builds too - I'm not sure precisely what triggers it).

Comment 9

[Daniel Holbert [:dholbert]]

Attachment: testcase 2 (fullscreen fixed-pos element)

Comment 10

[Daniel Holbert [:dholbert]]

Attachment: testcase 2 (fullscreen fixed-pos element)

Comment 11

[Daniel Holbert [:dholbert]]

hiro, perhaps you could take a look here? This seems likely to be an APZ-dynamic-toolbar-handling sort of bug.

(I don't see any obviously APZ/dynamic-toolbar-viewport-layout changes in the regression range, but given that this was previously occasionally-reproducible [per end of comment 8 and upcoming screencast of "good" build] and has now switched to be 100%-reproducible, I'm suspicious that this might just be a preexisting issue that we were inadvertently papering-over via some bit of frontend code, which was e.g. nudging us to update the viewport-sizing-used-for-fixedpos-elements in some case, and one of the frontend changes like the ones in comment 5 happened to remove that paperover.)

Comment 12

[Daniel Holbert [:dholbert]]

Attachment: screencast of testcase 2 in last-good-build (mozregression datestamp 2024-08-04, rev f19c35b800f5), mostly-good but showing the bad behavior at the very end

Comment 13

[Daniel Holbert [:dholbert]]

Attachment: screencast of testcase 2 in first-bad-build (mozregression datestamp 2024-08-05, rev b7131a95dd25), *only* showing bad behavior

Comment 14

[Hiroyuki Ikezoe (:hiro)]

I would say this is a regression caused by the new navi bar on Fenix. I think this is one of the cases that the existing GeckoView::setVerticalClipping isn't sufficient for the new navi bar (That's actually what I told in the last APZ/Mobile team meeting in the last all hands).

Petru, is there a meta bug for the new navi bar? This bug should be a blocker of the meta bug.

Comment 15

[Daniel Holbert [:dholbert]]

FWIW I ran across a website that does something like my testcase 2 here, with a full-screen fixed-pos overlay that ends up being not tall enough:
https://www.natesilver.net/p/kamala-harris-cant-meme-her-way-to
(Really any substack post, I think.)

STR to trigger the problem there: scroll down until you see the "Discover more from .... Subscribe | Continue Reading | Sign in" popup, with a partially-transparent overlay that covers the page. The partially-transparent overlay isn't tall enough and looks kinda awkward (with content shining through at the top and bottom of the viewport), just as in testcase 2 and the screencast in comment 13.

Comment 16

[Petru-Mugurel Lingurar [:petru]]

Petru, is there a meta bug for the new navi bar? This bug should be a blocker of the meta bug.
Indeed, the issue seems to be specific to using the navbar, added it as a blocker of bug 1894468.

@Hiro If I understand correctly for this we need bug 1912008?

Comment 17

[Kiril Isakov]

Here's another somewhat related, vertical misalignment bug you guys may be interested in checking: https://bugzilla.mozilla.org/show_bug.cgi?id=1913081

Comment 18

[Hiroyuki Ikezoe (:hiro)]

(In reply to Petru-Mugurel Lingurar [:petru] from comment #16)

Petru, is there a meta bug for the new navi bar? This bug should be a blocker of the meta bug.
Indeed, the issue seems to be specific to using the navbar, added it as a blocker of bug 1894468.

@Hiro If I understand correctly for this we need bug 1912008?

I don't think so, bug 1912008 is just for a relatively new feature interactive-widget while the software keyboard is visible.

In the test case in comment 10, I got MOZ_ASSERT(-mDynamicToolbarMaxHeight <= aOffset && aOffset <= 0) this assertion, the mDynamicToolbarMaxHeight is 208 and the incomming aOffset is -212, thus I would say we mis-use setVerticalClipping. Well, to be precise what I told in the meeting is that we need to extend setVerticalClipping to support the new navi bar behavior.

Comment 19

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #18)

(In reply to Petru-Mugurel Lingurar [:petru] from comment #16)

Petru, is there a meta bug for the new navi bar? This bug should be a blocker of the meta bug.
Indeed, the issue seems to be specific to using the navbar, added it as a blocker of bug 1894468.

@Hiro If I understand correctly for this we need bug 1912008?

I don't think so, bug 1912008 is just for a relatively new feature interactive-widget while the software keyboard is visible.

In the test case in comment 10, I got MOZ_ASSERT(-mDynamicToolbarMaxHeight <= aOffset && aOffset <= 0) this assertion, the mDynamicToolbarMaxHeight is 208 and the incomming aOffset is -212, thus I would say we mis-use setVerticalClipping. Well, to be precise what I told in the meeting is that we need to extend setVerticalClipping to support the new navi bar behavior.

Filed bug 1914524 for the assertion.

Comment 20

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1912854 - part 1 - Don't update toolbar configurations while find in page is active r=#android-reviewers

When find in page is active all toolbars should be hidden.
Changing orientation would normally allow to update if the navbar should be shown or not
but while find in page is active there's no need for this.

Comment 21

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1912854 - part 2 - Don't show the bottom navbar after returning to app if find in page is active r=#android-reviewers

As an implementation detail - we start observing the search dialog state immediately as the app is visible
and depending on that state we might decide to show the navigation bar.
Avoid doing this while find in page is active - no other toolbar should be shown / updated.

Comment 22

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1912854 - part 3 - Have BrowserFragment handle restoring the layout after find in page ends r=#android-reviewers

One important scenario which would bring a lot of complexity to FindInPageIntegration:
Entering find in page with a specific device orientation then rotating the screen and closing
find in page while having another device orientation.
In this case the navbar might be show or not, something that is controlled from code inside of
BrowserFragment. Instead of duplicating that logic we'll leave BrowserFragment handle it.

Comment 23

[Phabricator Automation]

Comment on attachment 9420741 [details]
Bug 1912854 - part 1 - Don't update toolbar configurations while find in page is active r=#android-reviewers

Revision D220097 was moved to bug 1912484. Setting attachment 9420741 [details] to obsolete.

Comment 24

[Phabricator Automation]

Comment on attachment 9420742 [details]
Bug 1912854 - part 2 - Don't show the bottom navbar after returning to app if find in page is active r=#android-reviewers

Revision D220098 was moved to bug 1912484. Setting attachment 9420742 [details] to obsolete.

Comment 25

[Phabricator Automation]

Comment on attachment 9420743 [details]
Bug 1912854 - part 3 - Have BrowserFragment handle restoring the layout after find in page ends r=#android-reviewers

Revision D220099 was moved to bug 1912484. Setting attachment 9420743 [details] to obsolete.

Comment 26

[Petru-Mugurel Lingurar [:petru]]

Sorry for the updates here. Not really ready to work on this, I think the current status is the one described by hiro in comment 19

Comment 27

[Mike a [:mavduevskiy]]

There is a bug we introduced in 1907879 where the actual height of the toolbar is bigger that the values defined in resources (the values are used for setting the dynamicToolbarHight). Reverting that patch solves the issue, so I believe if we don't make the toolbar unintentionally larger, this bug will get resolved as well.

Meaning, the content window will snap back to the top of the screen (I still find the scrolling behaviour of the content window buggy, but this particular bug seems to be about the content block not snapping back).

Comment 28

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

Set release status flags based on info from the regressing bug 1907879

:towhite, since you are the author of the regressor, bug 1907879, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 29

[twhite]

Mike - Is the linked bug here the correct one? I don't think bug 1907879 is related?

Comment 30

[Petru-Mugurel Lingurar [:petru]]

Attachment: VoxIssueFixed.mp4

This should be fixed together with bug 1912988 which showed a similar issue.
Attached is a recording of this scenario in a build with the patches for bug 1912988 applied.

Comment 31

[Mike a [:mavduevskiy]]

(In reply to twhite from comment #29)

Mike - Is the linked bug here the correct one? I don't think bug 1907879 is related?

Yep, reverting that patch solved the issue. Debugging the current central shows that the actual toolbar height is taller than what getBottomToolbarHeight returns.

Inconsistency between what we set as toolbarHight and the actual height calculated during scrolling is causing the issue.

I guess, it would make sense either make sure that the toolbar isn't taller than defined through resources (the values that getBottomToolbar relies on) or refactor getBottomToolbarHeight to actually calculate the view height instead of relying on resources values.

Comment 32

[Petru-Mugurel Lingurar [:petru]]

With the patch for bug 1912988 being merged I thing now this should be fixed also.
QA to confirm.

Comment 33

[Kiril Isakov]

(In reply to Petru-Mugurel Lingurar [:petru] from comment #32)

With the patch for bug 1912988 being merged I thing now this should be fixed also.
QA to confirm.

Hello guys, I'm the user who reported the original bug on GitHub.

Thank you guys for the effort and dedication.

Still, two important things:

• can the transition be made smoother, crispier than that?

• there's still a voffset position bug when you switch between apps, see this screen recording I made earlier today: https://youtube.com/shorts/eoG8nErq46c

132.0a1 (Build #2016043439), hg-50016ed27344+
GV: 132.0a1-20240909092518
AS: 132.20240904050246

Comment 34

[Petru-Mugurel Lingurar [:petru]]

(In reply to Kiril Isakov from comment #33)

Still, two important things:

• can the transition be made smoother, crispier than that?

• there's still a voffset position bug when you switch between apps, see this screen recording I made earlier today: https://youtube.com/shorts/eoG8nErq46c

Thank you for detailing this cases also!
Think Botond is best suited to respond to the two scenarios described above which I see reproducing in Release also.
Could need more investigation in separate tickets.

Based on the initial report I think this ticket can be closed as fixed with the fixed element now behaving similar in Firefox Nightly and Release?

Comment 35

[Daniel Holbert [:dholbert]]

I can confirm that my attached testcases (and the Vox website) now match Firefox release, when I load them in Nightly 2024-09-10 (launched via mozregression; I'm not sure if it's available in the play store or not).

I can reproduce both of the followup issues that Kiril mentioned; I'll spin off bugs for them using my testcases attached here. As Petru noted, they both affect release, and hence it's worth considering them separately from this bug here, which was a more recent regression.

Comment 36

[Daniel Holbert [:dholbert]]

So I think we can consider this bug here to be FIXED by bug 1912988.

Comment 37

[Daniel Holbert [:dholbert]]

I filed bug 1917978 and bug 1917984 on the "two important things" that Kiril mentioned.

I think I've addressed :petru's needinfo's in comment 34 (on behalf of Kiril and botond) so I'll go ahead and cancel those.

Comment 38

[Delia Pop (Android QA)]

Attachment: QA (4).mp4

Confirming this is verified fixed on the latest Nightly 132.0a1 from 09/12.
Tested with the following devices:

• Samsung Galaxy A53 5G (Android 14)
• Xiaomi 12 Pro (Android 13)