[css-scroll-snap] scroll-snap-align not honored on child with non-visible overflow
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 4 Feb 2019 15:39:48 +0000 (15:39 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 4 Feb 2019 15:39:48 +0000 (15:39 +0000)
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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240921 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/css3/scroll-snap/scroll-snap-children-with-overflow-expected.txt [new file with mode: 0644]
LayoutTests/css3/scroll-snap/scroll-snap-children-with-overflow.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/scrolling/AxisScrollSnapOffsets.cpp
Source/WebCore/rendering/RenderLayerModelObject.cpp
Source/WebCore/rendering/RenderObject.cpp
Source/WebCore/rendering/RenderObject.h

index 56c7ee0..c48cbd5 100644 (file)
@@ -1,3 +1,15 @@
+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>
 
         [iOS] Tiles not created in large scrollable iframes
diff --git a/LayoutTests/css3/scroll-snap/scroll-snap-children-with-overflow-expected.txt b/LayoutTests/css3/scroll-snap/scroll-snap-children-with-overflow-expected.txt
new file mode 100644 (file)
index 0000000..7fcb5c0
--- /dev/null
@@ -0,0 +1,2 @@
+Scroll-snap offsets are: vertical = { 0, 720, 1440, 2160, 2880, 3600 }
+
diff --git a/LayoutTests/css3/scroll-snap/scroll-snap-children-with-overflow.html b/LayoutTests/css3/scroll-snap/scroll-snap-children-with-overflow.html
new file mode 100644 (file)
index 0000000..7af0079
--- /dev/null
@@ -0,0 +1,61 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        #container {
+            position: absolute;
+            width: 400px;
+            height: 400px;
+            top: 0;
+            left: 0;
+            scroll-snap-type: y mandatory;
+            overflow-x: hidden;
+            overflow-y: scroll;
+            -webkit-overflow-scrolling: touch;
+            border: 1px black dashed;
+            opacity: 0.75;
+            background-color: #EEE;
+        }
+
+        .child {
+            width: 400px;
+            height: 400px;
+            scroll-snap-align: end;
+            position: absolute;
+            left: 0;
+        }
+    </style>
+    <script>
+    let write = s => output.innerHTML += s + "<br>";
+    if (window.testRunner) {
+        testRunner.dumpAsText();
+        testRunner.waitUntilDone();
+    }
+
+    function run()
+    {
+        if (!window.testRunner || !window.internals) {
+            write(`To manually test, verify that scrolling in the container snaps such that the bottom tip of each
+                colored box aligns with the bottom of the scrolling container.`);
+            return;
+        }
+
+         setTimeout(() => {
+            write("Scroll-snap offsets are: " + internals.scrollSnapOffsets(container));
+            testRunner.notifyDone();
+        }, 0);
+    }
+    </script>
+</head>
+<body onload=run()>
+    <div id="container">
+        <div class="child" style="background-color: red;     top: 0px; overflow: visible;"></div>
+        <div class="child" style="background-color: green;   top: 720px;  overflow: hidden;"></div>
+        <div class="child" style="background-color: blue;    top: 1440px; overflow: scroll;"></div>
+        <div class="child" style="background-color: aqua;    top: 2160px; overflow: auto;"></div>
+        <div class="child" style="background-color: yellow;  top: 2880px; overflow: overlay;"></div>
+        <div class="child" style="background-color: fuchsia; top: 3600px;"></div>
+    </div>
+    <div id="output"></div>
+</body>
+</html>
index c800a64..fe9d8a2 100644 (file)
@@ -1,3 +1,31 @@
+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>
 
         Rename GraphicsLayer and PlatformCALayer scrolling layer type enum values to be less ambiguous
index 60379b5..5933287 100644 (file)
@@ -234,7 +234,7 @@ void updateSnapOffsetsForScrollableArea(ScrollableArea& scrollableArea, HTMLElem
     LOG(Scrolling, "Computing scroll snap offsets in snap port: %s", snapPortOrAreaToString(scrollSnapPort).utf8().data());
 #endif
     for (auto* child : scrollContainer->view().boxesWithScrollSnapPositions()) {
-        if (child->findEnclosingScrollableContainer() != scrollContainer)
+        if (child->enclosingScrollableContainerForSnapping() != scrollContainer)
             continue;
 
         // The bounds of the child element's snap area, where the top left of the scrolling container's border box is the origin.
index 06fccde..0ce0bb4 100644 (file)
@@ -219,7 +219,7 @@ void RenderLayerModelObject::styleDidChange(StyleDifference diff, const RenderSt
         }
     }
     if (oldStyle && oldStyle->scrollSnapArea() != newStyle.scrollSnapArea()) {
-        const RenderBox* scrollSnapBox = enclosingBox().findEnclosingScrollableContainer();
+        auto* scrollSnapBox = enclosingScrollableContainerForSnapping();
         if (scrollSnapBox && scrollSnapBox->layer()) {
             const RenderStyle& style = scrollSnapBox->style();
             if (style.scrollSnapType().strictness != ScrollSnapStrictness::None) {
index 676017c..51cc458 100644 (file)
@@ -438,6 +438,19 @@ RenderBoxModelObject& RenderObject::enclosingBoxModelObject() const
     return *lineageOfType<RenderBoxModelObject>(const_cast<RenderObject&>(*this)).first();
 }
 
+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
 {
     return nullptr;
index 97cc0fa..d08c1db 100644 (file)
@@ -165,6 +165,7 @@ public:
     // Convenience function for getting to the nearest enclosing box of a RenderObject.
     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
     // function follows the containing block chain.