REGRESSION (r187593): Scroll position jumps when selecting text in an iframe
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 Dec 2015 00:35:04 +0000 (00:35 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 Dec 2015 00:35:04 +0000 (00:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=152541
rdar://problem/23886181

Reviewed by Tim Horton.

Source/WebCore:

r154382 added code that modifies parentLayer traversal, looking for ancestor
scrollable layers. However, it confusingly added another code path in which
the ancestor layer traversal cross a frame boundary, when RenderLayer::scrollRectToVisible()
already has one. I fixed this new location to adjust the rect coordinates in r187593,
but then code that hit both crossing points double-mapped the coordinates, causing
autoscroll jumping.

Fix by reverting r154382 and r187593, going back to doing the ancestor walk in
one place. Re-fix r154382 by implementing RenderLayer::allowsCurrentScroll(),
which contains the logic for line clamp, autoscroll and ensuring that overflow:hidden
can be programmatically scrolled.

Form controls are special; they can have overflow:hidden but still be user-scrollable
during autoscroll; this is handled via the confusingly-named canBeProgramaticallyScrolled().
RenderTextControlSingleLine implements this to ensure that readonly text inputs
autoscroll (which is exercised by a test).

The frame-to-parent-frame rect mapping in RenderLayer::scrollRectToVisible() is
fixed to use the coordinate mapping functions from Widget/ScrollView, with the
addition of a new utility function contentsToContainingViewContents().

A "Scrolling" logging channel is added with a few log points.

Test: fast/events/autoscroll-in-iframe-body.html

* page/scrolling/ScrollingCoordinator.cpp:
(WebCore::ScrollingCoordinator::absoluteNonFastScrollableRegionForFrame):
use contentsToContainingViewContents().
* platform/Logging.h:
* platform/ScrollView.cpp:
(WebCore::ScrollView::contentsToContainingViewContents):
* platform/ScrollView.h:
* platform/graphics/IntPoint.cpp:
(WebCore::IntPoint::constrainedBetween): New helper to constrain a point between
two other points.
* platform/graphics/IntPoint.h:
(WebCore::IntPoint::expandedTo):
(WebCore::IntPoint::shrunkTo):
* rendering/RenderBox.cpp:
* rendering/RenderLayer.cpp:
(WebCore::parentLayerCrossFrame):
(WebCore::RenderLayer::enclosingScrollableLayer):
(WebCore::frameElementAndViewPermitScroll):
(WebCore::RenderLayer::allowsCurrentScroll):
(WebCore::RenderLayer::scrollRectToVisible):
* rendering/RenderLayer.h:
* rendering/RenderTextControlSingleLine.h:

LayoutTests:

New test for autoscrolling iframe contents (an existing test scrolled an overflow:scroll
inside an iframe, and didn't catch the bug).

* fast/events/autoscroll-in-iframe-body-expected.txt: Added.
* fast/events/autoscroll-in-iframe-body.html: Added.
* fast/forms/input-readonly-autoscroll.html: Fix a missing double quote.

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

15 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/events/autoscroll-in-iframe-body-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/autoscroll-in-iframe-body.html [new file with mode: 0644]
LayoutTests/fast/forms/input-readonly-autoscroll.html
Source/WebCore/ChangeLog
Source/WebCore/page/scrolling/ScrollingCoordinator.cpp
Source/WebCore/platform/Logging.h
Source/WebCore/platform/ScrollView.cpp
Source/WebCore/platform/ScrollView.h
Source/WebCore/platform/graphics/IntPoint.cpp
Source/WebCore/platform/graphics/IntPoint.h
Source/WebCore/rendering/RenderBox.cpp
Source/WebCore/rendering/RenderLayer.cpp
Source/WebCore/rendering/RenderLayer.h
Source/WebCore/rendering/RenderTextControlSingleLine.h

index b5649a5..1e2efe4 100644 (file)
@@ -1,3 +1,18 @@
+2015-12-23  Simon Fraser  <simon.fraser@apple.com>
+
+        REGRESSION (r187593): Scroll position jumps when selecting text in an iframe
+        https://bugs.webkit.org/show_bug.cgi?id=152541
+        rdar://problem/23886181
+
+        Reviewed by Tim Horton.
+        
+        New test for autoscrolling iframe contents (an existing test scrolled an overflow:scroll
+        inside an iframe, and didn't catch the bug).
+
+        * fast/events/autoscroll-in-iframe-body-expected.txt: Added.
+        * fast/events/autoscroll-in-iframe-body.html: Added.
+        * fast/forms/input-readonly-autoscroll.html: Fix a missing double quote.
+
 2015-12-22  Simon Fraser  <simon.fraser@apple.com>
 
         Minor cleanup in RenderBox::canBeProgramaticallyScrolled()
diff --git a/LayoutTests/fast/events/autoscroll-in-iframe-body-expected.txt b/LayoutTests/fast/events/autoscroll-in-iframe-body-expected.txt
new file mode 100644 (file)
index 0000000..f1a090d
--- /dev/null
@@ -0,0 +1,3 @@
+
+PASSED: selecting in the iframe did not scroll the page.
+
diff --git a/LayoutTests/fast/events/autoscroll-in-iframe-body.html b/LayoutTests/fast/events/autoscroll-in-iframe-body.html
new file mode 100644 (file)
index 0000000..64dcca7
--- /dev/null
@@ -0,0 +1,96 @@
+<html>
+    <head>
+        <style>
+            body {
+                height: 2000px;
+            }
+            iframe {
+                position: absolute;
+                top: 800px;
+            }
+        </style>
+        <script src="../../resources/js-test-pre.js"></script>
+        <script>
+
+        if (window.testRunner) {
+            testRunner.waitUntilDone();
+            testRunner.dumpAsText();
+        }
+
+        function log(msg)
+        {
+            document.getElementById('console').appendChild(document.createTextNode(msg + '\n'));
+        }
+        
+        var verticalScrollOffset;
+        var iframe;
+        var mainFrameScrollOffset = 400;
+        
+        function getDragStart()
+        {
+            iframe = document.getElementById('targetFrame');
+            // Start dragging near the bottom of the iframe.
+            return {
+                'x' : iframe.clientLeft + iframe.clientWidth / 2,
+                'y' : iframe.clientTop + iframe.clientHeight - 10
+            };
+        }
+
+        function doTest()
+        {
+            document.scrollingElement.scrollTop = mainFrameScrollOffset;
+            if (document.scrollingElement.scrollTop != mainFrameScrollOffset)
+                log("FAILED: failed to scroll by " + mainFrameScrollOffset + "px");
+
+            iframe = document.getElementById('targetFrame');
+            // Start dragging near the bottom of the iframe.
+            var startPoint = getDragStart();
+            if (window.eventSender) {
+                eventSender.dragMode = false;
+                eventSender.mouseMoveTo(startPoint.x, startPoint.y);
+                eventSender.mouseDown();
+                eventSender.mouseUp();
+            }
+            setTimeout(autoscrollTestPart1, 0);
+        }
+
+        function autoscrollTestPart1()
+        {
+            var mainDocumentTop = document.scrollingElement.scrollTop;
+            if (mainDocumentTop != mainFrameScrollOffset)
+                log("FAILED: Clicking in the iframe scrolled the page (window.scrollTop is " + mainDocumentTop + ")");
+
+            if (window.eventSender) {
+                var startPoint = getDragStart();
+                eventSender.dragMode = false;
+                eventSender.mouseMoveTo(startPoint.x, startPoint.y);
+                eventSender.mouseDown();
+                eventSender.mouseMoveTo(startPoint.x + 10, startPoint.y);
+            }
+            setTimeout(autoscrollTestPart2, 100);
+        }
+
+        function autoscrollTestPart2()
+        {
+            if (window.eventSender)
+                eventSender.mouseUp();
+
+            var mainDocumentTop = document.scrollingElement.scrollTop;
+            if (mainDocumentTop == mainFrameScrollOffset)
+                log("PASSED: selecting in the iframe did not scroll the page.");
+            else
+                log("FAILED: the page autoscrolled (window.scrollTop is " + mainDocumentTop + ").");
+
+            if (window.testRunner)
+                testRunner.notifyDone();
+        }
+        
+        window.addEventListener('load', doTest, false);
+        </script>
+    </head>
+<body>
+    <iframe id="targetFrame" src="data:text/html,Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua."></iframe>
+    <div id="console"></div>
+</body>
+</html>
+
index bc8634c..9ba499d 100644 (file)
@@ -38,7 +38,7 @@
         </script>
     </head>
     <body onload="test()">
-        <p>Test for <a href=http://bugs.webkit.org/show_bug.cgi?id=11534">bug 11534</a>.</p>
+        <p>Test for <a href="http://bugs.webkit.org/show_bug.cgi?id=11534">bug 11534</a>.</p>
         <p>Readonly text fields don't scroll when selecting content.</p>
         <input id="tf" readonly value="abcdefghijklmnopqrstuvwxyz"></input>
         <div id="res"></div>
index b33d7c9..71229d1 100644 (file)
@@ -1,3 +1,59 @@
+2015-12-23  Simon Fraser  <simon.fraser@apple.com>
+
+        REGRESSION (r187593): Scroll position jumps when selecting text in an iframe
+        https://bugs.webkit.org/show_bug.cgi?id=152541
+        rdar://problem/23886181
+
+        Reviewed by Tim Horton.
+        
+        r154382 added code that modifies parentLayer traversal, looking for ancestor
+        scrollable layers. However, it confusingly added another code path in which
+        the ancestor layer traversal cross a frame boundary, when RenderLayer::scrollRectToVisible()
+        already has one. I fixed this new location to adjust the rect coordinates in r187593,
+        but then code that hit both crossing points double-mapped the coordinates, causing
+        autoscroll jumping.
+        
+        Fix by reverting r154382 and r187593, going back to doing the ancestor walk in
+        one place. Re-fix r154382 by implementing RenderLayer::allowsCurrentScroll(),
+        which contains the logic for line clamp, autoscroll and ensuring that overflow:hidden
+        can be programmatically scrolled.
+        
+        Form controls are special; they can have overflow:hidden but still be user-scrollable
+        during autoscroll; this is handled via the confusingly-named canBeProgramaticallyScrolled().
+        RenderTextControlSingleLine implements this to ensure that readonly text inputs
+        autoscroll (which is exercised by a test).
+        
+        The frame-to-parent-frame rect mapping in RenderLayer::scrollRectToVisible() is
+        fixed to use the coordinate mapping functions from Widget/ScrollView, with the
+        addition of a new utility function contentsToContainingViewContents().
+        
+        A "Scrolling" logging channel is added with a few log points.
+
+        Test: fast/events/autoscroll-in-iframe-body.html
+
+        * page/scrolling/ScrollingCoordinator.cpp:
+        (WebCore::ScrollingCoordinator::absoluteNonFastScrollableRegionForFrame):
+        use contentsToContainingViewContents().
+        * platform/Logging.h:
+        * platform/ScrollView.cpp:
+        (WebCore::ScrollView::contentsToContainingViewContents):
+        * platform/ScrollView.h:
+        * platform/graphics/IntPoint.cpp:
+        (WebCore::IntPoint::constrainedBetween): New helper to constrain a point between
+        two other points.
+        * platform/graphics/IntPoint.h:
+        (WebCore::IntPoint::expandedTo):
+        (WebCore::IntPoint::shrunkTo):
+        * rendering/RenderBox.cpp:
+        * rendering/RenderLayer.cpp:
+        (WebCore::parentLayerCrossFrame):
+        (WebCore::RenderLayer::enclosingScrollableLayer):
+        (WebCore::frameElementAndViewPermitScroll):
+        (WebCore::RenderLayer::allowsCurrentScroll):
+        (WebCore::RenderLayer::scrollRectToVisible):
+        * rendering/RenderLayer.h:
+        * rendering/RenderTextControlSingleLine.h:
+
 2015-12-22  Simon Fraser  <simon.fraser@apple.com>
 
         Minor cleanup in RenderBox::canBeProgramaticallyScrolled()
index bc93bec..53b98c1 100644 (file)
@@ -160,9 +160,7 @@ Region ScrollingCoordinator::absoluteNonFastScrollableRegionForFrame(const Frame
 
         Region subframeRegion = absoluteNonFastScrollableRegionForFrame(*subframe);
         // Map from the frame document to our document.
-        IntPoint offset = subframeView->contentsToView(IntPoint());
-        offset = subframeView->convertToContainingView(offset);
-        offset = frameView->viewToContents(offset);
+        IntPoint offset = subframeView->contentsToContainingViewContents(IntPoint());
 
         // FIXME: this translation ignores non-trival transforms on the frame.
         subframeRegion.translate(toIntSize(offset));
index 9c48555..3f0fd7a 100644 (file)
@@ -77,6 +77,7 @@ namespace WebCore {
     M(SQLDatabase) \
     M(SVG) \
     M(Services) \
+    M(Scrolling) \
     M(SpellingAndGrammar) \
     M(StorageAPI) \
     M(Threading) \
index f858234..d0dd2a9 100644 (file)
 #include "GraphicsContext.h"
 #include "GraphicsLayer.h"
 #include "HostWindow.h"
+#include "Logging.h"
 #include "PlatformMouseEvent.h"
 #include "PlatformWheelEvent.h"
 #include "ScrollAnimator.h"
 #include "Scrollbar.h"
 #include "ScrollbarTheme.h"
+#include "TextStream.h"
 #include <wtf/StdLibExtras.h>
 
 namespace WebCore {
@@ -871,6 +873,26 @@ IntRect ScrollView::contentsToView(IntRect rect) const
     return rect;
 }
 
+IntPoint ScrollView::contentsToContainingViewContents(const IntPoint& point) const
+{
+    if (const ScrollView* parentScrollView = parent()) {
+        IntPoint pointInContainingView = convertToContainingView(contentsToView(point));
+        return parentScrollView->viewToContents(pointInContainingView);
+    }
+
+    return contentsToView(point);
+}
+
+IntRect ScrollView::contentsToContainingViewContents(IntRect rect) const
+{
+    if (const ScrollView* parentScrollView = parent()) {
+        IntRect rectInContainingView = convertToContainingView(contentsToView(rect));
+        return parentScrollView->viewToContents(rectInContainingView);
+    }
+
+    return contentsToView(rect);
+}
+
 IntPoint ScrollView::rootViewToContents(const IntPoint& rootViewPoint) const
 {
     if (delegatesScrolling())
index 21c8539..f46e017 100644 (file)
@@ -294,6 +294,9 @@ public:
     IntRect viewToContents(IntRect) const;
     IntRect contentsToView(IntRect) const;
 
+    IntPoint contentsToContainingViewContents(const IntPoint&) const;
+    IntRect contentsToContainingViewContents(IntRect) const;
+
     WEBCORE_EXPORT IntPoint rootViewToTotalContents(const IntPoint&) const;
 
     // Event coordinates are assumed to be in the coordinate space of a window that contains
index 2264a42..0f7deab 100644 (file)
@@ -37,6 +37,15 @@ IntPoint::IntPoint(const FloatPoint& p)
 {
 }
 
+IntPoint IntPoint::constrainedBetween(const IntPoint& min, const IntPoint& max) const
+{
+    return {
+        std::max(min.x(), std::min(max.x(), m_x)),
+        std::max(min.y(), std::min(max.y(), m_y))
+    };
+}
+
+
 TextStream& operator<<(TextStream& ts, const IntPoint& p)
 {
     return ts << "(" << p.x() << "," << p.y() << ")";
index a2bc331..0b5d43a 100644 (file)
@@ -84,16 +84,22 @@ public:
     
     IntPoint expandedTo(const IntPoint& other) const
     {
-        return IntPoint(m_x > other.m_x ? m_x : other.m_x,
-            m_y > other.m_y ? m_y : other.m_y);
+        return {
+            m_x > other.m_x ? m_x : other.m_x,
+            m_y > other.m_y ? m_y : other.m_y
+        };
     }
 
     IntPoint shrunkTo(const IntPoint& other) const
     {
-        return IntPoint(m_x < other.m_x ? m_x : other.m_x,
-            m_y < other.m_y ? m_y : other.m_y);
+        return {
+            m_x < other.m_x ? m_x : other.m_x,
+            m_y < other.m_y ? m_y : other.m_y
+        };
     }
 
+    IntPoint constrainedBetween(const IntPoint& min, const IntPoint& max) const;
+
     int distanceSquaredToPoint(const IntPoint&) const;
 
     void clampNegativeToZero()
index 584f51a..68250f3 100644 (file)
@@ -875,6 +875,7 @@ bool RenderBox::isScrollableOrRubberbandableBox() const
     return canBeScrolledAndHasScrollableArea();
 }
 
+// FIXME: This is badly named. overflow:hidden can be programmatically scrolled, yet this returns false in that case.
 bool RenderBox::canBeProgramaticallyScrolled() const
 {
     if (isRenderView())
index f30527a..e8dea63 100644 (file)
@@ -78,6 +78,7 @@
 #include "HitTestingTransformState.h"
 #include "HitTestRequest.h"
 #include "HitTestResult.h"
+#include "Logging.h"
 #include "OverflowEvent.h"
 #include "OverlapTestRequestClient.h"
 #include "Page.h"
@@ -1454,7 +1455,7 @@ RenderLayer* RenderLayer::enclosingAncestorForPosition(EPosition position) const
     return curr;
 }
 
-static RenderLayer* parentLayerCrossFrame(const RenderLayer& layer, LayoutRect* rect = nullptr)
+static RenderLayer* parentLayerCrossFrame(const RenderLayer& layer)
 {
     if (layer.parent())
         return layer.parent();
@@ -1467,18 +1468,12 @@ static RenderLayer* parentLayerCrossFrame(const RenderLayer& layer, LayoutRect*
     if (!ownerRenderer)
         return nullptr;
 
-    // Convert the rect into the coordinate space of the parent frame's document.
-    if (rect) {
-        IntRect viewRect = layer.renderer().frame().view()->convertToContainingView(enclosingIntRect(*rect));
-        *rect = ownerRenderer->frame().view()->viewToContents(viewRect);
-    }
-
     return ownerRenderer->enclosingLayer();
 }
 
-RenderLayer* RenderLayer::enclosingScrollableLayer(LayoutRect* rect) const
+RenderLayer* RenderLayer::enclosingScrollableLayer() const
 {
-    for (RenderLayer* nextLayer = parentLayerCrossFrame(*this, rect); nextLayer; nextLayer = parentLayerCrossFrame(*nextLayer, rect)) {
+    for (RenderLayer* nextLayer = parentLayerCrossFrame(*this); nextLayer; nextLayer = parentLayerCrossFrame(*nextLayer)) {
         if (is<RenderBox>(nextLayer->renderer()) && downcast<RenderBox>(nextLayer->renderer()).canBeScrolledAndHasScrollableArea())
             return nextLayer;
     }
@@ -2283,6 +2278,7 @@ void RenderLayer::panScrollFromPoint(const IntPoint& sourcePoint)
     scrollByRecursively(adjustedScrollDelta(delta), ScrollOffsetClamped);
 }
 
+// FIXME: unify with the scrollRectToVisible() code below.
 void RenderLayer::scrollByRecursively(const IntSize& delta, ScrollOffsetClamping clamp, ScrollableArea** scrolledArea)
 {
     if (delta.isZero())
@@ -2436,23 +2432,48 @@ void RenderLayer::scrollTo(int x, int y)
     view.frameView().viewportContentsChanged();
 }
 
-static inline bool frameElementAndViewPermitScroll(HTMLFrameElementBase* frameElementBase, FrameView* frameView) 
+static inline bool frameElementAndViewPermitScroll(HTMLFrameElementBase* frameElementBase, FrameView& frameView)
 {
     // If scrollbars aren't explicitly forbidden, permit scrolling.
     if (frameElementBase && frameElementBase->scrollingMode() != ScrollbarAlwaysOff)
         return true;
 
     // If scrollbars are forbidden, user initiated scrolls should obviously be ignored.
-    if (frameView->wasScrolledByUser())
+    if (frameView.wasScrolledByUser())
         return false;
 
     // Forbid autoscrolls when scrollbars are off, but permits other programmatic scrolls,
     // like navigation to an anchor.
-    return !frameView->frame().eventHandler().autoscrollInProgress();
+    return !frameView.frame().eventHandler().autoscrollInProgress();
+}
+
+bool RenderLayer::allowsCurrentScroll() const
+{
+    if (!renderer().hasOverflowClip())
+        return false;
+
+    // Don't scroll to reveal an overflow layer that is restricted by the -webkit-line-clamp property.
+    // FIXME: Is this still needed? It used to be relevant for Safari RSS.
+    if (renderer().parent() && !renderer().parent()->style().lineClamp().isNone())
+        return false;
+
+    RenderBox* box = renderBox();
+    ASSERT(box); // Only boxes can have overflowClip set.
+
+    if (renderer().frame().eventHandler().autoscrollInProgress()) {
+        // The "programmatically" here is misleading; this asks whether the box has scrollable overflow,
+        // or is a special case like a form control.
+        return box->canBeProgramaticallyScrolled();
+    }
+
+    // Programmatic scrolls can scroll overflow:hidden.
+    return box->hasHorizontalOverflow() || box->hasVerticalOverflow();
 }
 
 void RenderLayer::scrollRectToVisible(const LayoutRect& rect, const ScrollAlignment& alignX, const ScrollAlignment& alignY)
 {
+    LOG_WITH_STREAM(Scrolling, stream << "Layer " << this << " scrollRectToVisible " << rect);
+
     RenderLayer* parentLayer = nullptr;
     LayoutRect newRect = rect;
 
@@ -2460,13 +2481,10 @@ void RenderLayer::scrollRectToVisible(const LayoutRect& rect, const ScrollAlignm
     // the end of the function since they could delete the layer or the layer's renderer().
     FrameView& frameView = renderer().view().frameView();
 
-    bool restrictedByLineClamp = false;
-    if (renderer().parent()) {
+    if (renderer().parent())
         parentLayer = renderer().parent()->enclosingLayer();
-        restrictedByLineClamp = !renderer().parent()->style().lineClamp().isNone();
-    }
 
-    if (renderer().hasOverflowClip() && !restrictedByLineClamp) {
+    if (allowsCurrentScroll()) {
         // Don't scroll to reveal an overflow layer that is restricted by the -webkit-line-clamp property.
         // This will prevent us from revealing text hidden by the slider in Safari RSS.
         RenderBox* box = renderBox();
@@ -2483,7 +2501,7 @@ void RenderLayer::scrollRectToVisible(const LayoutRect& rect, const ScrollAlignm
             localExposeRect.move(-scrollOffsetDifference);
             newRect = LayoutRect(box->localToAbsoluteQuad(FloatQuad(FloatRect(localExposeRect)), UseTransforms).boundingBox());
         }
-    } else if (!parentLayer && renderer().isBox() && renderBox()->canBeProgramaticallyScrolled()) {
+    } else if (!parentLayer && renderer().isRenderView()) {
         HTMLFrameOwnerElement* ownerElement = renderer().document().ownerElement();
 
         if (ownerElement && ownerElement->renderer()) {
@@ -2492,23 +2510,19 @@ void RenderLayer::scrollRectToVisible(const LayoutRect& rect, const ScrollAlignm
             if (is<HTMLFrameElementBase>(*ownerElement))
                 frameElementBase = downcast<HTMLFrameElementBase>(ownerElement);
 
-            if (frameElementAndViewPermitScroll(frameElementBase, &frameView)) {
+            if (frameElementAndViewPermitScroll(frameElementBase, frameView)) {
                 LayoutRect viewRect = frameView.visibleContentRect(LegacyIOSDocumentVisibleRect);
                 LayoutRect exposeRect = getRectToExpose(viewRect, viewRect, rect, alignX, alignY);
 
-                int xOffset = roundToInt(exposeRect.x());
-                int yOffset = roundToInt(exposeRect.y());
+                IntPoint scrollOffset(roundedIntPoint(exposeRect.location()));
                 // Adjust offsets if they're outside of the allowable range.
-                xOffset = std::max(0, std::min(frameView.contentsWidth(), xOffset));
-                yOffset = std::max(0, std::min(frameView.contentsHeight(), yOffset));
+                scrollOffset = scrollOffset.constrainedBetween(IntPoint(), IntPoint(frameView.contentsSize()));
+                frameView.setScrollPosition(scrollOffset);
 
-                frameView.setScrollPosition(IntPoint(xOffset, yOffset));
                 if (frameView.safeToPropagateScrollToParent()) {
                     parentLayer = ownerElement->renderer()->enclosingLayer();
-                    // FIXME: This doesn't correctly convert the rect to
-                    // absolute coordinates in the parent.
-                    newRect.setX(rect.x() - frameView.scrollX() + frameView.x());
-                    newRect.setY(rect.y() - frameView.scrollY() + frameView.y());
+                    // Convert the rect into the coordinate space of the parent frame's document.
+                    newRect = frameView.contentsToContainingViewContents(enclosingIntRect(newRect));
                 } else
                     parentLayer = nullptr;
             }
@@ -2537,9 +2551,6 @@ void RenderLayer::scrollRectToVisible(const LayoutRect& rect, const ScrollAlignm
         }
     }
     
-    if (renderer().frame().eventHandler().autoscrollInProgress())
-        parentLayer = enclosingScrollableLayer(&newRect);
-
     if (parentLayer)
         parentLayer->scrollRectToVisible(newRect, alignX, alignY);
 }
index cba8650..a773ecb 100644 (file)
@@ -404,7 +404,7 @@ public:
     RenderLayer* enclosingAncestorForPosition(EPosition) const;
 
     // Returns the nearest enclosing layer that is scrollable.
-    RenderLayer* enclosingScrollableLayer(LayoutRect* = nullptr) const;
+    RenderLayer* enclosingScrollableLayer() const;
 
     // The layer relative to which clipping rects for this layer are computed.
     RenderLayer* clippingRootForPainting() const;
@@ -897,6 +897,8 @@ private:
     IntSize scrollbarOffset(const Scrollbar*) const;
     
     void updateScrollableAreaSet(bool hasOverflow);
+    
+    bool allowsCurrentScroll() const;
 
     void dirtyAncestorChainVisibleDescendantStatus();
     void setAncestorChainHasVisibleDescendant();
index 9b162fe..fd3f70b 100644 (file)
@@ -105,6 +105,7 @@ public:
 private:
     virtual bool hasLineIfEmpty() const override { return true; }
     virtual bool isTextControlInnerBlock() const override { return true; }
+    virtual bool canBeProgramaticallyScrolled() const override { return true; }
 };
 
 } // namespace WebCore