2066409 - Toggling overflow on an ancestor fires a spurious scroll event on a scrolled descendant

Status: RESOLVED FIXED

(Core :: Layout: Scrolling and Overflow, defect, P3)

Description

[Dão Gottwald [:dao]]

Setting and then removing overflow on an ancestor of a scrolled element dispatches a scroll event on that element, though its scroll position and every metric are unchanged. Per spec scroll fires when the scroll position changes, so nothing should fire here.

Testcase:

<div id="host">
  <div id="scroller" style="height:100px;overflow-y:auto">
    <div style="height:2000px"></div>
  </div>
</div>

STR:

1. scroller.scrollTop = 50, let it settle
2. host.style.overflow = "hidden", read layout, host.style.removeProperty("overflow"), read layout
3. Wait a frame

Actual: one scroll event on #scroller. scrollTop, clientHeight, scrollHeight and the host's height are identical before, during and after.

Expected: no scroll event.

Only overflow does it -- overflow: clip, background-color and contain: paint, set and removed the same way, fire nothing. The frame reconstruction from the ancestor becoming a scroll container looks like the trigger.

Bug 2066403 is a consumer that hit this: panel-list sets overflow: hidden on its parent while it measures for positioning, and its own scroll-to-hide listener reads the resulting event as the anchor moving, closing the menu it just opened.

Comment 1

[Dão Gottwald [:dao]]

Attachment: Testcase: toggling overflow on an ancestor fires a scroll event

Open in Firefox; the table fills itself in on load. Each row scrolls #scroller to 50, lets it settle, then sets and removes one property on its ancestor #host and counts scroll events on #scroller.

Firefox 156.0a1 (local opt build):

set on #host, then removed	scroll events	metrics unchanged
overflow: hidden	1	yes
overflow: clip	0	yes
overflow: scroll	1	yes
background-color: red	0	yes
contain: paint	0	yes

So it is the ancestor becoming a scroll container that does it: hidden and scroll both fire one event, clip -- which clips without creating a scroll container -- fires none. scrollTop, clientHeight and scrollHeight are identical before, during and after in every row.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 2066409 - Don't fire a scroll event when restoring a non-root scroll position. r?#layout-reviewers

ScrollToRestoredPosition re-establishes a position the content already had,
either across a frame reconstruction or from session history. Nothing is
scrolled, so the scrolling events shouldn't run. Chrome fires nothing in
either case.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 2066409 - Don't fire a scroll event when a frame reconstruction restores the scroll position. r?#layout-reviewers

A reconstruction re-establishes a position the content already had, so nothing
scrolled and the scrolling events shouldn't run. Chrome fires nothing there
either.

Session history restore keeps firing, so the restore reason has to reach
ScrollToRestoredPosition: both restores go through the docShell's
nsILayoutHistoryState, and ScrollOrigin::Restore alone cannot tell them apart.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/77f86bec3345
https://hg.mozilla.org/integration/autoland/rev/31ffe32f402f
Don't fire a scroll event when restoring a non-root scroll position. r=layout-reviewers,emilio

Comment 5

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/62661 for changes under testing/web-platform/tests

Comment 6

[rperta]

https://hg.mozilla.org/mozilla-central/rev/31ffe32f402f

Comment 7

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot