Changeset 240921 in webkit

Timestamp:

Feb 4, 2019 7:39:48 AM (5 years ago)

Author:

commit-queue@webkit.org

Message:

[css-scroll-snap] scroll-snap-align not honored on child with non-visible overflow
​https://bugs.webkit.org/show_bug.cgi?id=191816

Patch by Frederic Wang <​fwang@igalia.com> on 2019-02-04
Reviewed by Wenson Hsieh.

Source/WebCore:

This patch fixes a bug that prevents children of a scroll container to create snap positions
when they have non-visible overflow. This happens because for such a child, the function
RenderBox::findEnclosingScrollableContainer() will return the child itself rather than the
scroll container. To address that issue, we introduce a new
RenderObject::enclosingScrollableContainerForSnapping() helper function that ensures that
a real RenderBox ancestor is returned.

Test: css3/scroll-snap/scroll-snap-children-with-overflow.html

• page/scrolling/AxisScrollSnapOffsets.cpp:

(WebCore::updateSnapOffsetsForScrollableArea): Use enclosingScrollableContainerForSnapping()
so that we don't skip children with non-visible overflow.

• rendering/RenderLayerModelObject.cpp:

(WebCore::RenderLayerModelObject::styleDidChange): Ditto. The new function calls
enclosingBox().

• rendering/RenderObject.cpp:

(WebCore::RenderObject::enclosingScrollableContainerForSnapping const): Return
the scrollable container of the enclosing box. If it is actually the render object itself
then start the search from the parent box instead.

• rendering/RenderObject.h: Declare enclosingScrollableContainerForSnapping().

LayoutTests:

Add a test to verify that children with non-visible overflow create snap offsets.

• css3/scroll-snap/scroll-snap-children-with-overflow-expected.txt: Added.
• css3/scroll-snap/scroll-snap-children-with-overflow.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/css3/scroll-snap/scroll-snap-children-with-overflow-expected.txt (added)
• LayoutTests/css3/scroll-snap/scroll-snap-children-with-overflow.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/scrolling/AxisScrollSnapOffsets.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderLayerModelObject.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderObject.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderObject.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-02-04  Frederic Wang  <fwang@igalia.com>
+
+        [css-scroll-snap] scroll-snap-align not honored on child with non-visible overflow
+        https://bugs.webkit.org/show_bug.cgi?id=191816
+
+        Reviewed by Wenson Hsieh.
+
+        Add a test to verify that children with non-visible overflow create snap offsets.
+
+        * css3/scroll-snap/scroll-snap-children-with-overflow-expected.txt: Added.
+        * css3/scroll-snap/scroll-snap-children-with-overflow.html: Added.
+
 2019-02-03  Antti Koivisto  <antti@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-02-04  Frederic Wang  <fwang@igalia.com>
+
+        [css-scroll-snap] scroll-snap-align not honored on child with non-visible overflow
+        https://bugs.webkit.org/show_bug.cgi?id=191816
+
+        Reviewed by Wenson Hsieh.
+
+        This patch fixes a bug that prevents children of a scroll container to create snap positions
+        when they have non-visible overflow. This happens because for such a child, the function
+        RenderBox::findEnclosingScrollableContainer() will return the child itself rather than the
+        scroll container. To address that issue, we introduce a new
+        RenderObject::enclosingScrollableContainerForSnapping() helper function that ensures that
+        a real RenderBox ancestor is returned.
+
+        Test: css3/scroll-snap/scroll-snap-children-with-overflow.html
+
+        * page/scrolling/AxisScrollSnapOffsets.cpp:
+        (WebCore::updateSnapOffsetsForScrollableArea): Use enclosingScrollableContainerForSnapping()
+        so that we don't skip children with non-visible overflow.
+        * rendering/RenderLayerModelObject.cpp:
+        (WebCore::RenderLayerModelObject::styleDidChange): Ditto. The new function calls
+        enclosingBox().
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::enclosingScrollableContainerForSnapping const): Return
+        the scrollable container of the enclosing box. If it is actually the render object itself
+        then start the search from the parent box instead.
+        * rendering/RenderObject.h: Declare enclosingScrollableContainerForSnapping(). 
+
 2019-02-04  Antti Koivisto  <antti@apple.com>
 

trunk/Source/WebCore/page/scrolling/AxisScrollSnapOffsets.cpp

@@ -235 +235 @@
 #endif
     for (auto* child : scrollContainer->view().boxesWithScrollSnapPositions()) {
-        if (child->findEnclosingScrollableContainer() != scrollContainer)
+        if (child->enclosingScrollableContainerForSnapping() != scrollContainer)
             continue;
 

trunk/Source/WebCore/rendering/RenderLayerModelObject.cpp

@@ -220 +220 @@
     }
     if (oldStyle && oldStyle->scrollSnapArea() != newStyle.scrollSnapArea()) {
-        const RenderBox* scrollSnapBox = enclosingBox().findEnclosingScrollableContainer();
+        auto* scrollSnapBox = enclosingScrollableContainerForSnapping();
         if (scrollSnapBox && scrollSnapBox->layer()) {
             const RenderStyle& style = scrollSnapBox->style();

trunk/Source/WebCore/rendering/RenderObject.cpp

@@ -439 +439 @@
 }
 
+const RenderBox* RenderObject::enclosingScrollableContainerForSnapping() const
+{
+    auto& renderBox = enclosingBox();
+    if (auto* scrollableContainer = renderBox.findEnclosingScrollableContainer()) {
+        // The scrollable container for snapping cannot be the node itself.
+        if (scrollableContainer != this)
+            return scrollableContainer;
+        if (renderBox.parentBox())
+            return renderBox.parentBox()->findEnclosingScrollableContainer();
+    }
+    return nullptr;
+}
+
 RenderBlock* RenderObject::firstLineBlock() const
 {

trunk/Source/WebCore/rendering/RenderObject.h

@@ -166 +166 @@
     WEBCORE_EXPORT RenderBox& enclosingBox() const;
     RenderBoxModelObject& enclosingBoxModelObject() const;
+    const RenderBox* enclosingScrollableContainerForSnapping() const;
 
     // Function to return our enclosing flow thread if we are contained inside one. This