Changeset 271788 in webkit


Ignore:
Timestamp:
Jan 25, 2021 1:45:47 AM (18 months ago)
Author:
commit-queue@webkit.org
Message:

scroll-padding should affect paging operations
​https://bugs.webkit.org/show_bug.cgi?id=219074
<rdar://problem/71747786>

Patch by Martin Robinson <​mrobinson@igalia.com> on 2021-01-25
Reviewed by Simon Fraser.

Source/WebCore:

Have scroll-padding affect the amount of the scrollable area that moves during
paging operations. This is the behavior specified in the scroll snap specification
and allows scrollable areas with partially obscured areas to properly page through
their content.

See ​https://drafts.csswg.org/css-scroll-snap-1/#propdef-scroll-padding.

Tests: css3/scroll-snap/scroll-padding-mainframe-paging.html

css3/scroll-snap/scroll-padding-overflow-paging.html

  • page/FrameView.cpp:

(WebCore::FrameView::updateScrollbarSteps): Added this override method which
properly sets page steps. Only FrameView has access to the document.

  • page/FrameView.h:
  • platform/ScrollView.cpp:

(WebCore::ScrollView::updateScrollbars): Added this helper method, which is
virtual so that FrameView can override it.
(WebCore::ScrollView::updateScrollbarSteps): Use the helper method to actually
set the scrollbar steps.

  • platform/ScrollView.h: Add new method declarations.
  • rendering/RenderBox.cpp:

(WebCore::RenderBox::scrollPaddingForViewportRect): Add this new helper which
gets the scroll-padding for a box.

  • rendering/RenderBox.h: Add new helper.
  • rendering/RenderLayer.cpp:

(WebCore::expandScrollRectToVisibleTargetRectToIncludeScrollPadding): Use the new
RenderBox helper.
(WebCore::RenderLayer::updateScrollbarsAfterLayout): Use the new updateScrollbarSteps helper.
(WebCore::RenderLayer::updateScrollbarSteps): Added this helper so that RenderLayerModelObject
can update steps when necessary.

  • rendering/RenderLayer.h: Added new declarations.
  • rendering/RenderLayerModelObject.cpp:

(WebCore::RenderLayerModelObject::styleDidChange): Update steps on FrameViews and RenderLayers
when the style change dictates it.

LayoutTests:

  • css3/scroll-snap/scroll-padding-mainframe-paging-expected.txt: Added.
  • css3/scroll-snap/scroll-padding-mainframe-paging.html: Added.
  • css3/scroll-snap/scroll-padding-overflow-paging-expected.txt: Added.
  • css3/scroll-snap/scroll-padding-overflow-paging.html: Added.
  • platform/ios-wk2/TestExpectations: Skip failing tests.
  • platform/mac-wk1/TestExpectations: Skip failing tests.
Location:
trunk
Files:
4 added
15 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r271787 r271788  
     12021-01-25  Martin Robinson  <mrobinson@igalia.com>
     2
     3        scroll-padding should affect paging operations
     4        https://bugs.webkit.org/show_bug.cgi?id=219074
     5        <rdar://problem/71747786>
     6
     7        Reviewed by Simon Fraser.
     8
     9        * css3/scroll-snap/scroll-padding-mainframe-paging-expected.txt: Added.
     10        * css3/scroll-snap/scroll-padding-mainframe-paging.html: Added.
     11        * css3/scroll-snap/scroll-padding-overflow-paging-expected.txt: Added.
     12        * css3/scroll-snap/scroll-padding-overflow-paging.html: Added.
     13        * platform/ios-wk2/TestExpectations: Skip failing tests.
     14        * platform/mac-wk1/TestExpectations: Skip failing tests.
     15
    1162021-01-25  Carlos Garcia Campos  <cgarcia@igalia.com>
    217

  • trunk/LayoutTests/platform/ios-wk2/TestExpectations

    r271320 r271788  
    135135scrollbars/scrolling-by-page-on-keyboard-spacebar.html [ Failure ]
    136136scrollbars/scrollbars-on-positioned-content.html [ Failure ]
     137css3/scroll-snap/scroll-padding-mainframe-paging.html [ Failure ]
     138css3/scroll-snap/scroll-padding-overflow-paging.html [ Failure ]
    137139
    138140# SVG tests that time out (these require EventSender)

  • trunk/LayoutTests/platform/mac-wk1/TestExpectations

    r271461 r271788  
    449449imported/w3c/web-platform-tests/cookies/secure/set-from-ws.sub.html [ Failure ]
    450450
    451 # WebKit1 frames use native scrollbars causing this reference test to fail.
     451# WebKit1 frames use native scrollbars causing these tests to fail.
    452452imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-target-padding-001.html [ ImageOnlyFailure ]
     453css3/scroll-snap/scroll-padding-mainframe-paging.html [ Failure ]
    453454
    454455### END OF (2) Failures without bug reports

  • trunk/Source/WebCore/ChangeLog

    r271787 r271788  
     12021-01-25  Martin Robinson  <mrobinson@igalia.com>
     2
     3        scroll-padding should affect paging operations
     4        https://bugs.webkit.org/show_bug.cgi?id=219074
     5        <rdar://problem/71747786>
     6
     7        Reviewed by Simon Fraser.
     8
     9        Have scroll-padding affect the amount of the scrollable area that moves during
     10        paging operations. This is the behavior specified in the scroll snap specification
     11        and allows scrollable areas with partially obscured areas to properly page through
     12        their content.
     13
     14        See https://drafts.csswg.org/css-scroll-snap-1/#propdef-scroll-padding.
     15
     16        Tests: css3/scroll-snap/scroll-padding-mainframe-paging.html
     17               css3/scroll-snap/scroll-padding-overflow-paging.html
     18
     19        * page/FrameView.cpp:
     20        (WebCore::FrameView::updateScrollbarSteps): Added this override method which
     21        properly sets page steps. Only FrameView has access to the document.
     22        * page/FrameView.h:
     23        * platform/ScrollView.cpp:
     24        (WebCore::ScrollView::updateScrollbars): Added this helper method, which is
     25        virtual so that FrameView can override it.
     26        (WebCore::ScrollView::updateScrollbarSteps): Use the helper method to actually
     27        set the scrollbar steps.
     28        * platform/ScrollView.h: Add new method declarations.
     29        * rendering/RenderBox.cpp:
     30        (WebCore::RenderBox::scrollPaddingForViewportRect): Add this new helper which
     31        gets the scroll-padding for a box.
     32        * rendering/RenderBox.h: Add new helper.
     33        * rendering/RenderLayer.cpp:
     34        (WebCore::expandScrollRectToVisibleTargetRectToIncludeScrollPadding): Use the new
     35        RenderBox helper.
     36        (WebCore::RenderLayer::updateScrollbarsAfterLayout): Use the new updateScrollbarSteps helper.
     37        (WebCore::RenderLayer::updateScrollbarSteps): Added this helper so that RenderLayerModelObject
     38        can update steps when necessary.
     39        * rendering/RenderLayer.h: Added new declarations.
     40        * rendering/RenderLayerModelObject.cpp:
     41        (WebCore::RenderLayerModelObject::styleDidChange): Update steps on FrameViews and RenderLayers
     42        when the style change dictates it.
     43
    1442021-01-25  Carlos Garcia Campos  <cgarcia@igalia.com>
    245

  • trunk/Source/WebCore/page/FrameView.cpp

    r271786 r271788  
    55455545}
    55465546
     5547void FrameView::updateScrollbarSteps()
     5548{
     5549    auto* documentElement = frame().document() ? frame().document()->documentElement() : nullptr;
     5550    auto* renderer = documentElement ? documentElement->renderBox() : nullptr;
     5551    if (!renderer) {
     5552        ScrollView::updateScrollbarSteps();
     5553        return;
     5554    }
     5555
     5556    LayoutRect paddedViewRect(LayoutPoint(), visibleSize());
     5557    paddedViewRect.contract(renderer->scrollPaddingForViewportRect(paddedViewRect));
     5558
     5559    if (horizontalScrollbar()) {
     5560        int pageStep = Scrollbar::pageStep(paddedViewRect.width());
     5561        horizontalScrollbar()->setSteps(Scrollbar::pixelsPerLineStep(), pageStep);
     5562
     5563    }
     5564    if (verticalScrollbar()) {
     5565        int pageStep = Scrollbar::pageStep(paddedViewRect.height());
     5566        verticalScrollbar()->setSteps(Scrollbar::pixelsPerLineStep(), pageStep);
     5567    }
     5568}
    55475569} // namespace WebCore
    55485570

  • trunk/Source/WebCore/page/FrameView.h

    r271786 r271788  
    681681    String debugDescription() const final;
    682682
     683    // ScrollView
     684    void updateScrollbarSteps() override;
     685
    683686private:
    684687    explicit FrameView(Frame&);

  • trunk/Source/WebCore/platform/ScrollView.cpp

    r271786 r271788  
    732732    if (m_horizontalScrollbar) {
    733733        int clientWidth = visibleWidth();
    734         int pageStep = Scrollbar::pageStep(clientWidth);
    735734        IntRect oldRect(m_horizontalScrollbar->frameRect());
    736735        IntRect hBarRect(shouldPlaceBlockDirectionScrollbarOnLeft() && m_verticalScrollbar ? m_verticalScrollbar->occupiedWidth() : 0,
    … …  
    745744            m_horizontalScrollbar->setSuppressInvalidation(true);
    746745        m_horizontalScrollbar->setEnabled(contentsWidth() > clientWidth);
    747         m_horizontalScrollbar->setSteps(Scrollbar::pixelsPerLineStep(), pageStep);
    748746        m_horizontalScrollbar->setProportion(clientWidth, contentsWidth());
    749747        if (m_scrollbarsSuppressed)
    … …  
    753751    if (m_verticalScrollbar) {
    754752        int clientHeight = visibleHeight();
    755         int pageStep = Scrollbar::pageStep(clientHeight);
    756753        IntRect oldRect(m_verticalScrollbar->frameRect());
    757754        IntRect vBarRect(shouldPlaceBlockDirectionScrollbarOnLeft() ? 0 : width() - m_verticalScrollbar->width(),
    … …  
    766763            m_verticalScrollbar->setSuppressInvalidation(true);
    767764        m_verticalScrollbar->setEnabled(totalContentsSize().height() > clientHeight);
    768         m_verticalScrollbar->setSteps(Scrollbar::pixelsPerLineStep(), pageStep);
    769765        m_verticalScrollbar->setProportion(clientHeight, totalContentsSize().height());
    770766        if (m_scrollbarsSuppressed)
    771767            m_verticalScrollbar->setSuppressInvalidation(false);
    772768    }
     769
     770    updateScrollbarSteps();
    773771
    774772    if (hasHorizontalScrollbar != newHasHorizontalScrollbar || hasVerticalScrollbar != newHasVerticalScrollbar) {
    … …  
    790788
    791789    m_inUpdateScrollbars = false;
     790}
     791
     792void ScrollView::updateScrollbarSteps()
     793{
     794    if (m_horizontalScrollbar)
     795        m_horizontalScrollbar->setSteps(Scrollbar::pixelsPerLineStep(), Scrollbar::pageStep(visibleWidth()));
     796    if (m_verticalScrollbar)
     797        m_verticalScrollbar->setSteps(Scrollbar::pixelsPerLineStep(), Scrollbar::pageStep(visibleHeight()));
    792798}
    793799

  • trunk/Source/WebCore/platform/ScrollView.h

    r271439 r271788  
    398398
    399399    bool managesScrollbars() const;
     400    virtual void updateScrollbarSteps();
    400401
    401402protected:

  • trunk/Source/WebCore/rendering/RenderBox.cpp

    r271740 r271788  
    51635163}
    51645164
     5165LayoutBoxExtent RenderBox::scrollPaddingForViewportRect(const LayoutRect& viewportRect)
     5166{
     5167    const auto& padding = style().scrollPadding();
     5168    return LayoutBoxExtent(
     5169        valueForLength(padding.top(), viewportRect.height()), valueForLength(padding.right(), viewportRect.width()),
     5170        valueForLength(padding.bottom(), viewportRect.height()), valueForLength(padding.left(), viewportRect.width()));
     5171}
     5172
    51655173} // namespace WebCore

  • trunk/Source/WebCore/rendering/RenderBox.h

    r271559 r271788  
    496496    bool hasScrollableOverflowY() const { return scrollsOverflowY() && hasVerticalOverflow(); }
    497497
     498    LayoutBoxExtent scrollPaddingForViewportRect(const LayoutRect& viewportRect);
     499
    498500    bool usesCompositedScrolling() const;
    499501   

  • trunk/Source/WebCore/rendering/RenderLayer.cpp

    r271740 r271788  
    651651    // simulate padding the scroll container. This rectangle is passed up the tree of scrolling elements to
    652652    // ensure that the padding on this scroll container is maintained.
    653     const auto& scrollPadding = renderBox->style().scrollPadding();
    654     LayoutBoxExtent scrollPaddingExtents(
    655         valueForLength(scrollPadding.top(), viewRect.height()), valueForLength(scrollPadding.right(), viewRect.width()),
    656         valueForLength(scrollPadding.bottom(), viewRect.height()), valueForLength(scrollPadding.left(), viewRect.width()));
    657     targetRect.expand(scrollPaddingExtents);
     653    targetRect.expand(renderBox->scrollPaddingForViewportRect(viewRect));
    658654}
    659655
    … …  
    28862882    if (m_scrollableArea)
    28872883        m_scrollableArea->updateScrollInfoAfterLayout();
     2884}
     2885
     2886void RenderLayer::updateScrollbarSteps()
     2887{
     2888    if (m_scrollableArea)
     2889        m_scrollableArea->updateScrollbarSteps();
    28882890}
    28892891

  • trunk/Source/WebCore/rendering/RenderLayer.h

    r271740 r271788  
    478478
    479479    void updateScrollInfoAfterLayout();
     480    void updateScrollbarSteps();
    480481
    481482    void autoscroll(const IntPoint&);

  • trunk/Source/WebCore/rendering/RenderLayerModelObject.cpp

    r271559 r271788  
    173173    }
    174174
     175    const RenderStyle& newStyle = style();
     176    if (oldStyle && oldStyle->scrollPadding() != newStyle.scrollPadding()) {
     177        if (isDocumentElementRenderer()) {
     178            FrameView& frameView = view().frameView();
     179            frameView.updateScrollbarSteps();
     180        } else if (RenderLayer* renderLayer = layer())
     181            renderLayer->updateScrollbarSteps();
     182    }
     183
    175184#if ENABLE(CSS_SCROLL_SNAP)
    176     const RenderStyle& newStyle = style();
    177185    if (oldStyle && scrollSnapContainerRequiresUpdateForStyleUpdate(*oldStyle, newStyle)) {
    178186        if (RenderLayer* renderLayer = layer()) {

  • trunk/Source/WebCore/rendering/RenderLayerScrollableArea.cpp

    r271701 r271788  
    11191119    }
    11201120
    1121     // Set up the range (and page step/line step).
     1121    // Set up the range.
     1122    if (m_hBar)
     1123        m_hBar->setProportion(roundToInt(box->clientWidth()), m_scrollWidth);
     1124    if (m_vBar)
     1125        m_vBar->setProportion(roundToInt(box->clientHeight()), m_scrollHeight);
     1126
     1127    updateScrollbarSteps();
     1128
     1129    updateScrollableAreaSet(hasScrollableHorizontalOverflow() || hasScrollableVerticalOverflow());
     1130}
     1131
     1132void RenderLayerScrollableArea::updateScrollbarSteps()
     1133{
     1134    RenderBox* box = m_layer.renderBox();
     1135    ASSERT(box);
     1136
     1137    LayoutRect paddedLayerBounds(0_lu, 0_lu, box->clientWidth(), box->clientHeight());
     1138    paddedLayerBounds.contract(box->scrollPaddingForViewportRect(paddedLayerBounds));
     1139
     1140    // Set up the  page step/line step.
    11221141    if (m_hBar) {
    1123         int clientWidth = roundToInt(box->clientWidth());
    1124         int pageStep = Scrollbar::pageStep(clientWidth);
     1142        int pageStep = Scrollbar::pageStep(roundToInt(paddedLayerBounds.width()));
    11251143        m_hBar->setSteps(Scrollbar::pixelsPerLineStep(), pageStep);
    1126         m_hBar->setProportion(clientWidth, m_scrollWidth);
    11271144    }
    11281145    if (m_vBar) {
    1129         int clientHeight = roundToInt(box->clientHeight());
    1130         int pageStep = Scrollbar::pageStep(clientHeight);
     1146        int pageStep = Scrollbar::pageStep(roundToInt(paddedLayerBounds.height()));
    11311147        m_vBar->setSteps(Scrollbar::pixelsPerLineStep(), pageStep);
    1132         m_vBar->setProportion(clientHeight, m_scrollHeight);
    1133     }
    1134 
    1135     updateScrollableAreaSet(hasScrollableHorizontalOverflow() || hasScrollableVerticalOverflow());
    1136 }
     1148    }
     1149}
     1150
    11371151
    11381152// This is called from layout code (before updateLayerPositions).

  • trunk/Source/WebCore/rendering/RenderLayerScrollableArea.h

    r271559 r271788  
    126126
    127127    void updateScrollInfoAfterLayout();
     128    void updateScrollbarSteps();
    128129
    129130    bool scroll(ScrollDirection, ScrollGranularity, float multiplier = 1);
Note: See TracChangeset for help on using the changeset viewer.