- Revision
- 240921
- Author
- commit-qu...@webkit.org
- Date
- 2019-02-04 07:39:48 -0800 (Mon, 04 Feb 2019)
Log 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 <fw...@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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (240920 => 240921)
--- trunk/LayoutTests/ChangeLog 2019-02-04 15:36:14 UTC (rev 240920)
+++ trunk/LayoutTests/ChangeLog 2019-02-04 15:39:48 UTC (rev 240921)
@@ -1,3 +1,15 @@
+2019-02-04 Frederic Wang <fw...@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 <an...@apple.com>
[iOS] Tiles not created in large scrollable iframes
Added: trunk/LayoutTests/css3/scroll-snap/scroll-snap-children-with-overflow-expected.txt (0 => 240921)
--- trunk/LayoutTests/css3/scroll-snap/scroll-snap-children-with-overflow-expected.txt (rev 0)
+++ trunk/LayoutTests/css3/scroll-snap/scroll-snap-children-with-overflow-expected.txt 2019-02-04 15:39:48 UTC (rev 240921)
@@ -0,0 +1,2 @@
+Scroll-snap offsets are: vertical = { 0, 720, 1440, 2160, 2880, 3600 }
+
Added: trunk/LayoutTests/css3/scroll-snap/scroll-snap-children-with-overflow.html (0 => 240921)
--- trunk/LayoutTests/css3/scroll-snap/scroll-snap-children-with-overflow.html (rev 0)
+++ trunk/LayoutTests/css3/scroll-snap/scroll-snap-children-with-overflow.html 2019-02-04 15:39:48 UTC (rev 240921)
@@ -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>
Modified: trunk/Source/WebCore/ChangeLog (240920 => 240921)
--- trunk/Source/WebCore/ChangeLog 2019-02-04 15:36:14 UTC (rev 240920)
+++ trunk/Source/WebCore/ChangeLog 2019-02-04 15:39:48 UTC (rev 240921)
@@ -1,3 +1,31 @@
+2019-02-04 Frederic Wang <fw...@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 <an...@apple.com>
Rename GraphicsLayer and PlatformCALayer scrolling layer type enum values to be less ambiguous
Modified: trunk/Source/WebCore/page/scrolling/AxisScrollSnapOffsets.cpp (240920 => 240921)
--- trunk/Source/WebCore/page/scrolling/AxisScrollSnapOffsets.cpp 2019-02-04 15:36:14 UTC (rev 240920)
+++ trunk/Source/WebCore/page/scrolling/AxisScrollSnapOffsets.cpp 2019-02-04 15:39:48 UTC (rev 240921)
@@ -234,7 +234,7 @@
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.
Modified: trunk/Source/WebCore/rendering/RenderLayerModelObject.cpp (240920 => 240921)
--- trunk/Source/WebCore/rendering/RenderLayerModelObject.cpp 2019-02-04 15:36:14 UTC (rev 240920)
+++ trunk/Source/WebCore/rendering/RenderLayerModelObject.cpp 2019-02-04 15:39:48 UTC (rev 240921)
@@ -219,7 +219,7 @@
}
}
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) {
Modified: trunk/Source/WebCore/rendering/RenderObject.cpp (240920 => 240921)
--- trunk/Source/WebCore/rendering/RenderObject.cpp 2019-02-04 15:36:14 UTC (rev 240920)
+++ trunk/Source/WebCore/rendering/RenderObject.cpp 2019-02-04 15:39:48 UTC (rev 240921)
@@ -438,6 +438,19 @@
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;
Modified: trunk/Source/WebCore/rendering/RenderObject.h (240920 => 240921)
--- trunk/Source/WebCore/rendering/RenderObject.h 2019-02-04 15:36:14 UTC (rev 240920)
+++ trunk/Source/WebCore/rendering/RenderObject.h 2019-02-04 15:39:48 UTC (rev 240921)
@@ -165,6 +165,7 @@
// 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.
