Closed Bug 1621725 Opened 4 years ago Closed 4 years ago

[wpt-sync] Sync PR 22195 - Fix position:sticky when inside fixed subtree

Categories

(Core :: Layout: Positioned, task, P4)

Product:
Core
Component:
Layout: Positioned
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla76
Tracking Flags:
Tracking Status
firefox76 --- fixed

People

(Reporter: mozilla.org, Unassigned)

References

(
URL
)

Details

(Whiteboard: [wptsync downstream])

Sync web-platform-tests PR 22195 into mozilla-central (this bug is closed when the sync is complete).

PR: https://github.com/web-platform-tests/wpt/pull/22195
Details from upstream follow.

David Bokan <bokan@chromium.org> wrote:

Fix position:sticky when inside fixed subtree

In https://crrev.com/4d25b125dae5 I changed the scroll tree parenting
logic so that elements in position:fixed subtrees have the LayoutView's
ScrollNode as the scroll parent. This made sense since scrolling over a
fixed element should cause the document to scroll. However, this is
slightly different from how the transform tree looks. Because scrolling
the document doesn't cause position:fixed eement so translate, these
nodes don't have the LayoutView's ScrollTranslation transform node as an
ancestor.

As a simple example, a scrolling document with a position:fixed \<div>
scroller will generate the following scroll and transform trees
(approximately):

   *ScrollTree*                          *TransformTree*

      Root                                    Root
       |                                       |

VisualViewport Translation VisualViewport
| /
LayoutView Translation /
| Fixed LayoutView
Fixed Scroller Translation

The situation above makes sense for what parent-child relationships mean
in each tree: the scroll tree encodes how scrolls chain; scrolling on a
child should bubble up to its parent in this tree. The transform tree
encodes the physical effect of scrolling a node. In the above example,
scrolling from the fixed scroller should bubble up to the LayoutView
(when the scroller is fully scrolled) but scrolling the LayoutView will
not cause movement of the fixed scroller.

The above makes sense but caused sticky code to get confused. A sticky
constraint is attached to the scroll translation node. With the above
situation, this meant that inside a fixed subtree, we'd attach it to the
VisualViewport's scroll translation node. This was unexpected; the
constraints are in "document coordinates", meaning that to translate
them into the viewport space we must apply the scroll offset [1]. The
compositor would use the visual viewport's (typically 0) scroll offset
to adjust these values, leading to incorrect calculations.

This previously worked because the scroll node used in a fixed subtree
would be the visual viewport (before the CL mentioned at the top). In
[2] we check whether the current overflow clip is also our scroller,
prior to the CL this check have failed because "our scroller" was the
visual viewport but our clip was the LayoutView. Now they are both the
LayoutView.

The fix in this CL is to make the check in [2] more stringent; we also
want to make sure that our scroller is the nearest scroller in the
transform tree. That is, if we scroll it, will we cause the current node
to move? If not, we don't need a sticky constraint on the compositor
because user scrolling can't change the sticky's offset relative to its
clip.

[1] https://cs.chromium.org/chromium/src/cc/trees/property_tree.cc?l=321&rcl=628f869d1fda631a85f051ad13b5d278319298fc
[2] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/paint/paint_property_tree_builder.cc?l=553&rcl=99a5a1266f303ba6ae46174a2b4cbd165ea7e934

Bug: 1019142
Change-Id: I781943ff43514905d399803c780c6081d7d47e8f

Reviewed-on: https://chromium-review.googlesource.com/2097542
WPT-Export-Revision: 47cbde7734c2e0eca83c0a9fc6ac64b4b33703d4

Component: web-platform-tests → Layout: Positioned
Product: Testing → Core

CI Results

Ran 13 Firefox configurations based on mozilla-central, and Firefox, and Chrome on GitHub CI

Total 2 tests

Status Summary

Firefox

FAIL : 1[Gecko-windows10-64-qr-opt] 2[Gecko-android-em-7.0-x86_64-debug-geckoview, Gecko-android-em-7.0-x86_64-opt-geckoview, Gecko-linux1804-64-asan-opt, Gecko-linux1804-64-debug, Gecko-linux1804-64-opt, Gecko-linux1804-64-qr-debug, Gecko-linux1804-64-qr-opt, Gecko-windows10-64-debug, Gecko-windows10-64-opt, Gecko-windows10-64-qr-debug, Gecko-windows7-32-debug, Gecko-windows7-32-opt, GitHub]
ERROR: 1

Chrome

FAIL : 2

Links

Gecko CI (Treeherder)
GitHub PR Head
GitHub PR Base

Details

New Tests That Don't Pass

/css/css-position/position-sticky-fixed-ancestor-iframe.html: ERROR [Gecko-windows10-64-qr-opt], FAIL [Gecko-android-em-7.0-x86_64-debug-geckoview, Gecko-android-em-7.0-x86_64-opt-geckoview, Gecko-linux1804-64-asan-opt, Gecko-linux1804-64-debug, Gecko-linux1804-64-opt, Gecko-linux1804-64-qr-debug, Gecko-linux1804-64-qr-opt, Gecko-windows10-64-debug, Gecko-windows10-64-opt, Gecko-windows10-64-qr-debug, Gecko-windows7-32-debug, Gecko-windows7-32-opt, GitHub] (Chrome: FAIL)
/css/css-position/position-sticky-fixed-ancestor.html: FAIL (Chrome: FAIL)

Pushed by wptsync@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b7893b69cea5
[wpt PR 22195] - Fix position:sticky when inside fixed subtree, a=testonly
https://hg.mozilla.org/integration/autoland/rev/1520600a5838
[wpt PR 22195] - Update wpt metadata, a=testonly

https://hg.mozilla.org/mozilla-central/rev/b7893b69cea5
https://hg.mozilla.org/mozilla-central/rev/1520600a5838

Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox76: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla76
You need to log in before you can comment on or make changes to this bug.