Improve the behavior of scroll-into-view when the target is inside position:fixed
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 3 Dec 2016 18:33:54 +0000 (18:33 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 3 Dec 2016 18:33:54 +0000 (18:33 +0000)
https://bugs.webkit.org/show_bug.cgi?id=165354

Reviewed by Zalan Bujtas.
Source/WebCore:

The existing RenderLayer::scrollRectToVisible() code paid no heed to whether the
target was inside position:fixed, resulting in unwanted scrolls.

Fix this by plumbing through from the call sites a "insideFixed" flag which we get
when we call localToAbsolute(), and use this flag to avoid scrolling at all if
unzoomed.

If zoomed and we're focussing something inside position:fixed, and if visual viewports
are enabled, we can compute the visual viewport required to reveal the target rect,
which gives us the ideal scroll position.

Fix a bug on non-iOS platforms when zoomed, which is to scale the viewRect since
frameView.visibleContentRect() gives an unscaled rect on those platforms.

Not all callers of scrollRectToVisible() are fixed, but those that are not will get
the current behavior.

Tests: fast/overflow/scroll-anchor-in-position-fixed.html
       fast/visual-viewport/zoomed-scroll-into-view-fixed.html
       fast/visual-viewport/zoomed-scroll-to-anchor-in-position-fixed.html

* dom/Element.cpp:
(WebCore::Element::scrollIntoView):
(WebCore::Element::scrollIntoViewIfNeeded):
(WebCore::Element::scrollIntoViewIfNotVisible):
(WebCore::Element::updateFocusAppearance):
* editing/FrameSelection.cpp:
(WebCore::FrameSelection::FrameSelection):
(WebCore::FrameSelection::absoluteCaretBounds):
(WebCore::FrameSelection::recomputeCaretRect):
(WebCore::FrameSelection::revealSelection):
* editing/FrameSelection.h:
* editing/VisiblePosition.cpp:
(WebCore::VisiblePosition::absoluteCaretBounds):
* editing/VisiblePosition.h:
* editing/htmlediting.cpp:
(WebCore::absoluteBoundsForLocalCaretRect):
* editing/htmlediting.h:
* page/FrameView.cpp:
(WebCore::FrameView::scrollElementToRect):
(WebCore::FrameView::scrollToAnchor):
* page/PrintContext.cpp:
(WebCore::PrintContext::outputLinkedDestinations):
* rendering/RenderElement.cpp:
(WebCore::RenderElement::getLeadingCorner):
(WebCore::RenderElement::getTrailingCorner):
(WebCore::RenderElement::absoluteAnchorRect):
(WebCore::RenderElement::anchorRect): Renamed to absoluteAnchorRect().
* rendering/RenderElement.h:
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::scrollRectToVisible):
(WebCore::RenderLayer::getRectToExpose):
(WebCore::RenderLayer::autoscroll):
* rendering/RenderLayer.h:
* rendering/RenderObject.cpp:
(WebCore::RenderObject::scrollRectToVisible):
* rendering/RenderObject.h:

Source/WebKit/mac:

Plumb through 'insideFixed'. We don't get compute it, so behavior from
these call sites won't change.

* WebView/WebFrame.mm:
(-[WebFrame _scrollDOMRangeToVisible:]):
(-[WebFrame _scrollDOMRangeToVisible:withInset:]):

LayoutTests:

* fast/overflow/scroll-anchor-in-position-fixed-expected.txt: Added.
* fast/overflow/scroll-anchor-in-position-fixed.html: Added.
* fast/visual-viewport/zoomed-scroll-to-anchor-in-position-fixed-expected.txt: Added.
* fast/visual-viewport/zoomed-scroll-to-anchor-in-position-fixed.html: Added.
* platform/ios-simulator/TestExpectations:

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

27 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/overflow/scroll-anchor-in-position-fixed-expected.txt [new file with mode: 0644]
LayoutTests/fast/overflow/scroll-anchor-in-position-fixed.html [new file with mode: 0644]
LayoutTests/fast/transforms/selection-bounds-in-transformed-view.html
LayoutTests/fast/visual-viewport/zoomed-scroll-into-view-fixed-expected.txt [new file with mode: 0644]
LayoutTests/fast/visual-viewport/zoomed-scroll-into-view-fixed.html [new file with mode: 0644]
LayoutTests/fast/visual-viewport/zoomed-scroll-to-anchor-in-position-fixed-expected.txt [new file with mode: 0644]
LayoutTests/fast/visual-viewport/zoomed-scroll-to-anchor-in-position-fixed.html [new file with mode: 0644]
LayoutTests/platform/ios-simulator/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/dom/Element.cpp
Source/WebCore/editing/FrameSelection.cpp
Source/WebCore/editing/FrameSelection.h
Source/WebCore/editing/VisiblePosition.cpp
Source/WebCore/editing/VisiblePosition.h
Source/WebCore/editing/htmlediting.cpp
Source/WebCore/editing/htmlediting.h
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/PrintContext.cpp
Source/WebCore/rendering/RenderElement.cpp
Source/WebCore/rendering/RenderElement.h
Source/WebCore/rendering/RenderLayer.cpp
Source/WebCore/rendering/RenderLayer.h
Source/WebCore/rendering/RenderObject.cpp
Source/WebCore/rendering/RenderObject.h
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/WebView/WebFrame.mm

index 4d25eeb..989a202 100644 (file)
@@ -1,3 +1,16 @@
+2016-12-02  Simon Fraser  <simon.fraser@apple.com>
+
+        Improve the behavior of scroll-into-view when the target is inside position:fixed
+        https://bugs.webkit.org/show_bug.cgi?id=165354
+
+        Reviewed by Zalan Bujtas.
+
+        * fast/overflow/scroll-anchor-in-position-fixed-expected.txt: Added.
+        * fast/overflow/scroll-anchor-in-position-fixed.html: Added.
+        * fast/visual-viewport/zoomed-scroll-to-anchor-in-position-fixed-expected.txt: Added.
+        * fast/visual-viewport/zoomed-scroll-to-anchor-in-position-fixed.html: Added.
+        * platform/ios-simulator/TestExpectations:
+
 2016-11-30  Simon Fraser  <simon.fraser@apple.com>
 
         localToAbsolute() does incorrect conversion for elements inside position:fixed with zooming
diff --git a/LayoutTests/fast/overflow/scroll-anchor-in-position-fixed-expected.txt b/LayoutTests/fast/overflow/scroll-anchor-in-position-fixed-expected.txt
new file mode 100644 (file)
index 0000000..d040084
--- /dev/null
@@ -0,0 +1,11 @@
+Tests scrolling to an anchor inside position:fixed doesn't try to scroll the page
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS document.scrollingElement.scrollTop is 800
+PASS document.scrollingElement.scrollLeft is 100
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Anchor is here
diff --git a/LayoutTests/fast/overflow/scroll-anchor-in-position-fixed.html b/LayoutTests/fast/overflow/scroll-anchor-in-position-fixed.html
new file mode 100644 (file)
index 0000000..d4aa9db
--- /dev/null
@@ -0,0 +1,53 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        body {
+            height: 2000px;
+            width: 2000px;
+        }
+        
+        .fixed {
+            position: fixed;
+            top: 50px;
+            left: 40px;
+            border: 1px solid black;
+        }
+    </style>
+    <script src="../../resources/js-test-pre.js"></script>
+    <script>
+    description("Tests scrolling to an anchor inside position:fixed doesn't try to scroll the page");
+    window.jsTestIsAsync = true;
+
+    function runTest()
+    {
+        window.scrollTo(100, 800);
+        setTimeout(function() {
+            window.location='#anchor';
+            setTimeout(finishTest, 0);
+        }, 0);
+    }
+
+    function finishTest()
+    {
+        if (window.location.toString().indexOf("#") == -1) {
+            setTimeout(finishTest, 0);
+            return;
+        }
+        
+        shouldBe('document.scrollingElement.scrollTop', '800');
+        shouldBe('document.scrollingElement.scrollLeft', '100');
+
+        finishJSTest();
+    }
+    </script>
+</head>
+<body onload="runTest()">
+
+<div class="fixed">
+    <a name="anchor">Anchor is here</a>
+</div>
+
+<script src="../../resources/js-test-post.js"></script>
+
+</body></html>
index c6e0f6f..01d9c39 100644 (file)
@@ -10,6 +10,6 @@
         }
 
         document.execCommand("FindString", false, "target");
-        document.getElementById("result").innerText = document.body.scrollTop === 864 ? "PASS" : "FAIL";
+        document.getElementById("result").innerText = document.body.scrollTop === 937 ? "PASS" : "FAIL";
     </script>
 </body>
diff --git a/LayoutTests/fast/visual-viewport/zoomed-scroll-into-view-fixed-expected.txt b/LayoutTests/fast/visual-viewport/zoomed-scroll-into-view-fixed-expected.txt
new file mode 100644 (file)
index 0000000..8095534
--- /dev/null
@@ -0,0 +1,24 @@
+Tests revealing elements inside position:fixed after zooming.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Reveal "left-target"
+PASS document.scrollingElement.scrollTop is 838
+PASS document.scrollingElement.scrollLeft is 40
+
+Reveal "bottom-target"
+PASS document.scrollingElement.scrollTop is 1048
+PASS document.scrollingElement.scrollLeft is 40
+
+Reveal "right-target"
+PASS document.scrollingElement.scrollTop is 1086
+PASS document.scrollingElement.scrollLeft is 333
+
+Reveal "top-target"
+PASS document.scrollingElement.scrollTop is 834
+PASS document.scrollingElement.scrollLeft is 230
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/visual-viewport/zoomed-scroll-into-view-fixed.html b/LayoutTests/fast/visual-viewport/zoomed-scroll-into-view-fixed.html
new file mode 100644 (file)
index 0000000..a6cb54b
--- /dev/null
@@ -0,0 +1,120 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        body {
+            height: 2000px;
+            width: 2000px;
+        }
+        
+        .fixed {
+            position: fixed;
+            top: 50px;
+            left: 40px;
+            height: 200px;
+            width: 200px;
+            background-color: rgba(0, 0, 0, 0.3);
+        }
+        
+        .fixed > div {
+            background-color: blue;
+            width: 20px;
+            height: 10px;
+            margin: 30px;
+        }
+        
+        .left, .right {
+            top: 500px;
+            width: 100px;
+        }
+
+        .top, .bottom {
+            left: 200px;
+            height: 100px;
+        }
+        
+        .left {
+            top: 300px;
+            left: 10px;
+        }
+
+        .right {
+            top: 300px;
+            left: auto;
+            right: 10px;
+        }
+
+        .top {
+            top: 11px;
+        }
+
+        .bottom {
+            top: auto;
+            bottom: 12px;
+        }
+    </style>
+    <script src="../../resources/js-test-pre.js"></script>
+    <script>
+
+    if (window.internals)
+        internals.settings.setVisualViewportEnabled(true);
+
+    description("Tests revealing elements inside position:fixed after zooming.");
+
+    window.jsTestIsAsync = true;
+
+    function runTest()
+    {
+        if (window.eventSender)
+            eventSender.scalePageBy(2);
+
+        window.scrollTo(300, 800);
+
+        debug('Reveal "left-target"');
+        document.getElementById('left-target').scrollIntoView();
+        shouldBe('document.scrollingElement.scrollTop', '838');
+        shouldBe('document.scrollingElement.scrollLeft', '40');
+
+        debug('');
+        debug('Reveal "bottom-target"');
+        document.getElementById('bottom-target').scrollIntoView();
+        shouldBe('document.scrollingElement.scrollTop', '1048');
+        shouldBe('document.scrollingElement.scrollLeft', '40');
+
+        debug('');
+        debug('Reveal "right-target"');
+        document.getElementById('right-target').scrollIntoView();
+        shouldBe('document.scrollingElement.scrollTop', '1086');
+        shouldBe('document.scrollingElement.scrollLeft', '333');
+
+        debug('');
+        debug('Reveal "top-target"');
+        document.getElementById('top-target').scrollIntoView();
+        shouldBe('document.scrollingElement.scrollTop', '834');
+        shouldBe('document.scrollingElement.scrollLeft', '230');
+        
+        finishJSTest();
+    }
+    </script>
+</head>
+<body onload="runTest()">
+
+<div class="left fixed">
+    <div id="left-target"></div>
+</div>
+
+<div class="right fixed">
+    <div id="right-target"></div>
+</div>
+
+<div class="top fixed">
+    <div id="top-target"></div>
+</div>
+
+<div class="bottom fixed">
+    <div id="bottom-target"></div>
+</div>
+
+<script src="../../resources/js-test-post.js"></script>
+
+</body></html>
diff --git a/LayoutTests/fast/visual-viewport/zoomed-scroll-to-anchor-in-position-fixed-expected.txt b/LayoutTests/fast/visual-viewport/zoomed-scroll-to-anchor-in-position-fixed-expected.txt
new file mode 100644 (file)
index 0000000..48670cb
--- /dev/null
@@ -0,0 +1,11 @@
+Tests scrolling to an anchor inside position:fixed after zooming doesn't try to scroll the page
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS document.scrollingElement.scrollTop is 559
+PASS document.scrollingElement.scrollLeft is 41
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Anchor is here
diff --git a/LayoutTests/fast/visual-viewport/zoomed-scroll-to-anchor-in-position-fixed.html b/LayoutTests/fast/visual-viewport/zoomed-scroll-to-anchor-in-position-fixed.html
new file mode 100644 (file)
index 0000000..ba24b91
--- /dev/null
@@ -0,0 +1,62 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        body {
+            height: 2000px;
+            width: 2000px;
+        }
+        
+        .fixed {
+            position: fixed;
+            top: 50px;
+            left: 40px;
+            border: 1px solid black;
+        }
+    </style>
+    <script src="../../resources/js-test-pre.js"></script>
+    <script>
+
+    if (window.internals)
+        internals.settings.setVisualViewportEnabled(true);
+
+    description("Tests scrolling to an anchor inside position:fixed after zooming doesn't try to scroll the page");
+
+    window.jsTestIsAsync = true;
+
+    function runTest()
+    {
+        if (window.eventSender)
+            eventSender.scalePageBy(2);
+
+        window.scrollTo(300, 800);
+
+        setTimeout(function() {
+            window.location='#anchor';
+            setTimeout(finishTest, 0);
+        }, 0);
+    }
+
+    function finishTest()
+    {
+        if (window.location.toString().indexOf("#") == -1) {
+            setTimeout(finishTest, 0);
+            return;
+        }
+        
+        shouldBe('document.scrollingElement.scrollTop', '559');
+        shouldBe('document.scrollingElement.scrollLeft', '41');
+
+        finishJSTest();
+    }
+    </script>
+</head>
+<body onload="runTest()">
+
+<div class="fixed">
+    <a name="anchor">Anchor is here</a>
+</div>
+
+<script src="../../resources/js-test-post.js"></script>
+
+</body></html>
index f1bf501..f51ada4 100644 (file)
@@ -2767,3 +2767,5 @@ webkit.org/b/163046 js/regress-141098.html [ Pass Failure ]
 
 # Test relies on window.scrollTo
 fast/zooming/client-rect-in-fixed-zoomed.html [ Skip ]
+fast/visual-viewport/zoomed-scroll-to-anchor-in-position-fixed.html [ Skip ]
+fast/visual-viewport/zoomed-scroll-into-view-fixed.html [ Skip ]
index e05fbe5..457672b 100644 (file)
@@ -1,3 +1,68 @@
+2016-12-02  Simon Fraser  <simon.fraser@apple.com>
+
+        Improve the behavior of scroll-into-view when the target is inside position:fixed
+        https://bugs.webkit.org/show_bug.cgi?id=165354
+
+        Reviewed by Zalan Bujtas.
+        
+        The existing RenderLayer::scrollRectToVisible() code paid no heed to whether the 
+        target was inside position:fixed, resulting in unwanted scrolls.
+        
+        Fix this by plumbing through from the call sites a "insideFixed" flag which we get
+        when we call localToAbsolute(), and use this flag to avoid scrolling at all if
+        unzoomed.
+        
+        If zoomed and we're focussing something inside position:fixed, and if visual viewports
+        are enabled, we can compute the visual viewport required to reveal the target rect,
+        which gives us the ideal scroll position.
+        
+        Fix a bug on non-iOS platforms when zoomed, which is to scale the viewRect since
+        frameView.visibleContentRect() gives an unscaled rect on those platforms.
+        
+        Not all callers of scrollRectToVisible() are fixed, but those that are not will get
+        the current behavior.
+
+        Tests: fast/overflow/scroll-anchor-in-position-fixed.html
+               fast/visual-viewport/zoomed-scroll-into-view-fixed.html
+               fast/visual-viewport/zoomed-scroll-to-anchor-in-position-fixed.html
+
+        * dom/Element.cpp:
+        (WebCore::Element::scrollIntoView):
+        (WebCore::Element::scrollIntoViewIfNeeded):
+        (WebCore::Element::scrollIntoViewIfNotVisible):
+        (WebCore::Element::updateFocusAppearance):
+        * editing/FrameSelection.cpp:
+        (WebCore::FrameSelection::FrameSelection):
+        (WebCore::FrameSelection::absoluteCaretBounds):
+        (WebCore::FrameSelection::recomputeCaretRect):
+        (WebCore::FrameSelection::revealSelection):
+        * editing/FrameSelection.h:
+        * editing/VisiblePosition.cpp:
+        (WebCore::VisiblePosition::absoluteCaretBounds):
+        * editing/VisiblePosition.h:
+        * editing/htmlediting.cpp:
+        (WebCore::absoluteBoundsForLocalCaretRect):
+        * editing/htmlediting.h:
+        * page/FrameView.cpp:
+        (WebCore::FrameView::scrollElementToRect):
+        (WebCore::FrameView::scrollToAnchor):
+        * page/PrintContext.cpp:
+        (WebCore::PrintContext::outputLinkedDestinations):
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::getLeadingCorner):
+        (WebCore::RenderElement::getTrailingCorner):
+        (WebCore::RenderElement::absoluteAnchorRect):
+        (WebCore::RenderElement::anchorRect): Renamed to absoluteAnchorRect().
+        * rendering/RenderElement.h:
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::scrollRectToVisible):
+        (WebCore::RenderLayer::getRectToExpose):
+        (WebCore::RenderLayer::autoscroll):
+        * rendering/RenderLayer.h:
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::scrollRectToVisible):
+        * rendering/RenderObject.h:
+
 2016-11-30  Simon Fraser  <simon.fraser@apple.com>
 
         localToAbsolute() does incorrect conversion for elements inside position:fixed with zooming
index d8eaa22..4f46916 100644 (file)
@@ -649,12 +649,13 @@ void Element::scrollIntoView(bool alignToTop)
     if (!renderer())
         return;
 
-    LayoutRect bounds = renderer()->anchorRect();
+    bool insideFixed;
+    LayoutRect absoluteBounds = renderer()->absoluteAnchorRect(&insideFixed);
     // Align to the top / bottom and to the closest edge.
     if (alignToTop)
-        renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, bounds, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignTopAlways);
+        renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, absoluteBounds, insideFixed, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignTopAlways);
     else
-        renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, bounds, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignBottomAlways);
+        renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, absoluteBounds, insideFixed, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignBottomAlways);
 }
 
 void Element::scrollIntoViewIfNeeded(bool centerIfNeeded)
@@ -664,11 +665,12 @@ void Element::scrollIntoViewIfNeeded(bool centerIfNeeded)
     if (!renderer())
         return;
 
-    LayoutRect bounds = renderer()->anchorRect();
+    bool insideFixed;
+    LayoutRect absoluteBounds = renderer()->absoluteAnchorRect(&insideFixed);
     if (centerIfNeeded)
-        renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, bounds, ScrollAlignment::alignCenterIfNeeded, ScrollAlignment::alignCenterIfNeeded);
+        renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, absoluteBounds, insideFixed, ScrollAlignment::alignCenterIfNeeded, ScrollAlignment::alignCenterIfNeeded);
     else
-        renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, bounds, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded);
+        renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, absoluteBounds, insideFixed, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded);
 }
 
 void Element::scrollIntoViewIfNotVisible(bool centerIfNotVisible)
@@ -678,11 +680,12 @@ void Element::scrollIntoViewIfNotVisible(bool centerIfNotVisible)
     if (!renderer())
         return;
     
-    LayoutRect bounds = renderer()->anchorRect();
+    bool insideFixed;
+    LayoutRect absoluteBounds = renderer()->absoluteAnchorRect(&insideFixed);
     if (centerIfNotVisible)
-        renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, bounds, ScrollAlignment::alignCenterIfNotVisible, ScrollAlignment::alignCenterIfNotVisible);
+        renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, absoluteBounds, insideFixed, ScrollAlignment::alignCenterIfNotVisible, ScrollAlignment::alignCenterIfNotVisible);
     else
-        renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, bounds, ScrollAlignment::alignToEdgeIfNotVisible, ScrollAlignment::alignToEdgeIfNotVisible);
+        renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, absoluteBounds, insideFixed, ScrollAlignment::alignToEdgeIfNotVisible, ScrollAlignment::alignToEdgeIfNotVisible);
 }
 
 void Element::scrollBy(const ScrollToOptions& options)
@@ -2426,8 +2429,11 @@ void Element::updateFocusAppearance(SelectionRestorationMode, SelectionRevealMod
             frame->selection().setSelection(newSelection, FrameSelection::defaultSetSelectionOptions(), Element::defaultFocusTextStateChangeIntent());
             frame->selection().revealSelection(revealMode);
         }
-    } else if (renderer() && !renderer()->isWidget())
-        renderer()->scrollRectToVisible(revealMode, renderer()->anchorRect());
+    } else if (renderer() && !renderer()->isWidget()) {
+        bool insideFixed;
+        LayoutRect absoluteBounds = renderer()->absoluteAnchorRect(&insideFixed);
+        renderer()->scrollRectToVisible(revealMode, absoluteBounds, insideFixed);
+    }
 }
 
 void Element::blur()
index 3561749..6759ff1 100644 (file)
@@ -112,6 +112,7 @@ FrameSelection::FrameSelection(Frame* frame)
     , m_xPosForVerticalArrowNavigation(NoXPosForVerticalArrowNavigation())
     , m_granularity(CharacterGranularity)
     , m_caretBlinkTimer(*this, &FrameSelection::caretBlinkTimerFired)
+    , m_caretInsidePositionFixed(false)
     , m_absCaretBoundsDirty(true)
     , m_caretPaint(true)
     , m_isCaretBlinkingSuspended(false)
@@ -1577,12 +1578,14 @@ static bool isNonOrphanedCaret(const VisibleSelection& selection)
     return selection.isCaret() && !selection.start().isOrphan() && !selection.end().isOrphan();
 }
 
-IntRect FrameSelection::absoluteCaretBounds()
+IntRect FrameSelection::absoluteCaretBounds(bool* insideFixed)
 {
     if (!m_frame)
         return IntRect();
     updateSelectionByUpdatingLayoutOrStyle(*m_frame);
     recomputeCaretRect();
+    if (insideFixed)
+        *insideFixed = m_caretInsidePositionFixed;
     return m_absCaretBounds;
 }
 
@@ -1624,7 +1627,9 @@ bool FrameSelection::recomputeCaretRect()
         return false;
 
     IntRect oldAbsCaretBounds = m_absCaretBounds;
-    m_absCaretBounds = absoluteBoundsForLocalCaretRect(rendererForCaretPainting(caretNode.get()), newRect);
+    bool isInsideFixed;
+    m_absCaretBounds = absoluteBoundsForLocalCaretRect(rendererForCaretPainting(caretNode.get()), newRect, &isInsideFixed);
+    m_caretInsidePositionFixed = isInsideFixed;
 
     if (m_absCaretBoundsDirty && m_selection.isCaret()) // We should be able to always assert this condition.
         ASSERT(m_absCaretBounds == m_selection.visibleStart().absoluteCaretBounds());
@@ -2300,12 +2305,12 @@ void FrameSelection::revealSelection(SelectionRevealMode revealMode, const Scrol
         return;
 
     LayoutRect rect;
-
+    bool insideFixed = false;
     switch (m_selection.selectionType()) {
     case VisibleSelection::NoSelection:
         return;
     case VisibleSelection::CaretSelection:
-        rect = absoluteCaretBounds();
+        rect = absoluteCaretBounds(&insideFixed);
         break;
     case VisibleSelection::RangeSelection:
         rect = revealExtentOption == RevealExtent ? VisiblePosition(m_selection.extent()).absoluteCaretBounds() : enclosingIntRect(selectionBounds(false));
@@ -2319,7 +2324,7 @@ void FrameSelection::revealSelection(SelectionRevealMode revealMode, const Scrol
         if (RenderLayer* layer = start.deprecatedNode()->renderer()->enclosingLayer()) {
             if (!m_scrollingSuppressCount) {
                 layer->setAdjustForIOSCaretWhenScrolling(true);
-                layer->scrollRectToVisible(revealMode, rect, alignment, alignment);
+                layer->scrollRectToVisible(revealMode, rect, insideFixed, alignment, alignment);
                 layer->setAdjustForIOSCaretWhenScrolling(false);
                 updateAppearance();
                 if (m_frame->page())
@@ -2330,7 +2335,7 @@ void FrameSelection::revealSelection(SelectionRevealMode revealMode, const Scrol
         // FIXME: This code only handles scrolling the startContainer's layer, but
         // the selection rect could intersect more than just that.
         // See <rdar://problem/4799899>.
-        if (start.deprecatedNode()->renderer()->scrollRectToVisible(revealMode, rect, alignment, alignment))
+        if (start.deprecatedNode()->renderer()->scrollRectToVisible(revealMode, rect, insideFixed, alignment, alignment))
             updateAppearance();
 #endif
     }
index 150473a..db28ce6 100644 (file)
@@ -173,7 +173,7 @@ public:
     RenderBlock* caretRendererWithoutUpdatingLayout() const;
 
     // Bounds of (possibly transformed) caret in absolute coords
-    WEBCORE_EXPORT IntRect absoluteCaretBounds();
+    WEBCORE_EXPORT IntRect absoluteCaretBounds(bool* insideFixed = nullptr);
     void setCaretRectNeedsUpdate() { CaretBase::setCaretRectNeedsUpdate(); }
 
     void willBeModified(EAlteration, SelectionDirection);
@@ -336,6 +336,7 @@ private:
     Timer m_caretBlinkTimer;
     // The painted bounds of the caret in absolute coordinates
     IntRect m_absCaretBounds;
+    bool m_caretInsidePositionFixed : 1;
     bool m_absCaretBoundsDirty : 1;
     bool m_caretPaint : 1;
     bool m_isCaretBlinkingSuspended : 1;
index 819a6d3..bda4a14 100644 (file)
@@ -660,11 +660,11 @@ LayoutRect VisiblePosition::localCaretRect(RenderObject*& renderer) const
     return renderer->localCaretRect(inlineBox, caretOffset);
 }
 
-IntRect VisiblePosition::absoluteCaretBounds() const
+IntRect VisiblePosition::absoluteCaretBounds(bool* insideFixed) const
 {
     RenderBlock* renderer = nullptr;
     LayoutRect localRect = localCaretRectInRendererForCaretPainting(*this, renderer);
-    return absoluteBoundsForLocalCaretRect(renderer, localRect);
+    return absoluteBoundsForLocalCaretRect(renderer, localRect, insideFixed);
 }
 
 int VisiblePosition::lineDirectionPointForBlockDirectionNavigation() const
index 4094d84..3b92735 100644 (file)
@@ -95,7 +95,7 @@ public:
     // Rect is local to the returned renderer
     WEBCORE_EXPORT LayoutRect localCaretRect(RenderObject*&) const;
     // Bounds of (possibly transformed) caret in absolute coords
-    WEBCORE_EXPORT IntRect absoluteCaretBounds() const;
+    WEBCORE_EXPORT IntRect absoluteCaretBounds(bool* insideFixed = nullptr) const;
     // Abs x/y position of the caret ignoring transforms.
     // FIXME: navigation with transforms should be smarter.
     WEBCORE_EXPORT int lineDirectionPointForBlockDirectionNavigation() const;
index 5d85573..a7befcd 100644 (file)
@@ -1290,14 +1290,14 @@ LayoutRect localCaretRectInRendererForRect(LayoutRect& localRect, Node* node, Re
     return localRect;
 }
 
-IntRect absoluteBoundsForLocalCaretRect(RenderBlock* rendererForCaretPainting, const LayoutRect& rect)
+IntRect absoluteBoundsForLocalCaretRect(RenderBlock* rendererForCaretPainting, const LayoutRect& rect, bool* insideFixed)
 {
     if (!rendererForCaretPainting || rect.isEmpty())
         return IntRect();
 
     LayoutRect localRect(rect);
     rendererForCaretPainting->flipForWritingMode(localRect);
-    return rendererForCaretPainting->localToAbsoluteQuad(FloatRect(localRect)).enclosingBoundingBox();
+    return rendererForCaretPainting->localToAbsoluteQuad(FloatRect(localRect), UseTransforms, insideFixed).enclosingBoundingBox();
 }
 
 } // namespace WebCore
index d8a23dd..5bf936a 100644 (file)
@@ -200,7 +200,7 @@ const String& nonBreakingSpaceString();
 RenderBlock* rendererForCaretPainting(Node*);
 LayoutRect localCaretRectInRendererForCaretPainting(const VisiblePosition&, RenderBlock*&);
 LayoutRect localCaretRectInRendererForRect(LayoutRect&, Node*, RenderObject*, RenderBlock*&);
-IntRect absoluteBoundsForLocalCaretRect(RenderBlock* rendererForCaretPainting, const LayoutRect&);
+IntRect absoluteBoundsForLocalCaretRect(RenderBlock* rendererForCaretPainting, const LayoutRect&, bool* insideFixed = nullptr);
 
 // -------------------------------------------------------------------------
 
index 59096dd..8061b75 100644 (file)
@@ -2351,7 +2351,7 @@ void FrameView::scrollElementToRect(const Element& element, const IntRect& rect)
 
     LayoutRect bounds;
     if (RenderElement* renderer = element.renderer())
-        bounds = renderer->anchorRect();
+        bounds = renderer->absoluteAnchorRect();
     int centeringOffsetX = (rect.width() - bounds.width()) / 2;
     int centeringOffsetY = (rect.height() - bounds.height()) / 2;
     setScrollPosition(IntPoint(bounds.x() - centeringOffsetX - rect.x(), bounds.y() - centeringOffsetY - rect.y()));
@@ -3271,17 +3271,18 @@ void FrameView::scrollToAnchor()
         return;
 
     LayoutRect rect;
+    bool insideFixed = false;
     if (anchorNode != frame().document() && anchorNode->renderer())
-        rect = anchorNode->renderer()->anchorRect();
+        rect = anchorNode->renderer()->absoluteAnchorRect(&insideFixed);
 
     // Scroll nested layers and frames to reveal the anchor.
     // Align to the top and to the closest side (this matches other browsers).
     if (anchorNode->renderer()->style().isHorizontalWritingMode())
-        anchorNode->renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, rect, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignTopAlways);
+        anchorNode->renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, rect, insideFixed, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignTopAlways);
     else if (anchorNode->renderer()->style().isFlippedBlocksWritingMode())
-        anchorNode->renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, rect, ScrollAlignment::alignRightAlways, ScrollAlignment::alignToEdgeIfNeeded);
+        anchorNode->renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, rect, insideFixed, ScrollAlignment::alignRightAlways, ScrollAlignment::alignToEdgeIfNeeded);
     else
-        anchorNode->renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, rect, ScrollAlignment::alignLeftAlways, ScrollAlignment::alignToEdgeIfNeeded);
+        anchorNode->renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, rect, insideFixed, ScrollAlignment::alignLeftAlways, ScrollAlignment::alignToEdgeIfNeeded);
 
     if (AXObjectCache* cache = frame().document()->existingAXObjectCache())
         cache->handleScrolledToAnchor(anchorNode.get());
index 38ca683..3176ebd 100644 (file)
@@ -274,7 +274,7 @@ void PrintContext::outputLinkedDestinations(GraphicsContext& graphicsContext, Do
         if (!renderer)
             continue;
 
-        FloatPoint point = renderer->anchorRect().minXMinYCorner();
+        FloatPoint point = renderer->absoluteAnchorRect().minXMinYCorner();
         point.expandedTo(FloatPoint());
 
         if (!pageRect.contains(roundedIntPoint(point)))
index fc37d77..9144810 100644 (file)
@@ -1625,10 +1625,10 @@ Color RenderElement::selectionBackgroundColor() const
     return theme().inactiveSelectionBackgroundColor();
 }
 
-bool RenderElement::getLeadingCorner(FloatPoint& point) const
+bool RenderElement::getLeadingCorner(FloatPoint& point, bool& insideFixed) const
 {
     if (!isInline() || isReplaced()) {
-        point = localToAbsolute(FloatPoint(), UseTransforms);
+        point = localToAbsolute(FloatPoint(), UseTransforms, &insideFixed);
         return true;
     }
 
@@ -1654,7 +1654,7 @@ bool RenderElement::getLeadingCorner(FloatPoint& point) const
         ASSERT(o);
 
         if (!o->isInline() || o->isReplaced()) {
-            point = o->localToAbsolute(FloatPoint(), UseTransforms);
+            point = o->localToAbsolute(FloatPoint(), UseTransforms, &insideFixed);
             return true;
         }
 
@@ -1666,7 +1666,7 @@ bool RenderElement::getLeadingCorner(FloatPoint& point) const
                 point.move(downcast<RenderText>(*o).linesBoundingBox().x(), downcast<RenderText>(*o).topOfFirstText());
             else if (is<RenderBox>(*o))
                 point.moveBy(downcast<RenderBox>(*o).location());
-            point = o->container()->localToAbsolute(point, UseTransforms);
+            point = o->container()->localToAbsolute(point, UseTransforms, &insideFixed);
             return true;
         }
     }
@@ -1680,10 +1680,10 @@ bool RenderElement::getLeadingCorner(FloatPoint& point) const
     return false;
 }
 
-bool RenderElement::getTrailingCorner(FloatPoint& point) const
+bool RenderElement::getTrailingCorner(FloatPoint& point, bool& insideFixed) const
 {
     if (!isInline() || isReplaced()) {
-        point = localToAbsolute(LayoutPoint(downcast<RenderBox>(*this).size()), UseTransforms);
+        point = localToAbsolute(LayoutPoint(downcast<RenderBox>(*this).size()), UseTransforms, &insideFixed);
         return true;
     }
 
@@ -1714,18 +1714,20 @@ bool RenderElement::getTrailingCorner(FloatPoint& point) const
                 point.moveBy(linesBox.maxXMaxYCorner());
             } else
                 point.moveBy(downcast<RenderBox>(*o).frameRect().maxXMaxYCorner());
-            point = o->container()->localToAbsolute(point, UseTransforms);
+            point = o->container()->localToAbsolute(point, UseTransforms, &insideFixed);
             return true;
         }
     }
     return true;
 }
 
-LayoutRect RenderElement::anchorRect() const
+LayoutRect RenderElement::absoluteAnchorRect(bool* insideFixed) const
 {
     FloatPoint leading, trailing;
-    getLeadingCorner(leading);
-    getTrailingCorner(trailing);
+    bool leadingInFixed = false;
+    bool trailingInFixed = false;
+    getLeadingCorner(leading, leadingInFixed);
+    getTrailingCorner(trailing, trailingInFixed);
 
     FloatPoint upperLeft = leading;
     FloatPoint lowerRight = trailing;
@@ -1736,6 +1738,11 @@ LayoutRect RenderElement::anchorRect() const
         lowerRight = FloatPoint(std::max(leading.x(), trailing.x()), std::max(leading.y(), trailing.y()));
     } // Otherwise, it's not obvious what to do.
 
+    if (insideFixed) {
+        // For now, just look at the leading corner. Handling one inside fixed and one not would be tricky.
+        *insideFixed = leadingInFixed;
+    }
+
     return enclosingLayoutRect(FloatRect(upperLeft, lowerRight.expandedTo(upperLeft) - upperLeft));
 }
 
index 230c1c7..e1c693b 100644 (file)
@@ -164,7 +164,7 @@ public:
     // anchorRect() is conceptually similar to absoluteBoundingBoxRect(), but is intended for scrolling to an anchor.
     // For inline renderers, this gets the logical top left of the first leaf child and the logical bottom right of the
     // last leaf child, converts them to absolute coordinates, and makes a box out of them.
-    LayoutRect anchorRect() const;
+    LayoutRect absoluteAnchorRect(bool* insideFixed = nullptr) const;
 
     bool hasFilter() const { return style().hasFilter(); }
     bool hasBackdropFilter() const
@@ -307,8 +307,8 @@ private:
 
     void newImageAnimationFrameAvailable(CachedImage&) final;
 
-    bool getLeadingCorner(FloatPoint& output) const;
-    bool getTrailingCorner(FloatPoint& output) const;
+    bool getLeadingCorner(FloatPoint& output, bool& insideFixed) const;
+    bool getTrailingCorner(FloatPoint& output, bool& insideFixed) const;
 
     void clearLayoutRootIfNeeded() const;
     
index 26a2e77..ef7075d 100644 (file)
@@ -2506,12 +2506,12 @@ bool RenderLayer::allowsCurrentScroll() const
     return box->hasHorizontalOverflow() || box->hasVerticalOverflow();
 }
 
-void RenderLayer::scrollRectToVisible(SelectionRevealMode revealMode, const LayoutRect& rect, const ScrollAlignment& alignX, const ScrollAlignment& alignY)
+void RenderLayer::scrollRectToVisible(SelectionRevealMode revealMode, const LayoutRect& absoluteRect, bool insideFixed, const ScrollAlignment& alignX, const ScrollAlignment& alignY)
 {
-    LOG_WITH_STREAM(Scrolling, stream << "Layer " << this << " scrollRectToVisible " << rect);
+    LOG_WITH_STREAM(Scrolling, stream << "Layer " << this << " scrollRectToVisible " << absoluteRect);
 
     RenderLayer* parentLayer = nullptr;
-    LayoutRect newRect = rect;
+    LayoutRect newRect = absoluteRect;
 
     // We may end up propagating a scroll event. It is important that we suspend events until 
     // the end of the function since they could delete the layer or the layer's renderer().
@@ -2525,11 +2525,11 @@ void RenderLayer::scrollRectToVisible(SelectionRevealMode revealMode, const Layo
         // This will prevent us from revealing text hidden by the slider in Safari RSS.
         RenderBox* box = renderBox();
         ASSERT(box);
-        LayoutRect localExposeRect(box->absoluteToLocalQuad(FloatQuad(FloatRect(rect))).boundingBox());
+        LayoutRect localExposeRect(box->absoluteToLocalQuad(FloatQuad(FloatRect(absoluteRect))).boundingBox());
         LayoutRect layerBounds(0, 0, box->clientWidth(), box->clientHeight());
-        LayoutRect r = getRectToExpose(layerBounds, layerBounds, localExposeRect, alignX, alignY);
+        LayoutRect revealRect = getRectToExpose(layerBounds, layerBounds, localExposeRect, insideFixed, alignX, alignY);
 
-        ScrollOffset clampedScrollOffset = clampScrollOffset(scrollOffset() + toIntSize(roundedIntRect(r).location()));
+        ScrollOffset clampedScrollOffset = clampScrollOffset(scrollOffset() + toIntSize(roundedIntRect(revealRect).location()));
         if (clampedScrollOffset != scrollOffset()) {
             ScrollOffset oldScrollOffset = scrollOffset();
             scrollToOffset(clampedScrollOffset);
@@ -2551,7 +2551,7 @@ void RenderLayer::scrollRectToVisible(SelectionRevealMode revealMode, const Layo
                 NoEventDispatchAssertion assertNoEventDispatch;
 
                 LayoutRect viewRect = frameView.visibleContentRect(LegacyIOSDocumentVisibleRect);
-                LayoutRect exposeRect = getRectToExpose(viewRect, viewRect, rect, alignX, alignY);
+                LayoutRect exposeRect = getRectToExpose(viewRect, viewRect, absoluteRect, insideFixed, alignX, alignY);
 
                 IntPoint scrollOffset(roundedIntPoint(exposeRect.location()));
                 // Adjust offsets if they're outside of the allowable range.
@@ -2562,6 +2562,7 @@ void RenderLayer::scrollRectToVisible(SelectionRevealMode revealMode, const Layo
                     parentLayer = ownerElement->renderer()->enclosingLayer();
                     // Convert the rect into the coordinate space of the parent frame's document.
                     newRect = frameView.contentsToContainingViewContents(enclosingIntRect(newRect));
+                    insideFixed = false; // FIXME: ideally need to determine if this <iframe> is inside position:fixed.
                 } else
                     parentLayer = nullptr;
             }
@@ -2571,16 +2572,17 @@ void RenderLayer::scrollRectToVisible(SelectionRevealMode revealMode, const Layo
 
 #if !PLATFORM(IOS)
             LayoutRect viewRect = frameView.visibleContentRect();
+            viewRect.scale(1 / frameView.frameScaleFactor());
+
             LayoutRect visibleRectRelativeToDocument = viewRect;
             visibleRectRelativeToDocument.setLocation(frameView.documentScrollPositionRelativeToScrollableAreaOrigin());
 #else
             LayoutRect viewRect = frameView.unobscuredContentRect();
             LayoutRect visibleRectRelativeToDocument = viewRect;
 #endif
-
-            LayoutRect r = getRectToExpose(viewRect, visibleRectRelativeToDocument, rect, alignX, alignY);
+            LayoutRect revealRect = getRectToExpose(viewRect, visibleRectRelativeToDocument, absoluteRect, insideFixed, alignX, alignY);
                 
-            frameView.setScrollPosition(roundedIntPoint(r.location()));
+            frameView.setScrollPosition(roundedIntPoint(revealRect.location()));
 
             // This is the outermost view of a web page, so after scrolling this view we
             // scroll its container by calling Page::scrollRectIntoView.
@@ -2588,12 +2590,12 @@ void RenderLayer::scrollRectToVisible(SelectionRevealMode revealMode, const Layo
             // that put web views into scrolling containers, such as Mac OS X Mail.
             // The canAutoscroll function in EventHandler also knows about this.
             if (Page* page = frameView.frame().page())
-                page->chrome().scrollRectIntoView(snappedIntRect(rect));
+                page->chrome().scrollRectIntoView(snappedIntRect(absoluteRect));
         }
     }
     
     if (parentLayer)
-        parentLayer->scrollRectToVisible(revealMode, newRect, alignX, alignY);
+        parentLayer->scrollRectToVisible(revealMode, newRect, insideFixed, alignX, alignY);
 }
 
 void RenderLayer::updateCompositingLayersAfterScroll()
@@ -2610,8 +2612,35 @@ void RenderLayer::updateCompositingLayersAfterScroll()
     }
 }
 
-LayoutRect RenderLayer::getRectToExpose(const LayoutRect &visibleRect, const LayoutRect &visibleRectRelativeToDocument, const LayoutRect &exposeRect, const ScrollAlignment& alignX, const ScrollAlignment& alignY)
+LayoutRect RenderLayer::getRectToExpose(const LayoutRect &visibleRect, const LayoutRect &visibleRectRelativeToDocument, const LayoutRect &exposeRect, bool insideFixed, const ScrollAlignment& alignX, const ScrollAlignment& alignY)
 {
+    FrameView& frameView = renderer().view().frameView();
+    if (insideFixed) {
+        // If the element is inside position:fixed and we're not scaled, no amount of scrolling is going to move things around.
+        if (frameView.frameScaleFactor() == 1)
+            return visibleRect;
+
+        if (frameView.frame().settings().visualViewportEnabled()) {
+            // exposeRect is in absolute coords, affected by page scale. Unscale it.
+            LayoutRect unscaledExposeRect = exposeRect;
+            unscaledExposeRect.scale(1 / frameView.frameScaleFactor());
+            // These are both in unscaled coordinates.
+            LayoutRect layoutViewport = frameView.layoutViewportRect();
+            LayoutRect visualViewport = frameView.visualViewportRect();
+
+            // The rect to expose may be partially offscreen, which we can't do anything about with position:fixed.
+            unscaledExposeRect.intersect(layoutViewport);
+            // Make sure it's not larger than the visual viewport; if so, we'll just move to the top left.
+            unscaledExposeRect.setSize(unscaledExposeRect.size().shrunkTo(visualViewport.size()));
+
+            // Compute how much we have to move the visualViewport to reveal the part of the layoutViewport that contains exposeRect.
+            LayoutRect requiredVisualViewport = getRectToExpose(visualViewport, visualViewport, unscaledExposeRect, false, alignX, alignY);
+            // Scale it back up.
+            requiredVisualViewport.scale(frameView.frameScaleFactor());
+            return requiredVisualViewport;
+        }
+    }
+
     // Determine the appropriate X behavior.
     ScrollAlignment::Behavior scrollX;
     LayoutRect exposeRectX(exposeRect.x(), visibleRect.y(), exposeRect.width(), visibleRect.height());
@@ -2686,7 +2715,7 @@ LayoutRect RenderLayer::getRectToExpose(const LayoutRect &visibleRect, const Lay
 void RenderLayer::autoscroll(const IntPoint& position)
 {
     IntPoint currentDocumentPosition = renderer().view().frameView().windowToContents(position);
-    scrollRectToVisible(SelectionRevealMode::Reveal, LayoutRect(currentDocumentPosition, LayoutSize(1, 1)), ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded);
+    scrollRectToVisible(SelectionRevealMode::Reveal, LayoutRect(currentDocumentPosition, LayoutSize(1, 1)), false, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded);
 }
 
 bool RenderLayer::canResize() const
index 3bcb57a..159aef6 100644 (file)
@@ -204,9 +204,9 @@ public:
 
     void availableContentSizeChanged(AvailableSizeChangeReason) override;
 
-    void scrollRectToVisible(SelectionRevealMode, const LayoutRect&, const ScrollAlignment& alignX, const ScrollAlignment& alignY);
+    void scrollRectToVisible(SelectionRevealMode, const LayoutRect& absoluteRect, bool insideFixed, const ScrollAlignment& alignX, const ScrollAlignment& alignY);
 
-    LayoutRect getRectToExpose(const LayoutRect& visibleRect, const LayoutRect& visibleRectRelativeToDocument, const LayoutRect& exposeRect, const ScrollAlignment& alignX, const ScrollAlignment& alignY);
+    LayoutRect getRectToExpose(const LayoutRect& visibleRect, const LayoutRect& visibleRectRelativeToDocument, const LayoutRect& exposeRect, bool insideFixed, const ScrollAlignment& alignX, const ScrollAlignment& alignY);
 
     bool scrollsOverflow() const;
     bool hasScrollbars() const { return m_hBar || m_vBar; }
index 61efd14..e3638a5 100644 (file)
@@ -396,7 +396,7 @@ RenderLayer* RenderObject::enclosingLayer() const
     return nullptr;
 }
 
-bool RenderObject::scrollRectToVisible(SelectionRevealMode revealMode, const LayoutRect& rect, const ScrollAlignment& alignX, const ScrollAlignment& alignY)
+bool RenderObject::scrollRectToVisible(SelectionRevealMode revealMode, const LayoutRect& absoluteRect, bool insideFixed, const ScrollAlignment& alignX, const ScrollAlignment& alignY)
 {
     if (revealMode == SelectionRevealMode::DoNotReveal)
         return false;
@@ -405,7 +405,7 @@ bool RenderObject::scrollRectToVisible(SelectionRevealMode revealMode, const Lay
     if (!enclosingLayer)
         return false;
 
-    enclosingLayer->scrollRectToVisible(revealMode, rect, alignX, alignY);
+    enclosingLayer->scrollRectToVisible(revealMode, absoluteRect, insideFixed, alignX, alignY);
     return true;
 }
 
index 88da92d..1a67271 100644 (file)
@@ -154,7 +154,7 @@ public:
     WEBCORE_EXPORT RenderLayer* enclosingLayer() const;
 
     // Scrolling is a RenderBox concept, however some code just cares about recursively scrolling our enclosing ScrollableArea(s).
-    WEBCORE_EXPORT bool scrollRectToVisible(SelectionRevealMode, const LayoutRect&, const ScrollAlignment& alignX = ScrollAlignment::alignCenterIfNeeded, const ScrollAlignment& alignY = ScrollAlignment::alignCenterIfNeeded);
+    WEBCORE_EXPORT bool scrollRectToVisible(SelectionRevealMode, const LayoutRect& absoluteRect, bool insideFixed, const ScrollAlignment& alignX = ScrollAlignment::alignCenterIfNeeded, const ScrollAlignment& alignY = ScrollAlignment::alignCenterIfNeeded);
 
     // Convenience function for getting to the nearest enclosing box of a RenderObject.
     WEBCORE_EXPORT RenderBox& enclosingBox() const;
index ae9d2dc..ec1592c 100644 (file)
@@ -1,3 +1,17 @@
+2016-12-02  Simon Fraser  <simon.fraser@apple.com>
+
+        Improve the behavior of scroll-into-view when the target is inside position:fixed
+        https://bugs.webkit.org/show_bug.cgi?id=165354
+
+        Reviewed by Zalan Bujtas.
+
+        Plumb through 'insideFixed'. We don't get compute it, so behavior from
+        these call sites won't change.
+
+        * WebView/WebFrame.mm:
+        (-[WebFrame _scrollDOMRangeToVisible:]):
+        (-[WebFrame _scrollDOMRangeToVisible:withInset:]):
+
 2016-12-02  Andy Estes  <aestes@apple.com>
 
         [Cocoa] Adopt the PRODUCT_BUNDLE_IDENTIFIER build setting
index 9fab5d5..9a80950 100644 (file)
@@ -711,17 +711,18 @@ static inline WebDataSource *dataSource(DocumentLoader* loader)
 
 - (void)_scrollDOMRangeToVisible:(DOMRange *)range
 {
+    bool insideFixed = false; // FIXME: get via firstRectForRange().
     NSRect rangeRect = [self _firstRectForDOMRange:range];    
     Node *startNode = core([range startContainer]);
         
     if (startNode && startNode->renderer()) {
 #if !PLATFORM(IOS)
-        startNode->renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, enclosingIntRect(rangeRect), ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded);
+        startNode->renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, enclosingIntRect(rangeRect), insideFixed, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded);
 #else
         RenderLayer* layer = startNode->renderer()->enclosingLayer();
         if (layer) {
             layer->setAdjustForIOSCaretWhenScrolling(true);
-            startNode->renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, enclosingIntRect(rangeRect), ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded);
+            startNode->renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, enclosingIntRect(rangeRect), insideFixed, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded);
             layer->setAdjustForIOSCaretWhenScrolling(false);
             _private->coreFrame->selection().setCaretRectNeedsUpdate();
             _private->coreFrame->selection().updateAppearance();
@@ -733,6 +734,7 @@ static inline WebDataSource *dataSource(DocumentLoader* loader)
 #if PLATFORM(IOS)
 - (void)_scrollDOMRangeToVisible:(DOMRange *)range withInset:(CGFloat)inset
 {
+    bool insideFixed = false; // FIXME: get via firstRectForRange().
     NSRect rangeRect = NSInsetRect([self _firstRectForDOMRange:range], inset, inset);
     Node *startNode = core([range startContainer]);
 
@@ -740,7 +742,7 @@ static inline WebDataSource *dataSource(DocumentLoader* loader)
         RenderLayer* layer = startNode->renderer()->enclosingLayer();
         if (layer) {
             layer->setAdjustForIOSCaretWhenScrolling(true);
-            startNode->renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, enclosingIntRect(rangeRect), ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded);
+            startNode->renderer()->scrollRectToVisible(SelectionRevealMode::Reveal, enclosingIntRect(rangeRect), insideFixed, ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded);
             layer->setAdjustForIOSCaretWhenScrolling(false);
 
             Frame *coreFrame = core(self);