Changeset 140855 in webkit

Timestamp:

Jan 25, 2013, 11:54:27 AM (12 years ago)

Author:

leviw@chromium.org

Message:

Merge 140370

Event target rects on the top level document shouldn't be clipped.
​https://bugs.webkit.org/show_bug.cgi?id=107339

Reviewed by James Robinson.

Source/WebCore:

clippedOverflowRectForRepaint clips the top-level RenderView to the viewport, which
is wrong for generating event target rects, as the result will not extend to the bounds
of the document on pages that scroll. Changing the top-level view to use documentRect
instead.

Tests updated to cover bug: platform/chromium/fast/events/touch/touch-hit-rects-in-iframe.html

platform/chromium/fast/events/touch/compositor-touch-hit-rects.html

• page/scrolling/ScrollingCoordinator.cpp:

(WebCore::accumulateRendererTouchEventTargetRects): Use converToRootView instead of
a loop around convertToContaining view. This is not a change in behavior.
(WebCore::accumulateDocumentEventTargetRects): Switch to use documentRect instead of
clippedOverflowRectForRepaint for the top-level Document, and use converToRootView
to put rects in the coordinates of the top-level document.

LayoutTests:

Updating existing tests to cover this issue.

• platform/chromium-linux/platform/chromium/fast/events/touch/compositor-touch-hit-rects-expected.txt:
• platform/chromium/fast/events/touch/compositor-touch-hit-rects.html:
• platform/chromium/fast/events/touch/touch-hit-rects-in-iframe-expected.txt:
• platform/chromium/fast/events/touch/touch-hit-rects-in-iframe.html:
• platform/chromium/fast/events/touch/resources/frame-with-document-touch-handler.html: Added.
• platform/chromium/TestExpectations:

TBR=​leviw@chromium.org

Location:

branches/chromium/1364

Files:

• LayoutTests/platform/chromium/TestExpectations (modified) (1 diff)
• LayoutTests/platform/chromium/fast/events/touch/compositor-touch-hit-rects.html (modified) (4 diffs)
• LayoutTests/platform/chromium/fast/events/touch/touch-hit-rects-in-iframe-expected.txt (modified) (1 diff)
• LayoutTests/platform/chromium/fast/events/touch/touch-hit-rects-in-iframe.html (modified) (1 diff)
• Source/WebCore/page/scrolling/ScrollingCoordinator.cpp (modified) (2 diffs)

branches/chromium/1364/LayoutTests/platform/chromium/TestExpectations

@@ -3923 +3923 @@
 webkit.org/b/98275 media/event-queue-crash.html [ Skip ]
 
+# Needs rebaselining on after webkit.org/b/107339
+webkit.org/b/107339 [ Mac Android Win ] platform/chromium/fast/events/touch/compositor-touch-hit-rects.html [ Pass Failure ]
+
 # Requires rebaselining after https://bugs.webkit.org/show_bug.cgi?id=11645
 webkit.org/b/11645 [ Mac Android ] fast/table/025.html [ Failure ]

branches/chromium/1364/LayoutTests/platform/chromium/fast/events/touch/compositor-touch-hit-rects.html

@@ -23 +23 @@
 #tests {
         font: 10px Ahem;
+}
+body {
+        height: 1000px;
 }
 </style>
@@ -77 +80 @@
 }
 
-function logRects(id) {
+function testElementWithId(id)
+{
         element = document.getElementById(id);
         element.addEventListener('touchstart', listener, false);
+        logRects(id);
+        element.removeEventListener('touchstart', listener, false);
+}
+
+function logRects(testName) {
+
         rects = window.internals.touchEventTargetClientRects(document);
         var sortedRects = new Array();
@@ -86 +96 @@
         sortedRects.sort(sortRects);
         for (var i = 0; i < rects.length; ++i)
-                log(id + "[" + i + "]: (" + sortedRects[i].left + ", " + sortedRects[i].top + ", " + sortedRects[i].width + ", " + sortedRects[i].height + ")");
-        element.removeEventListener('touchstart', listener, false);
+                log(testName + "[" + i + "]: (" + sortedRects[i].left + ", " + sortedRects[i].top + ", " + sortedRects[i].width + ", " + sortedRects[i].height + ")");
 }
 
@@ -95 +104 @@
 
         window.testRunner.dumpAsText();
-        logRects("normalFlow");
-        logRects("absoluteChildContainer");
-        logRects("relativeChildContainer");
-        logRects("overhangingContainer");
-        logRects("transformedChildContainer");
-        logRects("continuation");
-        logRects("inlineAbsoluteChildContainer");
+        testElementWithId("normalFlow");
+        testElementWithId("absoluteChildContainer");
+        testElementWithId("relativeChildContainer");
+        testElementWithId("overhangingContainer");
+        testElementWithId("transformedChildContainer");
+        testElementWithId("continuation");
+        testElementWithId("inlineAbsoluteChildContainer");
+
+        document.addEventListener('touchstart', listener, false);
+        logRects("document");
 
         var testContainer = document.getElementById("tests");

branches/chromium/1364/LayoutTests/platform/chromium/fast/events/touch/touch-hit-rects-in-iframe-expected.txt

@@ -1 +1 @@
 This test validates that touch hit tests rects are created in the coordinates of the outermost view, not their containing view. This test only works in DumpRenderTree.
-[0]: (60, 110, 50, 50)
-[1]: (420, 170, 50, 50)
+[0]: (10, 50, 285, 135)
+[1]: (60, 110, 50, 50)
+[2]: (420, 170, 50, 50)
 

branches/chromium/1364/LayoutTests/platform/chromium/fast/events/touch/touch-hit-rects-in-iframe.html

@@ -24 +24 @@
 <iframe id="iframe1" src="resources/frame-with-touch-handler.html"></iframe>
 <iframe id="iframe2"></iframe>
+<iframe id="iframe3" src="resources/frame-with-document-touch-handler.html"></iframe>
 <script>
 

branches/chromium/1364/Source/WebCore/page/scrolling/ScrollingCoordinator.cpp

@@ -189 +189 @@
             // Convert to the top-level view's coordinates.
             ASSERT(renderer->document()->view());
-            for (ScrollView* view = renderer->document()->view(); view && view->parent(); view = view->parent())
-                r = view->convertToContainingView(r);
+            r = renderer->document()->view()->convertToRootView(r);
 
             if (!parentRect.contains(r)) {
@@ -217 +216 @@
         if (touchTarget == document) {
             if (RenderView* view = document->renderView()) {
-                IntRect r = enclosingIntRect(view->clippedOverflowRectForRepaint(0));
-                if (!r.isEmpty())
+                IntRect r;
+                if (touchTarget == document->topDocument())
+                    r = view->documentRect();
+                else
+                    r = enclosingIntRect(view->clippedOverflowRectForRepaint(0));
+
+                if (!r.isEmpty()) {
+                    ASSERT(view->document()->view());
+                    r = view->document()->view()->convertToRootView(r);
                     rects.append(r);
+                }
             }
             return;