CSS scroll snap: defining snap points on axis that does not scroll does not work...
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Jun 2015 18:32:24 +0000 (18:32 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Jun 2015 18:32:24 +0000 (18:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=146043
<rdar://problem/20125511>

Reviewed by Simon Fraser.

Source/WebCore:

Tested by css3/scroll-snap/scroll-snap-mismatch.html

We always seed the set of scroll snap points with the start and end of the scroll container. This is not
the right behavior if there are no scroll points defined, because we end up creating a snap for the start
and end of the container, and any scroll gesture just takes us across the entire element.

Instead, when we do not find any scroll snap points, we should clear the snap point state for the container.

* page/scrolling/AxisScrollSnapOffsets.cpp:
(WebCore::updateFromStyle): If we did not find any snap points (i.e., the snapOffsets container
only holds '0', return an empty Vector.
(WebCore::updateSnapOffsetsForScrollableArea): If the set of snap points produced by 'updateFromStyle' is empty,
clear the horizontal (or vertical) snap offsets for the scroll area.

LayoutTests:

* css3/scroll-snap/scroll-snap-mismatch-expected.txt: Added.
* css3/scroll-snap/scroll-snap-mismatch.html: Added.

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

18 files changed:
LayoutTests/ChangeLog
LayoutTests/css3/scroll-snap/scroll-snap-mismatch-expected.txt [new file with mode: 0644]
LayoutTests/css3/scroll-snap/scroll-snap-mismatch.html [new file with mode: 0644]
Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
Source/ThirdParty/ANGLE/ANGLE.xcodeproj/project.pbxproj
Source/ThirdParty/gtest/xcode/gtest.xcodeproj/project.pbxproj
Source/WTF/WTF.xcodeproj/project.pbxproj
Source/WebCore/ChangeLog
Source/WebCore/WebCore.xcodeproj/project.pbxproj
Source/WebCore/page/scrolling/AxisScrollSnapOffsets.cpp
Source/WebInspectorUI/WebInspectorUI.xcodeproj/project.pbxproj
Source/WebKit/WebKit.xcodeproj/project.pbxproj
Source/WebKit2/WebKit2.xcodeproj/project.pbxproj
Source/bmalloc/bmalloc.xcodeproj/project.pbxproj
Tools/DumpRenderTree/DumpRenderTree.xcodeproj/project.pbxproj
Tools/MiniBrowser/MiniBrowser.xcodeproj/project.pbxproj
Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
Tools/WebKitTestRunner/WebKitTestRunner.xcodeproj/project.pbxproj

index 26910b7..34a3d84 100644 (file)
@@ -1,3 +1,14 @@
+2015-06-17  Brent Fulgham  <bfulgham@apple.com>
+
+        CSS scroll snap: defining snap points on axis that does not scroll does not work properly
+        https://bugs.webkit.org/show_bug.cgi?id=146043
+        <rdar://problem/20125511>
+
+        Reviewed by Simon Fraser.
+
+        * css3/scroll-snap/scroll-snap-mismatch-expected.txt: Added.
+        * css3/scroll-snap/scroll-snap-mismatch.html: Added.
+
 2015-06-17  Alexey Proskuryakov  <ap@apple.com>
 
         New test inspector/console/console-table.html frequently times out in debug.
diff --git a/LayoutTests/css3/scroll-snap/scroll-snap-mismatch-expected.txt b/LayoutTests/css3/scroll-snap/scroll-snap-mismatch-expected.txt
new file mode 100644 (file)
index 0000000..31f340b
--- /dev/null
@@ -0,0 +1,10 @@
+Tests that the scroll-snap feature works properly with mixed-up snap points.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Scroll-snap offsets for 'badHorizontalTarget': 
+Scroll-snap offsets for 'horizontalTarget': horizontal = { 0, 300, 600, 900, 1200, 1500 }
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/css3/scroll-snap/scroll-snap-mismatch.html b/LayoutTests/css3/scroll-snap/scroll-snap-mismatch.html
new file mode 100644 (file)
index 0000000..b999802
--- /dev/null
@@ -0,0 +1,87 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <style>
+            .horizontalGallery {
+                width: 300px;
+                height: 300px;
+                overflow-y: hidden;
+                overflow-x: auto;
+                margin-bottom: 2px;
+                -webkit-scroll-snap-type: mandatory;
+            }
+            .horizontal-good {
+                -webkit-scroll-snap-points-x: repeat(300px);                
+            }
+            .horizontal-bad {
+                -webkit-scroll-snap-points-y: repeat(100vh);
+            }
+            .horizontalGalleryDrawer {
+                width: 1800px;
+                height: 300px;
+            }
+            .colorBox {
+                height: 300px;
+                width: 300px;
+                float: left;
+            }
+            #itemH0, #itemV0 { background-color: red; }
+            #itemH1, #itemV1 { background-color: green; }
+            #itemH2, #itemV2 { background-color: blue; }
+            #itemH3, #itemV3 { background-color: aqua; }
+            #itemH4, #itemV4 { background-color: yellow; }
+            #itemH5, #itemV5 { background-color: fuchsia; }
+        </style>
+        <script src="../../resources/js-test-pre.js"></script>
+        <script>
+        function runTest()
+        {
+            var badHorizontalTarget = document.getElementById('badHorizontalTarget');
+            debug("Scroll-snap offsets for 'badHorizontalTarget': " + window.internals.scrollSnapOffsets(badHorizontalTarget));
+
+            var horizontalTarget = document.getElementById('horizontalTarget');
+            debug("Scroll-snap offsets for 'horizontalTarget': " + window.internals.scrollSnapOffsets(horizontalTarget));
+
+            finishJSTest();
+            testRunner.notifyDone();
+        }
+
+        function onLoad()
+        {
+            if (window.testRunner) {
+                window.jsTestIsAsync = true;
+                testRunner.dumpAsText();
+                testRunner.waitUntilDone();
+                setTimeout(runTest, 0);
+            }
+        }
+        </script>
+    </head>
+    <body onload="onLoad();">
+        <div style="position: relative; width: 300px">
+            <div>Tests that the scroll-snap feature works properly with mixed-up snap points.</div>
+            <div class="horizontalGallery horizontal-bad" id="badHorizontalTarget">
+                <div class="horizontalGalleryDrawer">
+                    <div id="itemH0" class="colorBox"></div>
+                    <div id="itemH1" class="colorBox"></div>
+                    <div id="itemH2" class="colorBox"></div>
+                    <div id="itemH3" class="colorBox"></div>
+                    <div id="itemH4" class="colorBox"></div>
+                    <div id="itemH5" class="colorBox"></div>
+                </div>
+            </div>
+            <div class="horizontalGallery horizontal-good" id="horizontalTarget">
+                <div class="horizontalGalleryDrawer">
+                    <div id="itemH0" class="colorBox"></div>
+                    <div id="itemH1" class="colorBox"></div>
+                    <div id="itemH2" class="colorBox"></div>
+                    <div id="itemH3" class="colorBox"></div>
+                    <div id="itemH4" class="colorBox"></div>
+                    <div id="itemH5" class="colorBox"></div>
+                </div>
+            </div>
+            <div id="console"></div>
+        </div>
+        <script src="../../resources/js-test-post.js"></script>
+    </body>
+</html>
\ No newline at end of file
index f7d2942..291cebc 100644 (file)
                        isa = PBXProject;
                        attributes = {
                                BuildIndependentTargetsInParallel = YES;
+                               LastSwiftUpdateCheck = 0700;
                                LastUpgradeCheck = 0600;
                        };
                        buildConfigurationList = 149C277108902AFE008A9EFC /* Build configuration list for PBXProject "JavaScriptCore" */;
index 7d2431e..dee3c80 100644 (file)
                FB39D0701200ED9200088E69 /* Project object */ = {
                        isa = PBXProject;
                        attributes = {
+                               LastSwiftUpdateCheck = 0700;
                                LastUpgradeCheck = 0600;
                        };
                        buildConfigurationList = FB39D0731200ED9200088E69 /* Build configuration list for PBXProject "ANGLE" */;
index fe3402d..1ff3fba 100644 (file)
                0867D690FE84028FC02AAC07 /* Project object */ = {
                        isa = PBXProject;
                        attributes = {
+                               LastSwiftUpdateCheck = 0700;
                                LastUpgradeCheck = 0600;
                        };
                        buildConfigurationList = 4FADC24608B4156D00ABE55E /* Build configuration list for PBXProject "gtest" */;
index 9d669fa..41bf4be 100644 (file)
                5D247B5914689B8600E78B76 /* Project object */ = {
                        isa = PBXProject;
                        attributes = {
+                               LastSwiftUpdateCheck = 0700;
                                LastUpgradeCheck = 0600;
                        };
                        buildConfigurationList = 5D247B5C14689B8600E78B76 /* Build configuration list for PBXProject "WTF" */;
index 04aa0fc..b7d4bcc 100644 (file)
@@ -1,3 +1,26 @@
+2015-06-17  Brent Fulgham  <bfulgham@apple.com>
+
+        CSS scroll snap: defining snap points on axis that does not scroll does not work properly
+        https://bugs.webkit.org/show_bug.cgi?id=146043
+        <rdar://problem/20125511>
+
+        Reviewed by Simon Fraser.
+
+        Tested by css3/scroll-snap/scroll-snap-mismatch.html
+
+        We always seed the set of scroll snap points with the start and end of the scroll container. This is not
+        the right behavior if there are no scroll points defined, because we end up creating a snap for the start
+        and end of the container, and any scroll gesture just takes us across the entire element.
+        
+        Instead, when we do not find any scroll snap points, we should clear the snap point state for the container.
+
+        * page/scrolling/AxisScrollSnapOffsets.cpp:
+        (WebCore::updateFromStyle): If we did not find any snap points (i.e., the snapOffsets container
+        only holds '0', return an empty Vector. 
+        (WebCore::updateSnapOffsetsForScrollableArea): If the set of snap points produced by 'updateFromStyle' is empty,
+        clear the horizontal (or vertical) snap offsets for the scroll area.
+        
+
 2015-06-17  Chris Fleizach  <cfleizach@apple.com>
 
         AX: input role="spinbutton" gets skipped in voiceover
index 0058786..0208cb5 100644 (file)
                        isa = PBXProject;
                        attributes = {
                                BuildIndependentTargetsInParallel = YES;
+                               LastSwiftUpdateCheck = 0700;
                                LastUpgradeCheck = 0600;
                        };
                        buildConfigurationList = 149C284308902B11008A9EFC /* Build configuration list for PBXProject "WebCore" */;
index 3cbb2b9..ee90ccc 100644 (file)
@@ -85,9 +85,6 @@ static void updateFromStyle(Vector<LayoutUnit>& snapOffsets, const RenderStyle&
     if (snapOffsetSubsequence.isEmpty())
         snapOffsetSubsequence.append(0);
 
-    // Always put a snap point on the zero offset.
-    snapOffsets.append(0);
-
     auto* points = (axis == ScrollEventAxis::Horizontal) ? style.scrollSnapPointsX() : style.scrollSnapPointsY();
     bool hasRepeat = points ? points->hasRepeat : false;
     LayoutUnit repeatOffset = points ? valueForLength(points->repeatOffset, viewSize) : LayoutUnit();
@@ -113,6 +110,13 @@ static void updateFromStyle(Vector<LayoutUnit>& snapOffsets, const RenderStyle&
         curSnapPositionShift = lastSnapPosition + repeatOffset;
     } while (hasRepeat && curSnapPositionShift < maxScrollOffset);
 
+    if (snapOffsets.isEmpty())
+        return;
+
+    // Always put a snap point on the zero offset.
+    if (snapOffsets.first())
+        snapOffsets.insert(0, 0);
+
     // Always put a snap point on the maximum scroll offset.
     // Not a part of the spec, but necessary to prevent unreachable content when snapping.
     if (snapOffsets.last() != maxScrollOffset)
@@ -179,12 +183,18 @@ void updateSnapOffsetsForScrollableArea(ScrollableArea& scrollableArea, HTMLElem
     if (canComputeHorizontalOffsets) {
         auto horizontalSnapOffsets = std::make_unique<Vector<LayoutUnit>>();
         updateFromStyle(*horizontalSnapOffsets, scrollingElementStyle, ScrollEventAxis::Horizontal, viewWidth, scrollWidth, horizontalSnapOffsetSubsequence);
-        scrollableArea.setHorizontalSnapOffsets(WTF::move(horizontalSnapOffsets));
+        if (horizontalSnapOffsets->isEmpty())
+            scrollableArea.clearHorizontalSnapOffsets();
+        else
+            scrollableArea.setHorizontalSnapOffsets(WTF::move(horizontalSnapOffsets));
     }
     if (canComputeVerticalOffsets) {
         auto verticalSnapOffsets = std::make_unique<Vector<LayoutUnit>>();
         updateFromStyle(*verticalSnapOffsets, scrollingElementStyle, ScrollEventAxis::Vertical, viewHeight, scrollHeight, verticalSnapOffsetSubsequence);
-        scrollableArea.setVerticalSnapOffsets(WTF::move(verticalSnapOffsets));
+        if (verticalSnapOffsets->isEmpty())
+            scrollableArea.clearVerticalSnapOffsets();
+        else
+            scrollableArea.setVerticalSnapOffsets(WTF::move(verticalSnapOffsets));
     }
 }
 
index 7aea39c..0d60d16 100644 (file)
                A54C224D148B23DE00373FA3 /* Project object */ = {
                        isa = PBXProject;
                        attributes = {
+                               LastSwiftUpdateCheck = 0700;
                                LastUpgradeCheck = 0600;
                                ORGANIZATIONNAME = Apple;
                        };
index 0867675..7c303dd 100644 (file)
                0867D690FE84028FC02AAC07 /* Project object */ = {
                        isa = PBXProject;
                        attributes = {
+                               LastSwiftUpdateCheck = 0700;
                                LastUpgradeCheck = 0600;
                        };
                        buildConfigurationList = 149C283208902B0F008A9EFC /* Build configuration list for PBXProject "WebKit" */;
index 0eac721..ccdebf8 100644 (file)
                0867D690FE84028FC02AAC07 /* Project object */ = {
                        isa = PBXProject;
                        attributes = {
+                               LastSwiftUpdateCheck = 0700;
                                LastUpgradeCheck = 0600;
                        };
                        buildConfigurationList = 1DEB91B108733DA50010E9CD /* Build configuration list for PBXProject "WebKit2" */;
index 337ec68..fa82904 100644 (file)
                145F6837179DC45F00D65598 /* Project object */ = {
                        isa = PBXProject;
                        attributes = {
+                               LastSwiftUpdateCheck = 0700;
                                LastUpgradeCheck = 0600;
                        };
                        buildConfigurationList = 145F683A179DC45F00D65598 /* Build configuration list for PBXProject "bmalloc" */;
index 5682fdc..5b02d5c 100644 (file)
                08FB7793FE84155DC02AAC07 /* Project object */ = {
                        isa = PBXProject;
                        attributes = {
+                               LastSwiftUpdateCheck = 0700;
                        };
                        buildConfigurationList = 149C29C308902C6D008A9EFC /* Build configuration list for PBXProject "DumpRenderTree" */;
                        compatibilityVersion = "Xcode 3.2";
index 5d12c1e..5267010 100644 (file)
                29B97313FDCFA39411CA2CEA /* Project object */ = {
                        isa = PBXProject;
                        attributes = {
+                               LastSwiftUpdateCheck = 0700;
                                LastUpgradeCheck = 0600;
                                TargetAttributes = {
                                        8D1107260486CEB800E47090 = {
index 728ec4c..0e95139 100644 (file)
                08FB7793FE84155DC02AAC07 /* Project object */ = {
                        isa = PBXProject;
                        attributes = {
+                               LastSwiftUpdateCheck = 0700;
                                TargetAttributes = {
                                        7CCE7E8B1A41144E00447C4C = {
                                                CreatedOnToolsVersion = 6.3;
index caa116b..c789572 100644 (file)
                08FB7793FE84155DC02AAC07 /* Project object */ = {
                        isa = PBXProject;
                        attributes = {
+                               LastSwiftUpdateCheck = 0700;
                        };
                        buildConfigurationList = 1DEB927808733DD40010E9CD /* Build configuration list for PBXProject "WebKitTestRunner" */;
                        compatibilityVersion = "Xcode 3.1";