Changeset 288947 in webkit

Timestamp:

Feb 2, 2022 1:12:57 AM (6 months ago)

Author:

Martin Robinson

Message:

scroll-margin-top doesn't work on inline elements
​https://bugs.webkit.org/show_bug.cgi?id=235933
<rdar://87983307>

Reviewed by Simon Fraser.

LayoutTests/imported/w3c:

• web-platform-tests/css/css-scroll-snap/scroll-target-margin-006-expected.txt: Added.
• web-platform-tests/css/css-scroll-snap/scroll-target-margin-006.html: Added.

Source/WebCore:

Test: imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-target-margin-005.html

Move the implementation of absoluteAnchorRectWithScrollMargin which actually processes
the CSS scroll-margin property to RenderElement. This allows it to be used for inline
as well as block elements. The specification says it should apply to both. The box
sizes passed in here do not matter too much, because the Length should never be a
percentage.

• rendering/RenderBox.cpp:

(WebCore::RenderBox::absoluteAnchorRectWithScrollMargin const): Deleted.

• rendering/RenderBox.h:
• rendering/RenderElement.cpp:

(WebCore::RenderElement::absoluteAnchorRectWithScrollMargin const):

• rendering/RenderElement.h:

Location:

trunk

Files:

• LayoutTests/imported/w3c/ChangeLog (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-target-margin-006-expected.txt (added)
• LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-target-margin-006.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderBox.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderBox.h (modified) (1 diff)
• Source/WebCore/rendering/RenderElement.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderElement.h (modified) (1 diff)

trunk/LayoutTests/imported/w3c/ChangeLog

@@ +1 @@
+2022-02-02  Martin Robinson  <mrobinson@webkit.org>
+
+        scroll-margin-top doesn't work on inline elements
+        https://bugs.webkit.org/show_bug.cgi?id=235933
+        <rdar://87983307>
+
+        Reviewed by Simon Fraser.
+
+        * web-platform-tests/css/css-scroll-snap/scroll-target-margin-006-expected.txt: Added.
+        * web-platform-tests/css/css-scroll-snap/scroll-target-margin-006.html: Added.
+
 2022-02-01  Antti Koivisto  <antti@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2022-02-02  Martin Robinson  <mrobinson@webkit.org>
+
+        scroll-margin-top doesn't work on inline elements
+        https://bugs.webkit.org/show_bug.cgi?id=235933
+        <rdar://87983307>
+
+        Reviewed by Simon Fraser.
+
+        Test: imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-target-margin-005.html
+
+        Move the implementation of absoluteAnchorRectWithScrollMargin which actually processes
+        the CSS scroll-margin property to RenderElement. This allows it to be used for inline
+        as well as block elements. The specification says it should apply to both. The box
+        sizes passed in here do not matter too much, because the Length should never be a
+        percentage.
+
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::absoluteAnchorRectWithScrollMargin const): Deleted.
+        * rendering/RenderBox.h:
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::absoluteAnchorRectWithScrollMargin const):
+        * rendering/RenderElement.h:
+
 2022-02-01  Alan Bujtas  <zalan@apple.com>
 

trunk/Source/WebCore/rendering/RenderBox.cpp

@@ -5473 +5473 @@
 }
 
-LayoutRect RenderBox::absoluteAnchorRectWithScrollMargin(bool* insideFixed) const
-{
-    LayoutRect anchorRect = absoluteAnchorRect(insideFixed);
-    const LengthBox& scrollMargin = style().scrollMargin();
-    if (scrollMargin.isZero())
-        return anchorRect;
-
-    // The scroll snap specification says that the scroll-margin should be applied in the
-    // coordinate system of the scroll container and applied to the rectangular bounding
-    // box of the transformed border box of the target element.
-    // See https://www.w3.org/TR/css-scroll-snap-1/#scroll-margin.
-    const LayoutSize boxSize = size();
-    const LayoutBoxExtent margin(
-        valueForLength(scrollMargin.top(), boxSize.height()),
-        valueForLength(scrollMargin.right(), boxSize.width()),
-        valueForLength(scrollMargin.bottom(), boxSize.height()),
-        valueForLength(scrollMargin.left(), boxSize.width()));
-    anchorRect.expand(margin);
-
-    return anchorRect;
-}
-
 LayoutBoxExtent RenderBox::scrollPaddingForViewportRect(const LayoutRect& viewportRect)
 {

trunk/Source/WebCore/rendering/RenderBox.h

@@ -670 +670 @@
     virtual void adjustBorderBoxRectForPainting(LayoutRect&) { };
 
-    LayoutRect absoluteAnchorRectWithScrollMargin(bool* insideFixed = nullptr) const override;
-
     bool shouldComputeLogicalHeightFromAspectRatio() const;
 

trunk/Source/WebCore/rendering/RenderElement.cpp

@@ -1735 +1735 @@
 LayoutRect RenderElement::absoluteAnchorRectWithScrollMargin(bool* insideFixed) const
 {
-    return absoluteAnchorRect(insideFixed);
+    LayoutRect anchorRect = absoluteAnchorRect(insideFixed);
+    const LengthBox& scrollMargin = style().scrollMargin();
+    if (scrollMargin.isZero())
+        return anchorRect;
+
+    // The scroll snap specification says that the scroll-margin should be applied in the
+    // coordinate system of the scroll container and applied to the rectangular bounding
+    // box of the transformed border box of the target element.
+    // See https://www.w3.org/TR/css-scroll-snap-1/#scroll-margin.
+    const LayoutBoxExtent margin(
+        valueForLength(scrollMargin.top(), anchorRect.height()),
+        valueForLength(scrollMargin.right(), anchorRect.width()),
+        valueForLength(scrollMargin.bottom(), anchorRect.height()),
+        valueForLength(scrollMargin.left(), anchorRect.width()));
+    anchorRect.expand(margin);
+    return anchorRect;
 }
 

trunk/Source/WebCore/rendering/RenderElement.h

@@ -191 +191 @@
     // absoluteAnchorRectWithScrollMargin() is similar to absoluteAnchorRect, but it also takes into account any
     // CSS scroll-margin that is set in the style of this RenderElement.
-    virtual LayoutRect absoluteAnchorRectWithScrollMargin(bool* insideFixed = nullptr) const;
+    LayoutRect absoluteAnchorRectWithScrollMargin(bool* insideFixed = nullptr) const;
 
     bool hasFilter() const { return style().hasFilter(); }