WebCore: Document.elementFromPoint() and Document.caretRangeFromPoint() erroneously...
authormrowe@apple.com <mrowe@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 Sep 2009 18:40:11 +0000 (18:40 +0000)
committermrowe@apple.com <mrowe@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 Sep 2009 18:40:11 +0000 (18:40 +0000)
https://bugs.webkit.org/show_bug.cgi?id=29245

Patch by Andrew Richards <randrew@gmail.com> on 2009-09-14
Reviewed by Sam Weinig.

Use visibleContentRect() instead of boundsRect() when checking hit point bounds on viewport.

* dom/Document.cpp:
(WebCore::Document::elementFromPoint):
(WebCore::Document::caretRangeFromPoint):

LayoutTests: Document.elementFromPoint() and Document.caretRangeFromPoint() returning null at points visible only after scrolling.
https://bugs.webkit.org/show_bug.cgi?id=29245

Patch by Andrew Richards <randrew@gmail.com> on 2009-09-14
Reviewed by Sam Weinig.

Extend tests to include hits in areas that are not in the initial containing block of the page.

* fast/dom/Document/CaretRangeFromPoint/hittest-relative-to-viewport-expected.txt:
* fast/dom/Document/CaretRangeFromPoint/hittest-relative-to-viewport.html:
* fast/dom/elementFromPoint-relative-to-viewport-expected.txt:
* fast/dom/elementFromPoint-relative-to-viewport.html:

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

LayoutTests/ChangeLog
LayoutTests/fast/dom/Document/CaretRangeFromPoint/hittest-relative-to-viewport-expected.txt
LayoutTests/fast/dom/Document/CaretRangeFromPoint/hittest-relative-to-viewport.html
LayoutTests/fast/dom/elementFromPoint-relative-to-viewport-expected.txt
LayoutTests/fast/dom/elementFromPoint-relative-to-viewport.html
WebCore/ChangeLog
WebCore/dom/Document.cpp

index 71ea024141721b011c0c03d731265d7c99440b8e..680215b2aedb3ac1be2d55065a8a694f83bb4da1 100644 (file)
@@ -1,3 +1,17 @@
+2009-09-14  Andrew Richards  <randrew@gmail.com>
+
+        Reviewed by Sam Weinig.
+
+        Document.elementFromPoint() and Document.caretRangeFromPoint() returning null at points visible only after scrolling.
+        https://bugs.webkit.org/show_bug.cgi?id=29245
+
+        Extend tests to include hits in areas that are not in the initial containing block of the page.
+
+        * fast/dom/Document/CaretRangeFromPoint/hittest-relative-to-viewport-expected.txt:
+        * fast/dom/Document/CaretRangeFromPoint/hittest-relative-to-viewport.html:
+        * fast/dom/elementFromPoint-relative-to-viewport-expected.txt:
+        * fast/dom/elementFromPoint-relative-to-viewport.html:
+
 2009-09-15  Jungshik Shin  <jshin@chromium.org>
 
         Reviewed by Eric Seidel
index d2eda151bb94b5fa1abcec7bfeb9de8c15d12f14..b41c6251d5f50230626497d6f8ba26ff6fe0e8fa 100644 (file)
@@ -1,14 +1,27 @@
-This box is here to create scrollbars.
-Testing with no scroll
-PASS: range.startContainer == element.firstChild.
-PASS: range.startOffset == 0.
-Test scrolling down
-PASS: range.startContainer == element.firstChild.
-PASS: range.startOffset == 12.
-Test scrolling right
-PASS: range.startContainer == element.firstChild.
-PASS: range.startOffset == 2.
-Test scrolling down and right
-PASS: range.startContainer == element.firstChild.
-PASS: range.startOffset == 14.
+This checks for proper behavior of caretRangeFromPoint before and after scrolling.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Range.startContainer check (got [object Text], expected [object Text])
+PASS Range.startOffset check (got 0, expected 0)
+PASS Range.startContainer check (got [object Text], expected [object Text])
+PASS Range.startOffset check (got 12, expected 12)
+PASS Range.startContainer check (got [object Text], expected [object Text])
+PASS Range.startOffset check (got 2, expected 2)
+PASS Range.startContainer check (got [object Text], expected [object Text])
+PASS Range.startOffset check (got 14, expected 14)
+PASS Range.startContainer check (got [object Text], expected [object Text])
+PASS Range.startOffset check (got 0, expected 0)
+PASS Range.startContainer check (got [object Text], expected [object Text])
+PASS Range.startOffset check (got 12, expected 12)
+PASS Range.startContainer check (got [object Text], expected [object Text])
+PASS Range.startOffset check (got 2, expected 2)
+PASS Range.startContainer check (got [object Text], expected [object Text])
+PASS Range.startOffset check (got 14, expected 14)
+PASS successfullyParsed is true
+
+TEST COMPLETE
 
index 57a359d8aa59d3dc5771f33ecb962a8e92c588c9..0d0cd1fe211ff57cabf616f51a9d365a397d657d 100644 (file)
@@ -1,87 +1,93 @@
-<html>
-<head>
-    <style>
-        #test {
-            width: 100px;
-            font-family: "Ahem";
-        }
-        #pusher {
-            width: 1000px;
-            height: 1000px;
-            outline: 1px solid black;
-        }
-    </style>
-    <script>
-        if (window.layoutTestController)
-            layoutTestController.dumpAsText();
-
-        var _log = "";
-        function log(msg)
-        {
-            _log += msg + "\n";
-        }
+<link rel="stylesheet" href="../../../js/resources/js-test-style.css">
+<script src="../../../js/resources/js-test-pre.js"></script>
+<style> 
+    .test {
+        width: 100px;
+        font-family: "Ahem";
+    }
+    .pusher {
+        width: 4000px;
+        height: 1000px;
+        outline: 1px solid black;
+    }
+</style>
 
-        function swapInLog()
-        {
-            var element = document.getElementById('test');
-            var parent = element.parentNode;
-            if (window.layoutTestController)
-                parent.removeChild(element);
-            var console = document.createElement("pre");
-            console.textContent = _log;
-            parent.appendChild(console);
-        }
+<div id="testArea">
+    <div id="test-top" class="test">xxxxx xxxxx xxxxx xxxxx</div>
+    <div class="pusher">This box is here to create scrollbars.</div>
+    <div id="test-bottom" class="test" style="margin-left: 900px;">xxxxx xxxxx xxxxx xxxxx</div>
+    <div class="pusher">This box is here to create additional space for the hit tests which must initially be in the scroll area.</div>
+</div>
+
+<p id="description"></p>
+<div id="console"></div>
 
-        var element;
+<script>
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
 
-        function test(pos, __expectedContainer, __expectedOffset)
-        {
-            var expectedContainer = eval(__expectedContainer);
-            var expectedOffset = eval(__expectedOffset);
+    description('This checks for proper behavior of caretRangeFromPoint before and after scrolling.');        
 
-            var range = document.caretRangeFromPoint(pos.x, pos.y);
+    var elementTop = document.getElementById('test-top'),
+        elementBottom = document.getElementById('test-bottom');
 
-            if (range.startContainer == expectedContainer) {
-                log("PASS: range.startContainer == " + __expectedContainer + ".");
-            } else {
-                log("FAIL: range.startContainer == " + range.startContainer + ".");
+    function testsWithBaseline(baselinePos, expectedContainer) {
+        function test(expectedOffset, scrollByX, scrollByY) {
+            var hitPosition = { x: 15, y: 15 },
+                range,
+                doesContainerPass = function() { return range.startContainer === expectedContainer },
+                doesOffsetPass = function() { return range.startOffset === expectedOffset };
+
+            // Scroll relative to target.
+            scrollRelativeToBaseline(scrollByX, scrollByY);
+
+            range = document.caretRangeFromPoint(hitPosition.x, hitPosition.y);
+
+            // shouldn't return null range on any of these tests
+            if (range === null) {
+                testFailed("null range was returned from document.caretRangeFromPoint(" + hitPosition.x + ", " + hitPosition.y + ") at window scroll position " + window.scrollX + "x" + window.scrollY);
+                return;
             }
 
-            if (range.startOffset == expectedOffset) {
-                log("PASS: range.startOffset == " + __expectedOffset + ".");
-            } else {
-                log("FAIL: range.startOffset == " + range.startOffset + ".");
+            // do an actual check
+            function check(thunk, message) {
+                if (thunk())
+                    testPassed(message);
+                else
+                    testFailed(message);
             }
+            check(doesContainerPass, "Range.startContainer check (got " + range.startContainer + ", expected " + expectedContainer + ")");
+            check(doesOffsetPass, "Range.startOffset check (got " + range.startOffset + ", expected " + expectedOffset + ")");
         }
 
-        window.onload = function()
-        {
-            element = document.getElementById('test');
-            text = element.firstChild;
+        function scrollRelativeToBaseline(x, y) {
+            window.scrollTo(baselinePos.x + x, baselinePos.y + y);
+        }
 
-            var positionToTest = { x: 15, y: 15 };
+        test(0, 0, 0);
+        test(12, 0, 25);
+        test(2, 25, 0);
+        test(14, 25, 25);
+        debug(" ");
+    }
 
-            log("Testing with no scroll");
-            test(positionToTest, "element.firstChild", "0");
+    var rectTop = elementTop.getBoundingClientRect(),
+        rectBottom = elementBottom.getBoundingClientRect(),
+        // Subtract some distance so we aren't in the very top left of the target.
+        topBaseline = { x: rectTop.left - 8, y: rectTop.top - 8 },
+        bottomBaseline = { x: rectBottom.left - 8, y: rectBottom.top - 8 };
 
-            log("Test scrolling down");
-            window.scrollTo(0, 25);
-            test(positionToTest, "element.firstChild", "12");
+     // Testing inside initial containing block (top left)
+    testsWithBaseline(topBaseline, elementTop.firstChild);
 
-            log("Test scrolling right");
-            window.scrollTo(25, 0);
-            test(positionToTest, "element.firstChild", "2");
+    // Testing outside initial containing block (mid-page)
+    testsWithBaseline(bottomBaseline, elementBottom.firstChild);
 
-            log("Test scrolling down and right");
-            window.scrollTo(25, 25);
-            test(positionToTest, "element.firstChild", "14");
+    if (window.layoutTestController) {
+        var area = document.getElementById('testArea');
+        area.parentNode.removeChild(area);
+    }
 
-            swapInLog();
-        }
-    </script>
-</head>
-<body>
-<div id="test">xxxxx xxxxx xxxxx xxxxx</div>
-<div id="pusher">This box is here to create scrollbars.</div>
-</body>
-</html>
+    successfullyParsed = true;
+</script>
+<script src="../../../js/resources/js-test-post.js"></script>
index 91c3961bfd2e9c9060a02c401ab625d1cef40a75..c88a511940022c9d985790dca13f43d810fa2b6d 100644 (file)
@@ -3,10 +3,14 @@ This test document.elementFromPoint is evaluated in with respect to the viewport
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS unscrolledBox is '0'
-PASS scrolledDownBox is '15'
-PASS scrolledRightBox is '3'
-PASS scrolledDownAndRightBox is '18'
+PASS unscrolledBoxInitial is '0'
+PASS scrolledDownBoxInitial is '15'
+PASS scrolledRightBoxInitial is '3'
+PASS scrolledDownAndRightBoxInitial is '18'
+PASS unscrolledBoxOffscreen is '0'
+PASS scrolledDownBoxOffscreen is '15'
+PASS scrolledRightBoxOffscreen is '3'
+PASS scrolledDownAndRightBoxOffscreen is '18'
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 2936943d202d8ee9a7bf505f9d2be5322a15aa67..4ae3841f4f0c8387e6224c6baf9d2bcea955bafc 100644 (file)
@@ -1,8 +1,9 @@
 <link rel="stylesheet" href="../js/resources/js-test-style.css">
 <script src="../js/resources/js-test-pre.js"></script>
 <style>
-    #test {
+    .test {
         width: 100px;
+        height: 100px;
         outline: 1px solid black;
     }
     .testItem {
         height: 20px;
         outline: 1px solid red;
     }
-    #pusher {
+    .pusher {
         width: 1000px;
         height: 1000px;
         outline: 1px solid black;
     }
 </style>
-<p id="description"></p>
-<div id="console"></div>
 <div id="testArea">
     <br>
-    <div id="test"></div>
-    <div id="pusher">This box is here to create scrollbars.</div>
+    <div id="test-initial" class="test"></div>
+    <div class="pusher">This box is here to create scrollbars.</div>
+    <div id="test-offscreen" class="test"></div>
+    <div class="pusher">This box is here to create even more scrollbars!</div>
 </div>
+<p id="description"></p>
+<div id="console"></div>
 <script>
     window.onclick = function(e)
     {
     if (window.layoutTestController)
         layoutTestController.dumpAsText();
 
-    var element = document.getElementById('test');
-    for (var i = 0; i < 25; ++i) {
-        var item = document.createElement("div");
-        item.className = "testItem";
-        item.textContent = String(i);
-        element.appendChild(item);
-    }
-
     description('This test document.elementFromPoint is evaluated in with respect to the viewport, not the document.');
 
-    var unScrolledBoundingBox = element.getBoundingClientRect();
-    var testX = unScrolledBoundingBox.left + 10;
-    var testY = unScrolledBoundingBox.top + 10;
+    function testElement(element, label, offsetX, offsetY) {
+        for (var i = 0; i < 25; ++i) {
+            var item = document.createElement("div");
+            item.className = "testItem";
+            item.textContent = String(i);
+            element.appendChild(item);
+        }
+        var testX = 10;
+        var testY = 10;
 
-    // Get initial box.
-    var unscrolledBox = document.elementFromPoint(testX, testY).textContent;
+        var unscrolledBox = "unscrolledBox" + label,
+            scrolledDownBox = "scrolledDownBox" + label,
+            scrolledRightBox = "scrolledRightBox" + label,
+            scrolledDownAndRightBox = "scrolledDownAndRightBox" + label;
 
-    // Test scrolling down
-    window.scrollTo(0, 50);
-    var scrolledDownBox = document.elementFromPoint(testX, testY).textContent;
+        function relativeScroll(x, y) {
+            window.scrollTo(offsetX + x, offsetY + y);
+        }
 
-    // Test scrolling right
-    window.scrollTo(50, 0);
-    var scrolledRightBox = document.elementFromPoint(testX, testY).textContent;
+        function getFromPoint(x, y) {
+            relativeScroll(x, y);
+            var hitElement = document.elementFromPoint(testX, testY);
+            // shouldn't return null range on any of these tests
+            if (hitElement === null)
+                return null;
+            else
+                return hitElement.textContent;
+        }
+        // Get initial box.
+        window[unscrolledBox] = getFromPoint(0, 0);
 
-    // Test scrolling down and right
-    window.scrollTo(50, 50);
-    var scrolledDownAndRightBox = document.elementFromPoint(testX, testY).textContent;
+        // Test scrolling down
+        window[scrolledDownBox] = getFromPoint(0, 50);
 
-    // Reset
-    window.scrollTo(0, 0);
-    
-    shouldBe("unscrolledBox", "'0'");
-    shouldBe("scrolledDownBox", "'15'");
-    shouldBe("scrolledRightBox", "'3'");
-    shouldBe("scrolledDownAndRightBox", "'18'");
+        // Test scrolling right
+        window[scrolledRightBox] = getFromPoint(50, 0);
+
+        // Test scrolling down and right
+        window[scrolledDownAndRightBox] = getFromPoint(50, 50);
+
+        shouldBe(unscrolledBox, "'0'");
+        shouldBe(scrolledDownBox, "'15'");
+        shouldBe(scrolledRightBox, "'3'");
+        shouldBe(scrolledDownAndRightBox, "'18'");
+    }
+
+    var elementInitial = document.getElementById('test-initial');
+    var elementOffscreen = document.getElementById('test-offscreen');
+    var offset = elementInitial.getBoundingClientRect();
+    testElement(elementInitial, "Initial", offset.left, offset.top);
+    testElement(elementOffscreen, "Offscreen", offset.left, offset.top + 1100);
     
     if (window.layoutTestController) {
         var area = document.getElementById('testArea');
index 7d4dac0d52fd3862f349979f323a8432588fa085..250d8215a87d90343d78b5ed121712f6c367984e 100644 (file)
@@ -1,3 +1,16 @@
+2009-09-14  Andrew Richards  <randrew@gmail.com>
+
+        Reviewed by Sam Weinig.
+
+        Document.elementFromPoint() and Document.caretRangeFromPoint() erroneously returning null at points visible only after scrolling.
+        https://bugs.webkit.org/show_bug.cgi?id=29245
+
+        Use visibleContentRect() instead of boundsRect() when checking hit point bounds on viewport.
+
+        * dom/Document.cpp:
+        (WebCore::Document::elementFromPoint):
+        (WebCore::Document::caretRangeFromPoint):
+
 2009-09-15  Jungshik Shin  <jshin@chromium.org>
 
         Reviewed by Eric Seidel
index 2f337203ef881dafdcf7adc4b8fdde672b79f68e..1154e1268b9533b1ce39f66434c13ca683b9aeaa 100644 (file)
@@ -940,7 +940,7 @@ Element* Document::elementFromPoint(int x, int y) const
     float zoomFactor = frame->pageZoomFactor();
     IntPoint point = roundedIntPoint(FloatPoint(x * zoomFactor, y * zoomFactor)) + view()->scrollOffset();
 
-    if (!frameView->boundsRect().contains(point))
+    if (!frameView->visibleContentRect().contains(point))
         return 0;
 
     HitTestRequest request(HitTestRequest::ReadOnly | HitTestRequest::Active);
@@ -970,7 +970,7 @@ PassRefPtr<Range> Document::caretRangeFromPoint(int x, int y)
     float zoomFactor = frame->pageZoomFactor();
     IntPoint point = roundedIntPoint(FloatPoint(x * zoomFactor, y * zoomFactor)) + view()->scrollOffset();
 
-    if (!frameView->boundsRect().contains(point))
+    if (!frameView->visibleContentRect().contains(point))
         return 0;
 
     HitTestRequest request(HitTestRequest::ReadOnly | HitTestRequest::Active);