1905441 - IntersectionObserver callback does not fire for zero-height element at the bottom of the document at some DPI scales (which breaks infinite-scroll on some sites)

Status: REOPENED

(Core :: Layout, defect)

Description

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

STR:

1. Make sure Firefox for Android has the dynamic toolbar enabled (this is the default config "Scroll to hide toolbar" enabled)
2. Load attached testcase.
3. Scroll to the very bottom of the page.

EXPECTED RESULTS:
The background should turn lime when you reach the bottom.

ACTUAL RESULTS:
The background never turns lime. It just stays pink.

I'm pretty sure this is the root cause of bug 1884144 at Taobao.

Firefox 127 release and 129 Nightly on Android give ACTUAL RESULTS -- though if I turn off the dynamic toolbar, they give EXPECTED RESULTS.

Chrome gives EXPECTED RESULTS, as does Firefox on Desktop.

Comment 1

[Daniel Holbert [:dholbert]]

The setup in the testcase here is:

• zero-height element at the very end of the document.
• an IntersectionObserver watching for that element to come into view and reach an intersectionRatio of 1.

Obviously it's a bit edge-casey to consider whether a zero-height element at the end of the document is intersecting or not-intersecting when you're scrolled to the very end. (It's zero-height and it coincides precisely with the edge of the viewport, so it's not exactly on-screen.)

But apparently there's consensus that it is intersecting with an intersectionRatio of 1, based on the browsers/configs that yield EXPECTED RESULTS in comment 0. Only Firefox-with-dynamic-toolbar considers it non-intersecting.

Comment 2

[Daniel Holbert [:dholbert]]

I guess this is bug 1788504, actually.

Comment 3

[Daniel Holbert [:dholbert]]

Hmm, dupe-target bug 1788504 landed last week, but I'm still able to reproduce this bug here with the attached testcase, in Nightly 2024-07-22 (built from a m-c commit that landed yesterday, much more recently than bug 1788504's patches).

So bug 1788504 didn't fix this, i.e. this bug is not a dupe of bug 1788504 after all.

--> moving dupe relationship to see-also relationship. emilio or hiro, maybe you could take a look here?

Comment 4

[Hiroyuki Ikezoe (:hiro)]

FWIW, on my local built GeckoView example based on this revision, the test case sometimes works, sometimes doesn't work. Odd.

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1905441 - Revert bug 1788504 partially. r?emilio

Since bug 1788504 the root scroll container's clipped region had been
over-clipped because GetScrollPortRectAccountingForMaxDynamicToolbar returns
the toolbar height divided by the current pinch zoom ratio. For example, given
that the toolbar height is 60px (in screen units) and the pinch zoom ratio is
2.0, then GetScrollPortRectAccountingForMaxDynamicToolbar returns 30px (in CSS
units), it's not sufficient for the clipped region.

The JUnit test in this change fails without this change, succeeds with this
change.

Comment 6

[Phabricator Automation]

Comment on attachment 9416512 [details]
Bug 1905441 - Revert bug 1788504 partially. r?emilio

Revision D217924 was moved to bug 1909181. Setting attachment 9416512 [details] to obsolete.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

The bug number was wrong. :/

Note that I can see this issue with static toolbar, FWIW.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

This repros as well on desktop at some zoom levels, afaict. E.g., on 130% zoom I see:

window.innerHeight // 553
let div = document.createElement("div"); div.style = "position: fixed; inset: 0;"; document.documentElement.appendChild(div); // undefined
div.getBoundingClientRect().height // 553.9166870117188
myTarget.getBoundingClientRect().y // 554.0999755859375 

So it seems that at some zoom levels we don't grow the scroll range to cover that?

This is likely to be due to this snapping here... Hiro, you have more context than me on that code, any idea of what is that trying to achieve?

Comment 9

[Hiroyuki Ikezoe (:hiro)]

I suspected it so that I tried to see with layout.scroll.disable-pixel-alignment=true, but the issue still happened, that's what I did yesterday.

Now I tried it again before leaving the above comment, with layout.scroll.disable-pixel-alignment=true the test case in comment 0 works fine on my Pixel 3 and on Android emulator. Maybe I didn't reload the test case after changing the pref.

Comment 10

[Daniel Holbert [:dholbert]]

I can confirm that the testcase works fine (changes to lime when I reach the end) if I set layout.scroll.disable-pixel-alignment=true in Firefox Nightly on my Pixel 8.