Incorrect rect-based hit-test result for culled-inline elements
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Jun 2012 17:02:19 +0000 (17:02 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Jun 2012 17:02:19 +0000 (17:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=85849

Patch by Raymes Khoury <raymes@chromium.org> on 2012-06-07
Reviewed by Levi Weintraub.

Source/WebCore:

Modified code which blindly adds culled inlines to rect-based hit-test
results so that it only does so if the child node does not fully cover
the hit-test region.

Test: fast/dom/nodesFromRect-culled-inline.html

* rendering/HitTestResult.cpp:
(WebCore::HitTestResult::addNodeToRectBasedTestResult):

LayoutTests:

Added tests for rect-based hit-testing for the case when the child of a
culled inline element is in the hit-test result. One of these tests is
expected to fail due to https://bugs.webkit.org/show_bug.cgi?id=88376.

* fast/dom/nodesFromRect-culled-inline-expected.txt: Added.
* fast/dom/nodesFromRect-culled-inline.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/dom/nodesFromRect-culled-inlines-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/nodesFromRect-culled-inlines.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/HitTestResult.cpp

index 25233ef..8f3674e 100644 (file)
@@ -1,3 +1,17 @@
+2012-06-07  Raymes Khoury  <raymes@chromium.org>
+
+        Incorrect rect-based hit-test result for culled-inline elements
+        https://bugs.webkit.org/show_bug.cgi?id=85849
+
+        Reviewed by Levi Weintraub.
+
+        Added tests for rect-based hit-testing for the case when the child of a
+        culled inline element is in the hit-test result. One of these tests is
+        expected to fail due to https://bugs.webkit.org/show_bug.cgi?id=88376.
+
+        * fast/dom/nodesFromRect-culled-inline-expected.txt: Added.
+        * fast/dom/nodesFromRect-culled-inline.html: Added.
+
 2012-06-07  Daniel Erat  <derat@chromium.org>
 
         [chromium] Add test for subpixel positioning.
diff --git a/LayoutTests/fast/dom/nodesFromRect-culled-inlines-expected.txt b/LayoutTests/fast/dom/nodesFromRect-culled-inlines-expected.txt
new file mode 100644 (file)
index 0000000..743ca92
--- /dev/null
@@ -0,0 +1,10 @@
+  PASS successfullyParsed is true
+
+TEST COMPLETE
+PASS All correct nodes found for rect
+PASS All correct nodes found for rect
+PASS All correct nodes found for rect
+PASS All correct nodes found for rect
+FAIL Different number of nodes for rect[0,98], [0,1,2,0]: '2' vs '3'
+PASS All correct nodes found for rect
+
diff --git a/LayoutTests/fast/dom/nodesFromRect-culled-inlines.html b/LayoutTests/fast/dom/nodesFromRect-culled-inlines.html
new file mode 100644 (file)
index 0000000..3cc3dae
--- /dev/null
@@ -0,0 +1,56 @@
+<html>
+<head>
+  <title>Document::nodesFromRect test - bug 85849</title>
+  <script src="../js/resources/js-test-pre.js"></script>
+  <script src="resources/nodesFromRect.js"></script>
+  <script type="application/javascript">
+    function runTest()
+    {
+      if (window.layoutTestController) {
+        layoutTestController.dumpAsText();
+        layoutTestController.waitUntilDone();
+      }
+
+      var e = {};
+
+      // Set up shortcut access to elements
+      e['html'] = document.getElementsByTagName("html")[0];
+      ['body', 'span', 'img'].forEach(function(a) {
+        e[a] = document.getElementById(a);
+      });
+
+      window.scrollTo(0, 0);
+
+      /* Point based test over the img only. */
+      check(20, 20, 0, 0, 0, 0, [e.img]);
+      /* Rect based test over the img only. */
+      check(20, 20, 5, 5, 5, 5, [e.img]);
+
+      /* Note that for the tests below, the img bounds are considered to be (99, 99). */
+      /* Point based test over the img and the span. */
+      check(0, 99, 0, 0, 0, 0, [e.img]);
+      /* Rect based test over the img and the span with the img fully covering the hit region. */
+      check(0, 98, 0, 1, 1, 0, [e.img]);
+      /* Rect based test over the img and the span with the img not fully covering the hit region. */
+      /* FIXME: This fails due to: https://bugs.webkit.org/show_bug.cgi?id=88376 */
+      check(0, 98, 0, 1, 2, 0, [e.img, e.span]);
+      /* Rect based test over the entire img. */
+      check(0, 0, 0, 99, 99, 0, [e.img]);
+
+      if (window.layoutTestController)
+        layoutTestController.notifyDone();
+    }
+
+    window.onload = runTest;
+  </script>
+</head>
+<body id="body" style="padding: 0; margin: 0;">
+  <span id="span" style="margin: 0; padding: 0; font-size:36px">
+    <img id="img" width="100" height="100" style="background-color: black; margin: 0; padding: 0;" />
+  </span>
+
+  <span id="console" style="position: absolute; top: 150px;"></span>
+  <script src="../js/resources/js-test-post.js"></script>
+</body>
+</html>
+
index 5baf51f..3cc4516 100644 (file)
@@ -1,3 +1,19 @@
+2012-06-07  Raymes Khoury  <raymes@chromium.org>
+
+        Incorrect rect-based hit-test result for culled-inline elements
+        https://bugs.webkit.org/show_bug.cgi?id=85849
+
+        Reviewed by Levi Weintraub.
+
+        Modified code which blindly adds culled inlines to rect-based hit-test
+        results so that it only does so if the child node does not fully cover
+        the hit-test region.
+
+        Test: fast/dom/nodesFromRect-culled-inline.html
+
+        * rendering/HitTestResult.cpp:
+        (WebCore::HitTestResult::addNodeToRectBasedTestResult):
+
 2012-06-07  Daniel Erat  <derat@chromium.org>
 
         Make Skia backend honor requests for subpixel-positioned text.
index 66e1da9..4e3f6bd 100644 (file)
@@ -622,21 +622,23 @@ bool HitTestResult::addNodeToRectBasedTestResult(Node* node, const LayoutPoint&
 
     mutableRectBasedTestResult().add(node);
 
-    if (node->renderer()->isInline()) {
+    bool regionFilled = rect.contains(rectForPoint(pointInContainer));
+    // FIXME: This code (incorrectly) attempts to correct for culled inline nodes. See https://bugs.webkit.org/show_bug.cgi?id=85849.
+    if (node->renderer()->isInline() && !regionFilled) {
         for (RenderObject* curr = node->renderer()->parent(); curr; curr = curr->parent()) {
             if (!curr->isRenderInline())
                 break;
-            
+
             // We need to make sure the nodes for culled inlines get included.
             RenderInline* currInline = toRenderInline(curr);
             if (currInline->alwaysCreateLineBoxes())
                 break;
-            
+
             if (currInline->visibleToHitTesting() && currInline->node())
                 mutableRectBasedTestResult().add(currInline->node()->shadowAncestorNode());
         }
     }
-    return !rect.contains(rectForPoint(pointInContainer));
+    return !regionFilled;
 }
 
 bool HitTestResult::addNodeToRectBasedTestResult(Node* node, const LayoutPoint& pointInContainer, const FloatRect& rect)
@@ -655,21 +657,23 @@ bool HitTestResult::addNodeToRectBasedTestResult(Node* node, const LayoutPoint&
 
     mutableRectBasedTestResult().add(node);
 
-    if (node->renderer()->isInline()) {
+    bool regionFilled = rect.contains(rectForPoint(pointInContainer));
+    // FIXME: This code (incorrectly) attempts to correct for culled inline nodes. See https://bugs.webkit.org/show_bug.cgi?id=85849.
+    if (node->renderer()->isInline() && !regionFilled) {
         for (RenderObject* curr = node->renderer()->parent(); curr; curr = curr->parent()) {
             if (!curr->isRenderInline())
                 break;
-            
+
             // We need to make sure the nodes for culled inlines get included.
             RenderInline* currInline = toRenderInline(curr);
             if (currInline->alwaysCreateLineBoxes())
                 break;
-            
+
             if (currInline->visibleToHitTesting() && currInline->node())
                 mutableRectBasedTestResult().add(currInline->node()->shadowAncestorNode());
         }
     }
-    return !rect.contains(rectForPoint(pointInContainer));
+    return !regionFilled;
 }
 
 void HitTestResult::append(const HitTestResult& other)