Title: [281446] trunk
Revision
281446
Author
mrobin...@webkit.org
Date
2021-08-23 08:11:36 -0700 (Mon, 23 Aug 2021)

Log 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.

Modified Paths

Diff

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (281445 => 281446)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2021-08-23 15:02:30 UTC (rev 281445)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2021-08-23 15:11:36 UTC (rev 281446)
@@ -1,3 +1,14 @@
+2021-08-23  Martin Robinson  <mrobin...@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  <cdu...@apple.com>
 
         WebKit2 can only have one active navigation policy check for a given frame

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-transforms-translate-expected.txt (281445 => 281446)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-transforms-translate-expected.txt	2021-08-23 15:02:30 UTC (rev 281445)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-transforms-translate-expected.txt	2021-08-23 15:11:36 UTC (rev 281446)
@@ -1,5 +1,5 @@
 
 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
 

Modified: trunk/Source/WebCore/ChangeLog (281445 => 281446)


--- trunk/Source/WebCore/ChangeLog	2021-08-23 15:02:30 UTC (rev 281445)
+++ trunk/Source/WebCore/ChangeLog	2021-08-23 15:11:36 UTC (rev 281446)
@@ -1,3 +1,19 @@
+2021-08-23  Martin Robinson  <mrobin...@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  <cgar...@igalia.com>
 
         Create a RenderLineBreak when BR element has unsupported content data style

Modified: trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp (281445 => 281446)


--- trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp	2021-08-23 15:02:30 UTC (rev 281445)
+++ trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp	2021-08-23 15:11:36 UTC (rev 281446)
@@ -488,8 +488,9 @@
     // Compute the container-relative area within which the sticky element is allowed to move.
     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() -
             FloatSize(enclosingClippingBox.borderLeft() + enclosingClippingBox.paddingLeft(),
@@ -508,15 +509,16 @@
     // Ideally, it would be possible to call this->localToContainerQuad to determine the frame
     // 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) {
         stickyBoxRelativeToScrollingAncestor.move(-FloatSize(enclosingClippingBox.borderLeft() + enclosingClippingBox.paddingLeft(),
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to