Changeset 148898 in webkit

Timestamp:

Apr 22, 2013 11:23:33 AM (11 years ago)

Author:

aestes@apple.com

Message:

Range.getClientRects() should not include rects for partially selected elements
​https://bugs.webkit.org/show_bug.cgi?id=76839

Reviewed by Sam Weinig.

Source/WebCore:

CSSOM says that Range.getClientRects() should include the border boxes
of each element selected by the range. However, we were also
incorrectly including border boxes for partially selected elements.
For instance, consider the following selection:

<div>line one<br>line two</div>
-------------

The <div> is two lines tall, but only the first line is selected.
Despite this, we'd include the <div>'s border box since its start tag
was included in the selection.

Fix this by excluding partially selected elements. This is accomplished
by skipping over elements that come after the start boundary point of
the range but are ancestors of the end boundary point.

Added test cases to LayoutTests/fast/dom/Range/getClientRects.html.

• dom/Range.cpp:

(WebCore::Range::getBorderAndTextQuads):

LayoutTests:

• fast/dom/Range/getClientRects-expected.txt: Updated expected result.
• fast/dom/Range/getClientRects.html: Added new test cases.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/dom/Range/getClientRects-expected.txt (modified) (1 diff)
• LayoutTests/fast/dom/Range/getClientRects.html (modified) (2 diffs)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/Range.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-04-22  Andy Estes  <aestes@apple.com>
+
+        Range.getClientRects() should not include rects for partially selected elements
+        https://bugs.webkit.org/show_bug.cgi?id=76839
+
+        Reviewed by Sam Weinig.
+
+        * fast/dom/Range/getClientRects-expected.txt: Updated expected result.
+        * fast/dom/Range/getClientRects.html: Added new test cases.
+
 2013-04-22  Eric Carlson  <eric.carlson@apple.com>
 

trunk/LayoutTests/fast/dom/Range/getClientRects-expected.txt

@@ -218 +218 @@
 PASS rects[3].height is 360
 Test 11
+PASS rects.length is 2
+PASS rects[0].left is 8
+PASS rects[0].top is 2524
+PASS rects[0].width is 400
+PASS rects[0].height is 40
+PASS rects[1].left is 8
+PASS rects[1].top is 2535
+PASS rects[1].width is 177
+PASS rects[1].height is 18
+Test 12
+PASS rects.length is 1
+PASS rects[0].left is 8
+PASS rects[0].top is 2760
+PASS rects[0].width is 400
+PASS rects[0].height is 160
+Test 13
+PASS rects.length is 4
+PASS rects[0].left is 8
+PASS rects[0].top is 2967
+PASS rects[0].width is 396
+PASS rects[0].height is 18
+PASS rects[1].left is 8
+PASS rects[1].top is 3007
+PASS rects[1].width is 398
+PASS rects[1].height is 18
+PASS rects[2].left is 8
+PASS rects[2].top is 3047
+PASS rects[2].width is 360
+PASS rects[2].height is 18
+PASS rects[3].left is 8
+PASS rects[3].top is 3087
+PASS rects[3].width is 306
+PASS rects[3].height is 18
+Test 14
 PASS rects.length is 0
 PASS successfullyParsed is true

trunk/LayoutTests/fast/dom/Range/getClientRects.html

@@ -81 +81 @@
 
 <div class="box" id="test10">weee! Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.</div>
+
+<br><br>
+
+<div class="box" id="test11"><div>Lorem ipsum dolor sit amet,</div> consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.</div>
+
+<br><br>
+
+<div class="box" id="test12">Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.</div>
+
+<br><br>
+
+<div class="box" id="test13">Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.</div>
 
 </div>
@@ -399 +411 @@
     shouldBe("rects[3].height", "360");
 
+    // Test 11
     debug("Test 11");
     var range11 = document.createRange();
+    range11.setStartBefore(document.getElementById('test11'));
+    range11.setEndAfter(document.getElementById('test11').firstChild);
+    show(range11);
     rects = range11.getClientRects();
+    shouldBe("rects.length", "2");
+    shouldBe("rects[0].left", "8");
+    shouldBe("rects[0].top", "2524");
+    shouldBe("rects[0].width", "400");
+    shouldBe("rects[0].height", "40");
+    shouldBe("rects[1].left", "8");
+    shouldBe("rects[1].top", "2535");
+    shouldBe("rects[1].width", "177");
+    shouldBe("rects[1].height", "18");
+
+    // Test 12
+    debug("Test 12");
+    var range12 = document.createRange();
+    range12.setStartBefore(document.getElementById('test12'));
+    range12.setEndBefore(document.getElementById('test12').firstChild);
+    show(range12);
+    rects = range12.getClientRects();
+    shouldBe("rects.length", "1");
+    shouldBe("rects[0].left", "8");
+    shouldBe("rects[0].top", "2760");
+    shouldBe("rects[0].width", "400");
+    shouldBe("rects[0].height", "160");
+
+    // Test 13
+    debug("Test 13");
+    var range13 = document.createRange();
+    range13.setStartBefore(document.getElementById('test13'));
+    range13.setEndAfter(document.getElementById('test13').firstChild);
+    show(range13);
+    rects = range13.getClientRects();
+    shouldBe("rects.length", "4");
+    shouldBe("rects[0].left", "8");
+    shouldBe("rects[0].top", "2967");
+    shouldBe("rects[0].width", "396");
+    shouldBe("rects[0].height", "18");
+    shouldBe("rects[1].left", "8");
+    shouldBe("rects[1].top", "3007");
+    shouldBe("rects[1].width", "398");
+    shouldBe("rects[1].height", "18");
+    shouldBe("rects[2].left", "8");
+    shouldBe("rects[2].top", "3047");
+    shouldBe("rects[2].width", "360");
+    shouldBe("rects[2].height", "18");
+    shouldBe("rects[3].left", "8");
+    shouldBe("rects[3].top", "3087");
+    shouldBe("rects[3].width", "306");
+    shouldBe("rects[3].height", "18");
+
+    // Test 14
+    debug("Test 14");
+    var range14 = document.createRange();
+    rects = range14.getClientRects();
     shouldBe("rects.length", "0");
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-04-22  Andy Estes  <aestes@apple.com>
+
+        Range.getClientRects() should not include rects for partially selected elements
+        https://bugs.webkit.org/show_bug.cgi?id=76839
+
+        Reviewed by Sam Weinig.
+
+        CSSOM says that Range.getClientRects() should include the border boxes
+        of each element selected by the range. However, we were also
+        incorrectly including border boxes for partially selected elements.
+        For instance, consider the following selection:
+
+            <div>line one<br>line two</div>
+            -------------
+
+        The <div> is two lines tall, but only the first line is selected.
+        Despite this, we'd include the <div>'s border box since its start tag
+        was included in the selection.
+
+        Fix this by excluding partially selected elements. This is accomplished
+        by skipping over elements that come after the start boundary point of
+        the range but are ancestors of the end boundary point.
+
+        Added test cases to LayoutTests/fast/dom/Range/getClientRects.html.
+
+        * dom/Range.cpp:
+        (WebCore::Range::getBorderAndTextQuads):
+
 2013-04-22  Yi Shen  <max.hong.shen@gmail.com>
 

trunk/Source/WebCore/dom/Range.cpp

@@ -1910 +1910 @@
     Node* stopNode = pastLastNode();
 
-    HashSet<Node*> nodeSet;
+    HashSet<Node*> selectedElementsSet;
     for (Node* node = firstNode(); node != stopNode; node = NodeTraversal::next(node)) {
         if (node->isElementNode())
-            nodeSet.add(node);
-    }
+            selectedElementsSet.add(node);
+    }
+
+    // Don't include elements that are only partially selected.
+    Node* lastNode = m_end.childBefore() ? m_end.childBefore() : endContainer;
+    for (Node* parent = lastNode->parentNode(); parent; parent = parent->parentNode())
+        selectedElementsSet.remove(parent);
 
     for (Node* node = firstNode(); node != stopNode; node = NodeTraversal::next(node)) {
-        if (node->isElementNode()) {
-            if (!nodeSet.contains(node->parentNode())) {
-                if (RenderBoxModelObject* renderBoxModelObject = toElement(node)->renderBoxModelObject()) {
-                    Vector<FloatQuad> elementQuads;
-                    renderBoxModelObject->absoluteQuads(elementQuads);
-                    m_ownerDocument->adjustFloatQuadsForScrollAndAbsoluteZoomAndFrameScale(elementQuads, renderBoxModelObject);
-
-                    quads.append(elementQuads);
-                }
+        if (node->isElementNode() && selectedElementsSet.contains(node) && !selectedElementsSet.contains(node->parentNode())) {
+            if (RenderBoxModelObject* renderBoxModelObject = toElement(node)->renderBoxModelObject()) {
+                Vector<FloatQuad> elementQuads;
+                renderBoxModelObject->absoluteQuads(elementQuads);
+                m_ownerDocument->adjustFloatQuadsForScrollAndAbsoluteZoomAndFrameScale(elementQuads, renderBoxModelObject);
+
+                quads.append(elementQuads);
             }
         } else if (node->isTextNode()) {