Title: [186865] trunk
Revision
186865
Author
[email protected]
Date
2015-07-15 15:33:44 -0700 (Wed, 15 Jul 2015)

Log Message

Scroll snapping to elements is broken for main frame scrolling
https://bugs.webkit.org/show_bug.cgi?id=146957

Patch by Wenson Hsieh <[email protected]> on 2015-07-15
Reviewed by Brent Fulgham.

Source/WebCore:

Fixes the case of elements with scroll snap coordinates in a scroll snapping mainframe by changing
RenderBox::findEnclosingScrollableContainer to return the body's RenderBox when all enclosing elements
are not overflow scrollable but the mainframe can scroll.

Test: css3/scroll-snap/scroll-snap-coordinate-mainframe.html

* page/FrameView.h: Export isScrollable so that the Internals API can use it.
* rendering/RenderBox.cpp: Include MainFrame.h.
(WebCore::RenderBox::findEnclosingScrollableContainer): Changed to return the body's RenderBox if
    none of its parent elements are overflow scrolling.
* testing/Internals.cpp:
(WebCore::Internals::scrollSnapOffsets): Updated to return snap offsets for the body element,
    allowing us to call window.internals.scrollSnapOffsets(document.body).

LayoutTests:

Tests that basic scroll snap coordinates in the mainframe works.

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

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (186864 => 186865)


--- trunk/LayoutTests/ChangeLog	2015-07-15 22:30:00 UTC (rev 186864)
+++ trunk/LayoutTests/ChangeLog	2015-07-15 22:33:44 UTC (rev 186865)
@@ -1,3 +1,15 @@
+2015-07-15  Wenson Hsieh  <[email protected]>
+
+        Scroll snapping to elements is broken for main frame scrolling
+        https://bugs.webkit.org/show_bug.cgi?id=146957
+
+        Reviewed by Brent Fulgham.
+
+        Tests that basic scroll snap coordinates in the mainframe works.
+
+        * css3/scroll-snap/scroll-snap-coordinate-mainframe-expected.txt: Added.
+        * css3/scroll-snap/scroll-snap-coordinate-mainframe.html: Added.
+
 2015-07-15  Saam barati  <[email protected]>
 
         [ES6] implement block scoping to enable 'let'

Added: trunk/LayoutTests/css3/scroll-snap/scroll-snap-coordinate-mainframe-expected.txt (0 => 186865)


--- trunk/LayoutTests/css3/scroll-snap/scroll-snap-coordinate-mainframe-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/css3/scroll-snap/scroll-snap-coordinate-mainframe-expected.txt	2015-07-15 22:33:44 UTC (rev 186865)
@@ -0,0 +1,8 @@
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Scroll-snap offsets: vertical = { 0, 600, 1200, 1800, 2400, 3000 }
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/css3/scroll-snap/scroll-snap-coordinate-mainframe.html (0 => 186865)


--- trunk/LayoutTests/css3/scroll-snap/scroll-snap-coordinate-mainframe.html	                        (rev 0)
+++ trunk/LayoutTests/css3/scroll-snap/scroll-snap-coordinate-mainframe.html	2015-07-15 22:33:44 UTC (rev 186865)
@@ -0,0 +1,47 @@
+<html>
+
+<head>
+    <style>
+        body {
+            margin: 0;
+            -webkit-scroll-snap-type: mandatory;
+        }
+
+        .vertical {
+            width: 100%;
+            height: 600px;
+            -webkit-scroll-snap-coordinate: 0px 0px;
+        }
+    </style>
+
+    <script src=""
+    <script>
+    function runTest()
+    {
+        debug("Scroll-snap offsets: " + window.internals.scrollSnapOffsets(document.body));
+        finishJSTest();
+    }
+
+    function setup()
+    {
+        if (window.testRunner) {
+            window.jsTestIsAsync = true;
+            testRunner.dumpAsText();
+            testRunner.waitUntilDone();
+            setTimeout(runTest, 0);
+        }
+    }
+    </script>
+</head>
+
+<body _onload_="setup()">
+    <div id="child00" class="vertical"><div id="console"></div></div>
+    <div id="child01" class="vertical"></div>
+    <div id="child02" class="vertical"></div>
+    <div id="child03" class="vertical"></div>
+    <div id="child04" class="vertical"></div>
+    <div id="child05" class="vertical"></div>
+    <script src=""
+</body>
+
+</html>

Modified: trunk/Source/WebCore/ChangeLog (186864 => 186865)


--- trunk/Source/WebCore/ChangeLog	2015-07-15 22:30:00 UTC (rev 186864)
+++ trunk/Source/WebCore/ChangeLog	2015-07-15 22:33:44 UTC (rev 186865)
@@ -1,3 +1,24 @@
+2015-07-15  Wenson Hsieh  <[email protected]>
+
+        Scroll snapping to elements is broken for main frame scrolling
+        https://bugs.webkit.org/show_bug.cgi?id=146957
+
+        Reviewed by Brent Fulgham.
+
+        Fixes the case of elements with scroll snap coordinates in a scroll snapping mainframe by changing
+        RenderBox::findEnclosingScrollableContainer to return the body's RenderBox when all enclosing elements
+        are not overflow scrollable but the mainframe can scroll.
+
+        Test: css3/scroll-snap/scroll-snap-coordinate-mainframe.html
+
+        * page/FrameView.h: Export isScrollable so that the Internals API can use it.
+        * rendering/RenderBox.cpp: Include MainFrame.h.
+        (WebCore::RenderBox::findEnclosingScrollableContainer): Changed to return the body's RenderBox if
+            none of its parent elements are overflow scrolling.
+        * testing/Internals.cpp:
+        (WebCore::Internals::scrollSnapOffsets): Updated to return snap offsets for the body element,
+            allowing us to call window.internals.scrollSnapOffsets(document.body).
+
 2015-07-15  Brady Eidson  <[email protected]>
 
         WebKit document.cookie mis-parsing.

Modified: trunk/Source/WebCore/page/FrameView.h (186864 => 186865)


--- trunk/Source/WebCore/page/FrameView.h	2015-07-15 22:30:00 UTC (rev 186864)
+++ trunk/Source/WebCore/page/FrameView.h	2015-07-15 22:33:44 UTC (rev 186865)
@@ -429,7 +429,7 @@
     // to rubber-band, which the main frame might be allowed to do even if there is no content to scroll to. In that case,
     // callers use Scrollability::ScrollableOrRubberbandable.
     enum class Scrollability { Scrollable, ScrollableOrRubberbandable };
-    bool isScrollable(Scrollability definitionOfScrollable = Scrollability::Scrollable);
+    WEBCORE_EXPORT bool isScrollable(Scrollability definitionOfScrollable = Scrollability::Scrollable);
 
     virtual bool isScrollableOrRubberbandable() override;
     virtual bool hasScrollableOrRubberbandableAncestor() override;

Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (186864 => 186865)


--- trunk/Source/WebCore/rendering/RenderBox.cpp	2015-07-15 22:30:00 UTC (rev 186864)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp	2015-07-15 22:33:44 UTC (rev 186865)
@@ -43,6 +43,7 @@
 #include "HTMLTextAreaElement.h"
 #include "HitTestResult.h"
 #include "InlineElementBox.h"
+#include "MainFrame.h"
 #include "Page.h"
 #include "PaintInfo.h"
 #include "RenderBoxRegionInfo.h"
@@ -5003,6 +5004,9 @@
         if (candidate.hasOverflowClip())
             return &candidate;
     }
+    // If all parent elements are not overflow scrollable, check the body.
+    if (document().body() && frame().mainFrame().view() && frame().mainFrame().view()->isScrollable())
+        return document().body()->renderBox();
     
     return nullptr;
 }

Modified: trunk/Source/WebCore/testing/Internals.cpp (186864 => 186865)


--- trunk/Source/WebCore/testing/Internals.cpp	2015-07-15 22:30:00 UTC (rev 186864)
+++ trunk/Source/WebCore/testing/Internals.cpp	2015-07-15 22:33:44 UTC (rev 186865)
@@ -2887,29 +2887,40 @@
         return String();
 
     RenderBox& box = *element->renderBox();
-    if (!box.canBeScrolledAndHasScrollableArea()) {
-        ec = INVALID_ACCESS_ERR;
-        return String();
+    ScrollableArea* scrollableArea;
+    
+    if (box.isBody()) {
+        FrameView* frameView = box.frame().mainFrame().view();
+        if (!frameView || !frameView->isScrollable()) {
+            ec = INVALID_ACCESS_ERR;
+            return String();
+        }
+        scrollableArea = frameView;
+        
+    } else {
+        if (!box.canBeScrolledAndHasScrollableArea()) {
+            ec = INVALID_ACCESS_ERR;
+            return String();
+        }
+        scrollableArea = box.layer();
     }
 
-    if (!box.layer())
+    if (!scrollableArea)
         return String();
     
-    ScrollableArea& scrollableArea = *box.layer();
-    
     StringBuilder result;
 
-    if (scrollableArea.horizontalSnapOffsets()) {
+    if (scrollableArea->horizontalSnapOffsets()) {
         result.append("horizontal = ");
-        appendOffsets(result, *scrollableArea.horizontalSnapOffsets());
+        appendOffsets(result, *scrollableArea->horizontalSnapOffsets());
     }
 
-    if (scrollableArea.verticalSnapOffsets()) {
+    if (scrollableArea->verticalSnapOffsets()) {
         if (result.length())
             result.append(", ");
 
         result.append("vertical = ");
-        appendOffsets(result, *scrollableArea.verticalSnapOffsets());
+        appendOffsets(result, *scrollableArea->verticalSnapOffsets());
     }
 
     return result.toString();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to