Fix scrollRectToVisible in the presence of transforms
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Jan 2013 21:44:25 +0000 (21:44 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Jan 2013 21:44:25 +0000 (21:44 +0000)
https://bugs.webkit.org/show_bug.cgi?id=105574

Patch by Chris Hopman <cjhopman@google.com> on 2013-01-18
Reviewed by Simon Fraser.

Source/WebCore:

When scrolling to reveal an overflow layer, the required scroll was
being calculated in absolute coordinates. To properly account for
transforms, this calculation should be done in the local coordinates
of the renderBox.

Tests: editing/input/reveal-selection-transformed-overflow-parent.html
       editing/input/reveal-selection-transformed-textarea.html

* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::scrollRectToVisible):
When scrolling to reveal an overflow layer, calculate the required
scroll in the local coordinates of the RenderBox.
* rendering/RenderObject.cpp:
(WebCore::RenderObject::absoluteToLocalQuad):
(WebCore):
* rendering/RenderObject.h:
(RenderObject):
Add function to convert an absolute quad to a local quad.

LayoutTests:

* editing/input/reveal-caret-of-transformed-input-scrollable-parent.html: Added.
* editing/input/reveal-caret-of-transformed-input-scrollable-parent-expected.txt: Added.
Test that when scrolling an overflow layer to reveal a rect, the rect
passed to the parent to scroll is calculated properly.
* editing/input/reveal-caret-of-transformed-multiline-input.html: Added.
* editing/input/reveal-caret-of-transformed-multiline-input-expected.txt: Added.
Test that scrolling to reveal a rect works properly on a transformed
overflow layer.

* platform/chromium/TestExpectations:
* platform/mac/TestExpectations:

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

LayoutTests/ChangeLog
LayoutTests/editing/input/reveal-caret-of-transformed-input-scrollable-parent-expected.txt [new file with mode: 0644]
LayoutTests/editing/input/reveal-caret-of-transformed-input-scrollable-parent.html [new file with mode: 0644]
LayoutTests/editing/input/reveal-caret-of-transformed-multiline-input-expected.txt [new file with mode: 0644]
LayoutTests/editing/input/reveal-caret-of-transformed-multiline-input.html [new file with mode: 0644]
LayoutTests/platform/chromium/TestExpectations
LayoutTests/platform/mac/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderLayer.cpp
Source/WebCore/rendering/RenderObject.cpp
Source/WebCore/rendering/RenderObject.h

index 58befc7..6d31bd1 100644 (file)
@@ -1,3 +1,22 @@
+2013-01-18  Chris Hopman  <cjhopman@google.com>
+
+        Fix scrollRectToVisible in the presence of transforms
+        https://bugs.webkit.org/show_bug.cgi?id=105574
+
+        Reviewed by Simon Fraser.
+
+        * editing/input/reveal-caret-of-transformed-input-scrollable-parent.html: Added.
+        * editing/input/reveal-caret-of-transformed-input-scrollable-parent-expected.txt: Added.
+        Test that when scrolling an overflow layer to reveal a rect, the rect
+        passed to the parent to scroll is calculated properly.
+        * editing/input/reveal-caret-of-transformed-multiline-input.html: Added.
+        * editing/input/reveal-caret-of-transformed-multiline-input-expected.txt: Added.
+        Test that scrolling to reveal a rect works properly on a transformed
+        overflow layer.
+
+        * platform/chromium/TestExpectations:
+        * platform/mac/TestExpectations:
+
 2013-01-18  Julien Chaffraix  <jchaffraix@webkit.org>
 
         [CSS Grid Layout] Add support for min-content
diff --git a/LayoutTests/editing/input/reveal-caret-of-transformed-input-scrollable-parent-expected.txt b/LayoutTests/editing/input/reveal-caret-of-transformed-input-scrollable-parent-expected.txt
new file mode 100644 (file)
index 0000000..7ef22e9
--- /dev/null
@@ -0,0 +1 @@
+PASS
diff --git a/LayoutTests/editing/input/reveal-caret-of-transformed-input-scrollable-parent.html b/LayoutTests/editing/input/reveal-caret-of-transformed-input-scrollable-parent.html
new file mode 100644 (file)
index 0000000..ccd6092
--- /dev/null
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<head>
+<script type="text/javascript" src="resources/reveal-utilities.js"></script>
+</head>
+<body>
+<div>When the caret is scrolled out, on starting typing it must be brought to the center of the control.</div>
+<div style="overflow:auto; height:150px; width:300px">
+<textarea name="textarea" id="textarea" rows="10" cols="10"
+  style="-webkit-transform:translate(110px, 110px) scale(2); height:200px"></textarea>
+<div style="height:1000px"></div>
+</div>
+<script>
+
+var textArea = document.getElementById("textarea");
+textArea.textContent = generateNumbers(0, 500, 2, "\n");
+textArea.focus();
+textArea.selectionStart = 1200;
+if (window.eventSender) {
+    eventSender.keyDown(">");
+    // If the parentNode's scrollTop is greater than 500, then the textarea
+    // isn't even visible.
+    document.body.innerHTML = textarea.parentNode.scrollTop < 600 ? "PASS" : "FAIL";
+}
+
+if (window.testRunner) {
+    testRunner.dumpAsText();
+}
+
+</script>
+</body>
diff --git a/LayoutTests/editing/input/reveal-caret-of-transformed-multiline-input-expected.txt b/LayoutTests/editing/input/reveal-caret-of-transformed-multiline-input-expected.txt
new file mode 100644 (file)
index 0000000..7ef22e9
--- /dev/null
@@ -0,0 +1 @@
+PASS
diff --git a/LayoutTests/editing/input/reveal-caret-of-transformed-multiline-input.html b/LayoutTests/editing/input/reveal-caret-of-transformed-multiline-input.html
new file mode 100644 (file)
index 0000000..7e21890
--- /dev/null
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<head>
+<script type="text/javascript" src="resources/reveal-utilities.js"></script>
+</head>
+<body>
+<div>When the caret is scrolled out, on starting typing it must be brought to the center of the control.</div>
+<textarea name="textarea" id="textarea" rows="10" cols="10" style="-webkit-transform:rotate(180deg)"></textarea>
+<script>
+
+var textArea = document.getElementById("textarea");
+textArea.textContent = generateNumbers(0, 30, 2, "\n");
+textArea.focus();
+textArea.selectionStart = 36;
+if (window.eventSender) {
+    eventSender.keyDown(">");
+    document.body.innerHTML = textarea.scrollTop > 0 ? "PASS" : "FAIL";
+}
+
+if (window.testRunner) {
+    testRunner.dumpAsText();
+}
+
+</script>
+</body>
index 97ce682..85d6fe4 100644 (file)
@@ -4311,6 +4311,14 @@ webkit.org/b/107033 [ Debug ] fast/events/form-iframe-target-before-load-crash.h
 webkit.org/b/107033 [ Debug ] fast/events/before-unload-with-subframes.html [ Crash ]
 webkit.org/b/107033 [ Debug ] fast/css/stylesheet-enable-first-alternate-link.html [ Crash ]
 
+# Needs rebaseline.
+webkit.org/b/105574 editing/input/caret-at-the-edge-of-contenteditable.html [ Failure ]
+webkit.org/b/105574 editing/input/reveal-caret-of-multiline-contenteditable.html [ Failure ]
+webkit.org/b/105574 editing/input/reveal-caret-of-multiline-input.html [ Failure ]
+webkit.org/b/105574 platform/chromium/fast/forms/suggestion-picker/date-suggestion-picker-appearance-with-scroll-bar.html [ ImageOnlyFailure ]
+webkit.org/b/105574 platform/chromium/fast/forms/suggestion-picker/month-suggestion-picker-appearance-with-scroll-bar.html [ ImageOnlyFailure ]
+webkit.org/b/105574 platform/chromium/fast/forms/suggestion-picker/week-suggestion-picker-appearance-with-scroll-bar.html [ ImageOnlyFailure ]
+
 # This is won't fix, as the debug and release versions differ.
 webkit.org/b/99138 [ Debug SnowLeopard ] svg/custom/foreign-object-skew.svg [ ImageOnlyFailure ]
 
index 60fdf2a..337c29c 100644 (file)
@@ -1280,6 +1280,12 @@ webkit.org/b/103955 fast/repaint/caret-outside-block.html [ ImageOnlyFailure ]
 webkit.org/b/103955 fast/repaint/selection-rl.html [ ImageOnlyFailure ]
 webkit.org/b/103955 fast/repaint/caret-with-transformation.html [ Missing ]
 
+# Needs rebaseline.
+webkit.org/b/105574 editing/input/caret-at-the-edge-of-contenteditable.html [ Failure ]
+webkit.org/b/105574 editing/input/reveal-caret-of-multiline-contenteditable.html [ Failure ]
+webkit.org/b/105574 editing/input/reveal-caret-of-multiline-input.html [ Failure ]
+webkit.org/b/105574 fast/spatial-navigation/snav-div-overflow-scrol-hidden.html [ Failure ]
+
 # Rebaseline required after https://webkit.org/b/107208
 webkit.org/b/107208 fast/regions/overflow-moving-below-floats-in-variable-width-regions.html [ Pass Failure ImageOnlyFailure ]
 webkit.org/b/107208 fast/regions/overflow-size-change-in-variable-width-regions.html [ Pass Failure ImageOnlyFailure ]
index acab1bd..dee09cf 100644 (file)
@@ -1,3 +1,29 @@
+2013-01-18  Chris Hopman  <cjhopman@google.com>
+
+        Fix scrollRectToVisible in the presence of transforms
+        https://bugs.webkit.org/show_bug.cgi?id=105574
+
+        Reviewed by Simon Fraser.
+
+        When scrolling to reveal an overflow layer, the required scroll was
+        being calculated in absolute coordinates. To properly account for
+        transforms, this calculation should be done in the local coordinates
+        of the renderBox.
+
+        Tests: editing/input/reveal-selection-transformed-overflow-parent.html
+               editing/input/reveal-selection-transformed-textarea.html
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::scrollRectToVisible):
+        When scrolling to reveal an overflow layer, calculate the required
+        scroll in the local coordinates of the RenderBox.
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::absoluteToLocalQuad):
+        (WebCore):
+        * rendering/RenderObject.h:
+        (RenderObject):
+        Add function to convert an absolute quad to a local quad.
+
 2013-01-18  Julien Chaffraix  <jchaffraix@webkit.org>
 
         [CSS Grid Layout] Add support for min-content
index 69a2c78..7990e43 100644 (file)
@@ -2142,21 +2142,17 @@ void RenderLayer::scrollRectToVisible(const LayoutRect& rect, const ScrollAlignm
         // This will prevent us from revealing text hidden by the slider in Safari RSS.
         RenderBox* box = renderBox();
         ASSERT(box);
-        FloatPoint absPos = box->localToAbsolute();
-        absPos.move(box->borderLeft(), box->borderTop());
+        LayoutRect localExposeRect(box->absoluteToLocalQuad(FloatQuad(FloatRect(rect)), UseTransforms).boundingBox());
+        LayoutRect layerBounds(0, 0, box->clientWidth(), box->clientHeight());
+        LayoutRect r = getRectToExpose(layerBounds, localExposeRect, alignX, alignY);
 
-        LayoutRect layerBounds = LayoutRect(absPos.x() + scrollXOffset(), absPos.y() + scrollYOffset(), box->clientWidth(), box->clientHeight());
-        LayoutRect exposeRect = LayoutRect(rect.x() + scrollXOffset(), rect.y() + scrollYOffset(), rect.width(), rect.height());
-        LayoutRect r = getRectToExpose(layerBounds, exposeRect, alignX, alignY);
-        
-        int roundedAdjustedX = roundToInt(r.x() - absPos.x());
-        int roundedAdjustedY = roundToInt(r.y() - absPos.y());
-        IntSize clampedScrollOffset = clampScrollOffset(IntSize(roundedAdjustedX, roundedAdjustedY));
+        IntSize clampedScrollOffset = clampScrollOffset(scrollOffset() + toIntSize(roundedIntRect(r).location()));
         if (clampedScrollOffset != scrollOffset()) {
             IntSize oldScrollOffset = scrollOffset();
             scrollToOffset(clampedScrollOffset);
             IntSize scrollOffsetDifference = scrollOffset() - oldScrollOffset;
-            newRect.move(-scrollOffsetDifference);
+            localExposeRect.move(-scrollOffsetDifference);
+            newRect = LayoutRect(box->localToAbsoluteQuad(FloatQuad(FloatRect(localExposeRect)), UseTransforms).boundingBox());
         }
     } else if (!parentLayer && renderer()->isBox() && renderBox()->canBeProgramaticallyScrolled()) {
         if (frameView) {
@@ -2183,6 +2179,8 @@ void RenderLayer::scrollRectToVisible(const LayoutRect& rect, const ScrollAlignm
                     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());
                     } else
index f560011..d5bcf22 100644 (file)
@@ -2050,6 +2050,14 @@ FloatPoint RenderObject::absoluteToLocal(const FloatPoint& containerPoint, MapCo
     return transformState.lastPlanarPoint();
 }
 
+FloatQuad RenderObject::absoluteToLocalQuad(const FloatQuad& quad, MapCoordinatesFlags mode) const
+{
+    TransformState transformState(TransformState::UnapplyInverseTransformDirection, quad.boundingBox().center(), quad);
+    mapAbsoluteToLocalPoint(mode, transformState);
+    transformState.flatten();
+    return transformState.lastPlanarQuad();
+}
+
 void RenderObject::mapLocalToContainer(const RenderLayerModelObject* repaintContainer, TransformState& transformState, MapCoordinatesFlags mode, bool* wasFixed) const
 {
     if (repaintContainer == this)
index 65b27f1..3bf1721 100644 (file)
@@ -734,6 +734,8 @@ public:
     {
         return localToContainerQuad(quad, 0, mode, wasFixed);
     }
+    // Convert an absolute quad to local coordinates.
+    FloatQuad absoluteToLocalQuad(const FloatQuad&, MapCoordinatesFlags mode = 0) const;
 
     // Convert a local quad into the coordinate system of container, taking transforms into account.
     FloatQuad localToContainerQuad(const FloatQuad&, const RenderLayerModelObject* repaintContainer, MapCoordinatesFlags = 0, bool* wasFixed = 0) const;