Changeset 281446 in webkit

Timestamp:

Aug 23, 2021 8:11:36 AM (11 months ago)

Author:

Martin Robinson

Message:

Sticky position should not use transformed position to compute sticky offset.
​https://bugs.webkit.org/show_bug.cgi?id=164292
<rdar://problem/29054773>

Reviewed by Simon Fraser.

LayoutTests/imported/w3c:

• web-platform-tests/css/css-position/sticky/position-sticky-transforms-translate-expected.txt: Update results

of test to show pass.

Source/WebCore:

No new tests. This change is tested by the following WPT test:

web-platform-tests/css/css-position/sticky/position-sticky-transforms-translate.html

• rendering/RenderBoxModelObject.cpp:

(WebCore::RenderBoxModelObject::computeStickyPositionConstraints const): When calling localToContainerQuad,
pass 0 for the mode which means that the transformation between coordinate systems does not include
transforms.

Location:

trunk

Files:

• LayoutTests/imported/w3c/ChangeLog (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-transforms-translate-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderBoxModelObject.cpp (modified) (2 diffs)

trunk/LayoutTests/imported/w3c/ChangeLog

@@ +1 @@
+2021-08-23  Martin Robinson  <mrobinson@webkit.org>
+
+        Sticky position should not use transformed position to compute sticky offset.
+        https://bugs.webkit.org/show_bug.cgi?id=164292
+        <rdar://problem/29054773>
+
+        Reviewed by Simon Fraser.
+
+        * web-platform-tests/css/css-position/sticky/position-sticky-transforms-translate-expected.txt: Update results
+        of test to show pass.
+
 2021-08-23  Chris Dumez  <cdumez@apple.com>
 

trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-transforms-translate-expected.txt

@@ -2 +2 @@
 PASS Translation transform can move sticky element past sticking point
 PASS Stuck elements can still be moved via translations
-FAIL The sticky element should stick before the container is offset by a translation assert_equals: expected 150 but got 100
+PASS The sticky element should stick before the container is offset by a translation
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-08-23  Martin Robinson  <mrobinson@webkit.org>
+
+        Sticky position should not use transformed position to compute sticky offset.
+        https://bugs.webkit.org/show_bug.cgi?id=164292
+        <rdar://problem/29054773>
+
+        Reviewed by Simon Fraser.
+
+        No new tests. This change is tested by the following WPT test:
+          web-platform-tests/css/css-position/sticky/position-sticky-transforms-translate.html
+
+        * rendering/RenderBoxModelObject.cpp:
+        (WebCore::RenderBoxModelObject::computeStickyPositionConstraints const): When calling localToContainerQuad,
+        pass 0 for the mode which means that the transformation between coordinate systems does not include
+        transforms.
+
 2021-08-23  Carlos Garcia Campos  <cgarcia@igalia.com>
 

trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp

@@ -489 +489 @@
     containerContentRect.contract(minMargin);
 
-    // Finally compute container rect relative to the scrolling ancestor.
-    FloatRect containerRectRelativeToScrollingAncestor = containingBlock->localToContainerQuad(FloatRect(containerContentRect), &enclosingClippingBox).boundingBox();
+    // Finally compute container rect relative to the scrolling ancestor. We pass an empty
+    // mode here, because sticky positioning should ignore transforms.
+    FloatRect containerRectRelativeToScrollingAncestor = containingBlock->localToContainerQuad(FloatRect(containerContentRect), &enclosingClippingBox, { } /* ignore transforms */).boundingBox();
     if (enclosingClippingLayer) {
         FloatPoint containerLocationRelativeToScrollingAncestor = containerRectRelativeToScrollingAncestor.location() -
@@ -509 +510 @@
     // rectangle in the coordinate system of the scrolling ancestor, but localToContainerQuad
     // itself depends on sticky positioning! Instead, start from the parent but first adjusting
-    // the rectangle for the writing mode of this stickily-positioned element.
+    // the rectangle for the writing mode of this stickily-positioned element. We also pass an
+    // empty mode here because sticky positioning should ignore transforms.
     //
-    // FIXME: For now, assume that |this| is not transformed. It would also be nice to not have to
-    // call localToContainerQuad again since we have already done a similar call to move from
-    // the containing block to the scrolling ancestor above, but localToContainerQuad takes care
-    // of a lot of complex situations involving inlines, tables, and transformations.
+    // FIXME: It would also be nice to not have to call localToContainerQuad again since we
+    // have already done a similar call to move from the containing block to the scrolling
+    // ancestor above, but localToContainerQuad takes care of a lot of complex situations
+    // involving inlines, tables, and transformations.
     if (parent()->isBox())
         downcast<RenderBox>(parent())->flipForWritingMode(stickyBoxRect);
-    auto stickyBoxRelativeToScrollingAncestor = parent()->localToContainerQuad(FloatRect(stickyBoxRect), &enclosingClippingBox).boundingBox();
+    auto stickyBoxRelativeToScrollingAncestor = parent()->localToContainerQuad(FloatRect(stickyBoxRect), &enclosingClippingBox, { } /* ignore transforms */).boundingBox();
 
     if (enclosingClippingLayer) {