Changeset 210117 in webkit

Timestamp:

Dec 22, 2016 2:55:31 PM (7 years ago)

Author:

Wenson Hsieh

Message:

CSS Scroll Snap does not work if scrollbar is hidden
​https://bugs.webkit.org/show_bug.cgi?id=160442
<rdar://problem/23317034>

Reviewed by Simon Fraser.

Source/WebCore:

Currently, the only reason scroll snapping works in overflow scrolling containers without forcing layout is
because we would initialize the scrolling container's ScrollAnimator in the process of updating scrollbars. If
there are no scrollbars to render, we won't bother creating a ScrollAnimator. Without an existing
ScrollAnimator, ScrollableArea::updateScrollSnapState will simply bail instead of setting up the scroll snap
state. Instead, we should take setting a non-empty vector of scroll offsets on the ScrollableArea as a cue that
the ScrollableArea also needs a ScrollAnimator, and initialize it there if necessary.

Test: tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-hidden-scrollbars.html

• platform/ScrollableArea.cpp:

(WebCore::ScrollableArea::setHorizontalSnapOffsets):
(WebCore::ScrollableArea::setVerticalSnapOffsets):

LayoutTests:

Adds a new layout test verifying that scroll snapping still works when scrollbars are hidden via CSS.

• tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-hidden-scrollbars-expected.txt: Added.
• tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-hidden-scrollbars.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-hidden-scrollbars-expected.txt (added)
• LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-hidden-scrollbars.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/ScrollableArea.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-12-22  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        CSS Scroll Snap does not work if scrollbar is hidden
+        https://bugs.webkit.org/show_bug.cgi?id=160442
+        <rdar://problem/23317034>
+
+        Reviewed by Simon Fraser.
+
+        Adds a new layout test verifying that scroll snapping still works when scrollbars are hidden via CSS.
+
+        * tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-hidden-scrollbars-expected.txt: Added.
+        * tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-hidden-scrollbars.html: Added.
+
 2016-12-22  Daniel Bates  <dabates@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-12-22  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        CSS Scroll Snap does not work if scrollbar is hidden
+        https://bugs.webkit.org/show_bug.cgi?id=160442
+        <rdar://problem/23317034>
+
+        Reviewed by Simon Fraser.
+
+        Currently, the only reason scroll snapping works in overflow scrolling containers without forcing layout is
+        because we would initialize the scrolling container's ScrollAnimator in the process of updating scrollbars. If
+        there are no scrollbars to render, we won't bother creating a ScrollAnimator. Without an existing
+        ScrollAnimator, ScrollableArea::updateScrollSnapState will simply bail instead of setting up the scroll snap
+        state. Instead, we should take setting a non-empty vector of scroll offsets on the ScrollableArea as a cue that
+        the ScrollableArea also needs a ScrollAnimator, and initialize it there if necessary.
+
+        Test: tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-hidden-scrollbars.html
+
+        * platform/ScrollableArea.cpp:
+        (WebCore::ScrollableArea::setHorizontalSnapOffsets):
+        (WebCore::ScrollableArea::setVerticalSnapOffsets):
+
 2016-12-22  Daniel Bates  <dabates@apple.com>
 

trunk/Source/WebCore/platform/ScrollableArea.cpp

@@ -428 +428 @@
 void ScrollableArea::setHorizontalSnapOffsets(std::unique_ptr<Vector<LayoutUnit>> horizontalSnapOffsets)
 {
+    ASSERT(horizontalSnapOffsets);
+    // Consider having a non-empty set of snap offsets as a cue to initialize the ScrollAnimator.
+    if (horizontalSnapOffsets->size())
+        scrollAnimator();
+
     m_horizontalSnapOffsets = WTFMove(horizontalSnapOffsets);
 }
@@ -433 +438 @@
 void ScrollableArea::setVerticalSnapOffsets(std::unique_ptr<Vector<LayoutUnit>> verticalSnapOffsets)
 {
+    ASSERT(verticalSnapOffsets);
+    // Consider having a non-empty set of snap offsets as a cue to initialize the ScrollAnimator.
+    if (verticalSnapOffsets->size())
+        scrollAnimator();
+
     m_verticalSnapOffsets = WTFMove(verticalSnapOffsets);
 }