Attachment 371468 Details for Bug 198597 – Patch

[patch] Patch

bug-198597-20190605195720.patch (text/plain), 26.70 KB, created by Simon Fraser (smfr) on 2019-06-05 19:57:21 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2019-06-05 19:57:21 PDT

Size: 26.70 KB

patch

obsolete

>Subversion Revision: 246042
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 5f6d5786955d54f2ca3cbb7a5da1d0fd9922edbc..f495a79bc8d8b7c64a872871993e0318077bc406 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,62 @@
>+2019-06-05  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Don't expose negative scrollLeft/scrollTop during rubber-banding
>+        https://bugs.webkit.org/show_bug.cgi?id=198597
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Several important sites and JS libraries are incompatible with values of scrollLeft/
>+        scrollTop becoming negative during rubber-banding (they get clamped to 0 and then set back,
>+        causing jumpiness).
>+
>+        The first step to fixing this is to clamps the JS-exposed values to be in the allowable range,
>+        for both overflow and page scrolling.
>+
>+        We pass ScrollClamping around in code that gets scrollPosition, scrollLeft and scrollTop,
>+        and add Element::scrollLeftForBindings() and Element::scrollTopForBindings() which ask for
>+        the clamped values.
>+
>+        Test: fast/scrolling/unconstrained-scrollto-page.html
>+
>+        * dom/Element.cpp:
>+        (WebCore::Element::scrollLeft):
>+        (WebCore::Element::scrollTop):
>+        * dom/Element.h:
>+        (WebCore::Element::scrollLeftForBindings):
>+        (WebCore::Element::scrollTopForBindings):
>+        (WebCore::Element::setScrollLeftForBindings):
>+        (WebCore::Element::setScrollTopForBindings):
>+        * dom/Element.idl:
>+        * platform/ScrollView.cpp:
>+        (WebCore::ScrollView::contentsScrollPosition const):
>+        (WebCore::ScrollView::scrollPosition const):
>+        * platform/ScrollView.h:
>+        * platform/ScrollableArea.cpp:
>+        (WebCore::ScrollableArea::scrolledToTop const):
>+        (WebCore::ScrollableArea::scrolledToBottom const):
>+        (WebCore::ScrollableArea::scrolledToLeft const):
>+        (WebCore::ScrollableArea::scrolledToRight const):
>+        (WebCore::ScrollableArea::scrollPosition const): Deleted. Removed this default implementation
>+        which had a weird dependency on scrollbar values, forcing implementors to write their own
>+        implementation.
>+        * platform/ScrollableArea.h:
>+        * rendering/RenderBox.cpp:
>+        (WebCore::RenderBox::scrollLeft const):
>+        (WebCore::RenderBox::scrollTop const):
>+        * rendering/RenderBox.h:
>+        * rendering/RenderLayer.cpp:
>+        (WebCore::RenderLayer::scrollPosition const):
>+        * rendering/RenderLayer.h:
>+        * rendering/RenderListBox.cpp:
>+        (WebCore::RenderListBox::scrollPosition const):
>+        (WebCore::RenderListBox::scrollLeft const):
>+        (WebCore::RenderListBox::scrollTop const):
>+        * rendering/RenderListBox.h:
>+        * rendering/RenderTextControlSingleLine.cpp:
>+        (WebCore::RenderTextControlSingleLine::scrollLeft const):
>+        (WebCore::RenderTextControlSingleLine::scrollTop const):
>+        * rendering/RenderTextControlSingleLine.h:
>+
> 2019-06-03  Timothy Hatcher  <timothy@apple.com>
> 
>         Tweak the text and underline color for data detected text.
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 3638a0682ed018fabaabb5c75b40302d57e2d0e6..ee254b2c5237e81ccd014020515697a97e681754 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,16 @@
>+2019-06-05  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Don't expose negative scrollLeft/scrollTop during rubber-banding
>+        https://bugs.webkit.org/show_bug.cgi?id=198597
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        PDFPlugin implements ScrollableArea so provide an implementation of scrollPosition().
>+
>+        * WebProcess/Plugins/PDF/PDFPlugin.h:
>+        * WebProcess/Plugins/PDF/PDFPlugin.mm:
>+        (WebKit::PDFPlugin::scrollPosition const):
>+
> 2019-06-03  Darin Adler  <darin@apple.com>
> 
>         Finish cleanup of String::number for floating point
>diff --git a/Source/WebCore/dom/Element.cpp b/Source/WebCore/dom/Element.cpp
>index 385e2223e0f3efc06163985b9a07e12909aa25ab..3753481c503efa3cde4f3f30ea463389caec48b5 100644
>--- a/Source/WebCore/dom/Element.cpp
>+++ b/Source/WebCore/dom/Element.cpp
>@@ -1119,32 +1119,42 @@ ALWAYS_INLINE Frame* Element::documentFrameWithNonNullView() const
> }
> 
> int Element::scrollLeft()
>+{
>+    return scrollLeft(ScrollClamping::Unclamped);
>+}
>+
>+int Element::scrollLeft(ScrollClamping clamping)
> {
>     document().updateLayoutIgnorePendingStylesheets();
> 
>     if (document().scrollingElement() == this) {
>         if (auto* frame = documentFrameWithNonNullView())
>-            return adjustContentsScrollPositionOrSizeForZoom(frame->view()->contentsScrollPosition().x(), *frame);
>+            return adjustContentsScrollPositionOrSizeForZoom(frame->view()->contentsScrollPosition(clamping).x(), *frame);
>         return 0;
>     }
> 
>     if (auto* renderer = renderBox())
>-        return adjustForAbsoluteZoom(renderer->scrollLeft(), *renderer);
>+        return adjustForAbsoluteZoom(renderer->scrollLeft(clamping), *renderer);
>     return 0;
> }
> 
> int Element::scrollTop()
>+{
>+    return scrollTop(ScrollClamping::Unclamped);
>+}
>+
>+int Element::scrollTop(ScrollClamping clamping)
> {
>     document().updateLayoutIgnorePendingStylesheets();
> 
>     if (document().scrollingElement() == this) {
>         if (auto* frame = documentFrameWithNonNullView())
>-            return adjustContentsScrollPositionOrSizeForZoom(frame->view()->contentsScrollPosition().y(), *frame);
>+            return adjustContentsScrollPositionOrSizeForZoom(frame->view()->contentsScrollPosition(clamping).y(), *frame);
>         return 0;
>     }
> 
>     if (RenderBox* renderer = renderBox())
>-        return adjustForAbsoluteZoom(renderer->scrollTop(), *renderer);
>+        return adjustForAbsoluteZoom(renderer->scrollTop(clamping), *renderer);
>     return 0;
> }
> 
>diff --git a/Source/WebCore/dom/Element.h b/Source/WebCore/dom/Element.h
>index 8ded545027fd5e88fe1f0fa3f3270b0469127bf5..230e6a16db829cc75d1cd9eb422f983ee7fe6bab 100644
>--- a/Source/WebCore/dom/Element.h
>+++ b/Source/WebCore/dom/Element.h
>@@ -180,12 +180,22 @@ public:
>     WEBCORE_EXPORT double clientWidth();
>     WEBCORE_EXPORT double clientHeight();
> 
>-    virtual int scrollLeft();
>-    virtual int scrollTop();
>-    virtual void setScrollLeft(int);
>-    virtual void setScrollTop(int);
>-    virtual int scrollWidth();
>-    virtual int scrollHeight();
>+    WEBCORE_EXPORT int scrollLeft();
>+    int scrollLeftForBindings() { return scrollLeft(ScrollClamping::Clamped); }
>+    int scrollLeft(ScrollClamping);
>+
>+    WEBCORE_EXPORT int scrollTop();
>+    int scrollTopForBindings() { return scrollTop(ScrollClamping::Clamped); }
>+    int scrollTop(ScrollClamping);
>+
>+    WEBCORE_EXPORT void setScrollLeft(int);
>+    void setScrollLeftForBindings(int value) { setScrollLeft(value); }
>+
>+    WEBCORE_EXPORT void setScrollTop(int);
>+    void setScrollTopForBindings(int value) { setScrollTop(value); }
>+
>+    WEBCORE_EXPORT int scrollWidth();
>+    WEBCORE_EXPORT int scrollHeight();
> 
>     WEBCORE_EXPORT IntRect boundsInRootViewSpace();
> 
>diff --git a/Source/WebCore/dom/Element.idl b/Source/WebCore/dom/Element.idl
>index 4c5550aee1b50f54dffbae12f270e8e0f3cadc52..e97201514c21d7fd8b0b41faf744248f3fd78dda 100644
>--- a/Source/WebCore/dom/Element.idl
>+++ b/Source/WebCore/dom/Element.idl
>@@ -78,8 +78,8 @@
>     void scrollTo(unrestricted double x, unrestricted double y);
>     void scrollBy(optional ScrollToOptions option);
>     void scrollBy(unrestricted double x, unrestricted double y);
>-    attribute long scrollTop; // FIXME(webkit.org/b/188045): should be unrestricted double.
>-    attribute long scrollLeft; // FIXME(webkit.org/b/188045): should be unrestricted double.
>+    [ImplementedAs=scrollTopForBindings] attribute long scrollTop; // FIXME(webkit.org/b/188045): should be unrestricted double.
>+    [ImplementedAs=scrollLeftForBindings] attribute long scrollLeft; // FIXME(webkit.org/b/188045): should be unrestricted double.
>     readonly attribute long scrollWidth;
>     readonly attribute long scrollHeight;
>     readonly attribute double clientTop; // FIXME: Should be of type long.
>diff --git a/Source/WebCore/platform/ScrollView.cpp b/Source/WebCore/platform/ScrollView.cpp
>index 61bb518bca1a95ea95bf978f89fd46114ecb5d40..ae7ffe730b2eb5c82043bcfbe910ac0890874ab7 100644
>--- a/Source/WebCore/platform/ScrollView.cpp
>+++ b/Source/WebCore/platform/ScrollView.cpp
>@@ -197,13 +197,13 @@ void ScrollView::setDelegatesScrolling(bool delegatesScrolling)
>     delegatesScrollingDidChange();
> }
> 
>-IntPoint ScrollView::contentsScrollPosition() const
>+IntPoint ScrollView::contentsScrollPosition(ScrollClamping clamping) const
> {
> #if PLATFORM(IOS_FAMILY)
>     if (platformWidget())
>         return actualScrollPosition();
> #endif
>-    return scrollPosition();
>+    return scrollPosition(clamping);
> }
> 
> void ScrollView::setContentsScrollPosition(const IntPoint& position)
>@@ -350,6 +350,15 @@ void ScrollView::setContentsSize(const IntSize& newSize)
>     updateOverhangAreas();
> }
> 
>+ScrollPosition ScrollView::scrollPosition(ScrollClamping clamping) const
>+{
>+    auto scrollPosition = visibleContentRect(LegacyIOSDocumentVisibleRect).location();
>+    if (clamping == ScrollClamping::Clamped)
>+        return constrainScrollPosition(scrollPosition);
>+
>+    return scrollPosition;
>+}
>+
> ScrollPosition ScrollView::maximumScrollPosition() const
> {
>     ScrollPosition maximumPosition = ScrollableArea::maximumScrollPosition();
>diff --git a/Source/WebCore/platform/ScrollView.h b/Source/WebCore/platform/ScrollView.h
>index ff82c27644ca0e10dc548c88e1b5e7aeac18fd97..58e86971683a82f8cbe816fb6377c69156a6c319 100644
>--- a/Source/WebCore/platform/ScrollView.h
>+++ b/Source/WebCore/platform/ScrollView.h
>@@ -221,7 +221,7 @@ public:
>     virtual void setContentsSize(const IntSize&);
> 
>     // Functions for querying the current scrolled position (both as a point, a size, or as individual X and Y values).
>-    ScrollPosition scrollPosition() const final { return visibleContentRect(LegacyIOSDocumentVisibleRect).location(); }
>+    WEBCORE_EXPORT ScrollPosition scrollPosition(ScrollClamping = ScrollClamping::Unclamped) const final;
> 
>     ScrollPosition maximumScrollPosition() const override; // The maximum position we can be scrolled to.
> 
>@@ -231,7 +231,7 @@ public:
>     int scrollY() const { return scrollPosition().y(); }
> 
>     // Scroll position used by web-exposed features (has legacy iOS behavior).
>-    WEBCORE_EXPORT IntPoint contentsScrollPosition() const;
>+    WEBCORE_EXPORT IntPoint contentsScrollPosition(ScrollClamping = ScrollClamping::Unclamped) const;
>     void setContentsScrollPosition(const IntPoint&);
> 
> #if PLATFORM(IOS_FAMILY)
>diff --git a/Source/WebCore/platform/ScrollableArea.cpp b/Source/WebCore/platform/ScrollableArea.cpp
>index a98e5dab6ef60dcab2f97719d5f0d8ece5ef131b..dd6645da9dbb2a7736be999e5ee0a7501eea57f7 100644
>--- a/Source/WebCore/platform/ScrollableArea.cpp
>+++ b/Source/WebCore/platform/ScrollableArea.cpp
>@@ -612,14 +612,6 @@ IntSize ScrollableArea::scrollbarIntrusion() const
>     return { horizontalScrollbarIntrusion(), verticalScrollbarIntrusion() };
> }
> 
>-ScrollPosition ScrollableArea::scrollPosition() const
>-{
>-    // FIXME: This relationship seems to be inverted. Scrollbars should be 'view', not 'model', and should get their values from us.
>-    int x = horizontalScrollbar() ? horizontalScrollbar()->value() : 0;
>-    int y = verticalScrollbar() ? verticalScrollbar()->value() : 0;
>-    return IntPoint(x, y);
>-}
>-
> ScrollPosition ScrollableArea::minimumScrollPosition() const
> {
>     return scrollPositionFromOffset(ScrollPosition());
>@@ -647,22 +639,22 @@ ScrollOffset ScrollableArea::scrollOffsetFromPosition(ScrollPosition position) c
> 
> bool ScrollableArea::scrolledToTop() const
> {
>-    return scrollPosition().y() <= minimumScrollPosition().y();
>+    return scrollPosition(ScrollClamping::Clamped).y() == minimumScrollPosition().y();
> }
> 
> bool ScrollableArea::scrolledToBottom() const
> {
>-    return scrollPosition().y() >= maximumScrollPosition().y();
>+    return scrollPosition(ScrollClamping::Clamped).y() == maximumScrollPosition().y();
> }
> 
> bool ScrollableArea::scrolledToLeft() const
> {
>-    return scrollPosition().x() <= minimumScrollPosition().x();
>+    return scrollPosition(ScrollClamping::Clamped).x() == minimumScrollPosition().x();
> }
> 
> bool ScrollableArea::scrolledToRight() const
> {
>-    return scrollPosition().x() >= maximumScrollPosition().x();
>+    return scrollPosition(ScrollClamping::Clamped).x() == maximumScrollPosition().x();
> }
> 
> void ScrollableArea::scrollbarStyleChanged(ScrollbarStyle, bool)
>diff --git a/Source/WebCore/platform/ScrollableArea.h b/Source/WebCore/platform/ScrollableArea.h
>index fde607f38682e390b914e4861781fffa13ea28af..34c5cf3d4e1bd7033401b119c7d66544c58ba43d 100644
>--- a/Source/WebCore/platform/ScrollableArea.h
>+++ b/Source/WebCore/platform/ScrollableArea.h
>@@ -196,7 +196,7 @@ public:
>     const IntPoint& scrollOrigin() const { return m_scrollOrigin; }
>     bool scrollOriginChanged() const { return m_scrollOriginChanged; }
> 
>-    virtual ScrollPosition scrollPosition() const;
>+    virtual ScrollPosition scrollPosition(ScrollClamping = ScrollClamping::Unclamped) const = 0;
>     virtual ScrollPosition minimumScrollPosition() const;
>     virtual ScrollPosition maximumScrollPosition() const;
> 
>diff --git a/Source/WebCore/rendering/RenderBox.cpp b/Source/WebCore/rendering/RenderBox.cpp
>index c71e73cf723475c2e28c5ce45c005cdc7d00910c..0d040530365f7c21c9263b33bcba95ac2efc8fc4 100644
>--- a/Source/WebCore/rendering/RenderBox.cpp
>+++ b/Source/WebCore/rendering/RenderBox.cpp
>@@ -557,14 +557,14 @@ int RenderBox::scrollHeight() const
>     return roundToInt(std::max(clientHeight(), layoutOverflowRect().maxY() - borderTop()));
> }
> 
>-int RenderBox::scrollLeft() const
>+int RenderBox::scrollLeft(ScrollClamping clamping) const
> {
>-    return hasOverflowClip() && layer() ? layer()->scrollPosition().x() : 0;
>+    return hasOverflowClip() && layer() ? layer()->scrollPosition(clamping).x() : 0;
> }
> 
>-int RenderBox::scrollTop() const
>+int RenderBox::scrollTop(ScrollClamping clamping) const
> {
>-    return hasOverflowClip() && layer() ? layer()->scrollPosition().y() : 0;
>+    return hasOverflowClip() && layer() ? layer()->scrollPosition(clamping).y() : 0;
> }
> 
> static void setupWheelEventTestTrigger(RenderLayer& layer)
>diff --git a/Source/WebCore/rendering/RenderBox.h b/Source/WebCore/rendering/RenderBox.h
>index b7865087f2f63497a3335bde412f1e1384e62036..eef0b09d609bdf16418cf474621b3e341c0f3d71 100644
>--- a/Source/WebCore/rendering/RenderBox.h
>+++ b/Source/WebCore/rendering/RenderBox.h
>@@ -243,8 +243,8 @@ public:
>     // scrollLeft/Top return the current scroll position.  These methods are virtual so that objects like
>     // textareas can scroll shadow content (but pretend that they are the objects that are
>     // scrolling).
>-    virtual int scrollLeft() const;
>-    virtual int scrollTop() const;
>+    virtual int scrollLeft(ScrollClamping = ScrollClamping::Unclamped) const;
>+    virtual int scrollTop(ScrollClamping = ScrollClamping::Unclamped) const;
>     virtual int scrollWidth() const;
>     virtual int scrollHeight() const;
>     virtual void setScrollLeft(int, ScrollType, ScrollClamping = ScrollClamping::Clamped);
>diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp
>index 3a0d23c4620e47e06e01d11730b1b1507293abc5..ff8549d119aa2256da851bc124e54f2ee20217f6 100644
>--- a/Source/WebCore/rendering/RenderLayer.cpp
>+++ b/Source/WebCore/rendering/RenderLayer.cpp
>@@ -2894,6 +2894,13 @@ void RenderLayer::resize(const PlatformMouseEvent& evt, const LayoutSize& oldOff
>     // FIXME (Radar 4118564): We should also autoscroll the window as necessary to keep the point under the cursor in view.
> }
> 
>+ScrollPosition RenderLayer::scrollPosition(ScrollClamping clamping) const
>+{
>+    if (clamping == ScrollClamping::Clamped)
>+        return constrainScrollPosition(m_scrollPosition);
>+    return m_scrollPosition;
>+}
>+
> int RenderLayer::scrollSize(ScrollbarOrientation orientation) const
> {
>     Scrollbar* scrollbar = ((orientation == HorizontalScrollbar) ? m_hBar : m_vBar).get();
>diff --git a/Source/WebCore/rendering/RenderLayer.h b/Source/WebCore/rendering/RenderLayer.h
>index 5e07bc775f22be480fde242c58c78da46373a32d..80aaa65ef96feb9d447dda111529b46b915d786d 100644
>--- a/Source/WebCore/rendering/RenderLayer.h
>+++ b/Source/WebCore/rendering/RenderLayer.h
>@@ -466,7 +466,7 @@ public:
>     bool verticalScrollbarHiddenByStyle() const override;
> 
>     // ScrollableArea overrides
>-    ScrollPosition scrollPosition() const override { return m_scrollPosition; }
>+    ScrollPosition scrollPosition(ScrollClamping = ScrollClamping::Unclamped) const override;
> 
>     Scrollbar* horizontalScrollbar() const override { return m_hBar.get(); }
>     Scrollbar* verticalScrollbar() const override { return m_vBar.get(); }
>diff --git a/Source/WebCore/rendering/RenderListBox.cpp b/Source/WebCore/rendering/RenderListBox.cpp
>index fe982e0477f0f912e21da3f4dc3309713a564ad6..48b6307ee2549c6d7198bcd3fcf2d08a5db54a0e 100644
>--- a/Source/WebCore/rendering/RenderListBox.cpp
>+++ b/Source/WebCore/rendering/RenderListBox.cpp
>@@ -744,7 +744,12 @@ int RenderListBox::scrollHeight() const
>     return roundToInt(std::max(clientHeight(), listHeight()));
> }
> 
>-int RenderListBox::scrollLeft() const
>+ScrollPosition RenderListBox::scrollPosition(ScrollClamping) const
>+{
>+    return { 0, m_indexOffset * itemHeight() };
>+}
>+
>+int RenderListBox::scrollLeft(ScrollClamping) const
> {
>     return 0;
> }
>@@ -753,7 +758,7 @@ void RenderListBox::setScrollLeft(int, ScrollType, ScrollClamping)
> {
> }
> 
>-int RenderListBox::scrollTop() const
>+int RenderListBox::scrollTop(ScrollClamping) const
> {
>     return m_indexOffset * itemHeight();
> }
>diff --git a/Source/WebCore/rendering/RenderListBox.h b/Source/WebCore/rendering/RenderListBox.h
>index 3cd1697e2a48a53ea9d0a1320afc9a0e150a426b..268d15eddd9af1ca5741af97a3d1d660492ca847 100644
>--- a/Source/WebCore/rendering/RenderListBox.h
>+++ b/Source/WebCore/rendering/RenderListBox.h
>@@ -102,8 +102,9 @@ private:
>     void panScroll(const IntPoint&) override;
> 
>     int verticalScrollbarWidth() const override;
>-    int scrollLeft() const override;
>-    int scrollTop() const override;
>+    ScrollPosition scrollPosition(ScrollClamping) const override;
>+    int scrollLeft(ScrollClamping) const override;
>+    int scrollTop(ScrollClamping) const override;
>     int scrollWidth() const override;
>     int scrollHeight() const override;
>     void setScrollLeft(int, ScrollType, ScrollClamping) override;
>diff --git a/Source/WebCore/rendering/RenderTextControlSingleLine.cpp b/Source/WebCore/rendering/RenderTextControlSingleLine.cpp
>index a3018fd077d9960c27a040eb0be331891092aaed..6ea403aff99d1246dee85f3e6edb902e402fe0dd 100644
>--- a/Source/WebCore/rendering/RenderTextControlSingleLine.cpp
>+++ b/Source/WebCore/rendering/RenderTextControlSingleLine.cpp
>@@ -362,18 +362,18 @@ int RenderTextControlSingleLine::scrollHeight() const
>     return RenderBlockFlow::scrollHeight();
> }
> 
>-int RenderTextControlSingleLine::scrollLeft() const
>+int RenderTextControlSingleLine::scrollLeft(ScrollClamping clamping) const
> {
>     if (innerTextElement())
>-        return innerTextElement()->scrollLeft();
>-    return RenderBlockFlow::scrollLeft();
>+        return innerTextElement()->scrollLeft(clamping);
>+    return RenderBlockFlow::scrollLeft(clamping);
> }
> 
>-int RenderTextControlSingleLine::scrollTop() const
>+int RenderTextControlSingleLine::scrollTop(ScrollClamping clamping) const
> {
>     if (innerTextElement())
>-        return innerTextElement()->scrollTop();
>-    return RenderBlockFlow::scrollTop();
>+        return innerTextElement()->scrollTop(clamping);
>+    return RenderBlockFlow::scrollTop(clamping);
> }
> 
> void RenderTextControlSingleLine::setScrollLeft(int newLeft, ScrollType, ScrollClamping)
>diff --git a/Source/WebCore/rendering/RenderTextControlSingleLine.h b/Source/WebCore/rendering/RenderTextControlSingleLine.h
>index 3f5786dfe400eb80fd8e2b3906662bd99b94f13a..895e7b6b1ec8ff9f25b7a8cb2a559b9cc681a7a2 100644
>--- a/Source/WebCore/rendering/RenderTextControlSingleLine.h
>+++ b/Source/WebCore/rendering/RenderTextControlSingleLine.h
>@@ -53,8 +53,8 @@ private:
>     void autoscroll(const IntPoint&) override;
> 
>     // Subclassed to forward to our inner div.
>-    int scrollLeft() const override;
>-    int scrollTop() const override;
>+    int scrollLeft(ScrollClamping) const override;
>+    int scrollTop(ScrollClamping) const override;
>     int scrollWidth() const override;
>     int scrollHeight() const override;
>     void setScrollLeft(int, ScrollType, ScrollClamping) override;
>diff --git a/Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.h b/Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.h
>index 6862e304fedb85b78634700b9d6b35a6c3c20cec..a7672bb62cfad3edc03afb4110a09362524744a2 100644
>--- a/Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.h
>+++ b/Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.h
>@@ -206,7 +206,7 @@ private:
>     bool isActive() const final;
>     bool isScrollCornerVisible() const final { return false; }
>     int scrollOffset(WebCore::ScrollbarOrientation) const final;
>-    WebCore::ScrollPosition scrollPosition() const final;
>+    WebCore::ScrollPosition scrollPosition(WebCore::ScrollClamping = WebCore::ScrollClamping::Unclamped) const final;
>     WebCore::ScrollPosition minimumScrollPosition() const final;
>     WebCore::ScrollPosition maximumScrollPosition() const final;
>     WebCore::IntSize visibleSize() const final { return m_size; }
>diff --git a/Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm b/Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm
>index 1588bd0e1c50798804ce1414ff033275f45c9bca..f8f4a6a3c701d103c810e68e32096005d442f9e3 100644
>--- a/Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm
>+++ b/Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm
>@@ -901,9 +901,14 @@ int PDFPlugin::scrollOffset(ScrollbarOrientation orientation) const
>     return 0;
> }
> 
>-ScrollPosition PDFPlugin::scrollPosition() const
>+ScrollPosition PDFPlugin::scrollPosition(ScrollClamping clamping) const
> {
>-    return IntPoint(m_scrollOffset.width(), m_scrollOffset.height());
>+    ScrollPosition scrollPosition = IntPoint(m_scrollOffset);
>+
>+    if (clamping == ScrollClamping::Clamped)
>+        return constrainScrollPosition(scrollPosition);
>+
>+    return scrollPosition;
> }
> 
> ScrollPosition PDFPlugin::minimumScrollPosition() const
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index ea4ebabfb378e9ffac8a68aff82ad0a5f05fc311..4933f126120ff9ff31421595cc8af429ab03d9bd 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,15 @@
>+2019-06-05  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Don't expose negative scrollLeft/scrollTop during rubber-banding
>+        https://bugs.webkit.org/show_bug.cgi?id=198597
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/scrolling/unconstrained-scrollto-expected.txt:
>+        * fast/scrolling/unconstrained-scrollto-page-expected.txt: Added.
>+        * fast/scrolling/unconstrained-scrollto-page.html: Added.
>+        * fast/scrolling/unconstrained-scrollto.html:
>+
> 2019-06-03  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [JSC] JSObject::attemptToInterceptPutByIndexOnHole should use getPrototype instead of getPrototypeDirect
>diff --git a/LayoutTests/fast/scrolling/unconstrained-scrollto-expected.txt b/LayoutTests/fast/scrolling/unconstrained-scrollto-expected.txt
>index d764a901e03aad20a47dddc8426cf037100b1b95..3808b7210decba47d78b48577b55ec2b7e1f351a 100644
>--- a/LayoutTests/fast/scrolling/unconstrained-scrollto-expected.txt
>+++ b/LayoutTests/fast/scrolling/unconstrained-scrollto-expected.txt
>@@ -1,5 +1,8 @@
>-PASS scroller.scrollTop is -150
> PASS successfullyParsed is true
> 
> TEST COMPLETE
>+PASS scroller.scrollLeft is 100
>+PASS scroller.scrollTop is 150
>+PASS scroller.scrollLeft is 0
>+PASS scroller.scrollTop is 0
> 
>diff --git a/LayoutTests/fast/scrolling/unconstrained-scrollto-page-expected.txt b/LayoutTests/fast/scrolling/unconstrained-scrollto-page-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..70e5d5bf26d6b3f99601463fd338de3146c29cc5
>--- /dev/null
>+++ b/LayoutTests/fast/scrolling/unconstrained-scrollto-page-expected.txt
>@@ -0,0 +1,8 @@
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+PASS document.scrollingElement.scrollLeft is 100
>+PASS document.scrollingElement.scrollTop is 150
>+PASS document.scrollingElement.scrollLeft is 0
>+PASS document.scrollingElement.scrollTop is 0
>+
>diff --git a/LayoutTests/fast/scrolling/unconstrained-scrollto-page.html b/LayoutTests/fast/scrolling/unconstrained-scrollto-page.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..ec59647030e30664bf276caab664b7e85d0c3c84
>--- /dev/null
>+++ b/LayoutTests/fast/scrolling/unconstrained-scrollto-page.html
>@@ -0,0 +1,33 @@
>+<!DOCTYPE html>
>+
>+<html>
>+<head>
>+    <script src="../../resources/js-test-pre.js"></script>
>+    <style>
>+        body {
>+            height: 2000px;
>+            width: 2000px;
>+        }
>+    </style>
>+    <script>
>+        function doTest()
>+        {
>+            if (!window.internals)
>+                return;
>+
>+            internals.unconstrainedScrollTo(document.scrollingElement, 100, 150);
>+            shouldBe('document.scrollingElement.scrollLeft', '100');
>+            shouldBe('document.scrollingElement.scrollTop', '150');
>+
>+            internals.unconstrainedScrollTo(document.scrollingElement, -100, -150);
>+            shouldBe('document.scrollingElement.scrollLeft', '0');
>+            shouldBe('document.scrollingElement.scrollTop', '0');
>+        }
>+        
>+        window.addEventListener('load', doTest, false);
>+    </script>
>+</head>
>+<body>
>+<script src="../../resources/js-test-post.js"></script>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/scrolling/unconstrained-scrollto.html b/LayoutTests/fast/scrolling/unconstrained-scrollto.html
>index c0ccf7978d987f84e55d9451f82199bc59713a65..052ff74be92b0c9c2384d36330488211b1e5c4fb 100644
>--- a/LayoutTests/fast/scrolling/unconstrained-scrollto.html
>+++ b/LayoutTests/fast/scrolling/unconstrained-scrollto.html
>@@ -12,23 +12,27 @@
>         }
>         
>         .contents {
>-            width: 100%;
>+            width: 200%;
>             height: 200%;
>             background-color: silver;
>         }
>     </style>
>     <script>
>-        var jsTestIsAsync = true;
>-
>         var scroller;
>         function doTest()
>         {
>+            if (!window.internals)
>+                return;
>+
>             scroller = document.getElementById('scroller');
>-            if (window.internals)
>-                internals.unconstrainedScrollTo(scroller, 0, -150);
> 
>-            shouldBe('scroller.scrollTop', '-150');
>-            finishJSTest();
>+            internals.unconstrainedScrollTo(scroller, 100, 150);
>+            shouldBe('scroller.scrollLeft', '100');
>+            shouldBe('scroller.scrollTop', '150');
>+
>+            internals.unconstrainedScrollTo(scroller, -100, -150);
>+            shouldBe('scroller.scrollLeft', '0');
>+            shouldBe('scroller.scrollTop', '0');
>         }
>         
>         window.addEventListener('load', doTest, false);

Actions: View | Formatted Diff | Diff

Attachments on bug 198597: 371468