Skip to content

Commit

Permalink
Change Range.getBoundingClientRect() when the first rect is empty
Browse files Browse the repository at this point in the history
crrev.com/376398 changed Range.getBoundingClientRect() to include the
first rectangle even when it is empty (either width or height is zero.)
This change improves when all rects are empty, but produces undesirable
result when the first rectangle is empty while the rest is not.

This patch changes it to return the first rectangle when all rectangles
are empty, following the spec change in [1].

content_editable_extractor_test.unitjs was reverted to before
crrev.com/376398.

[1] w3c/csswg-drafts@0e7a5cb

BUG=606662
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/1949373003
Cr-Commit-Position: refs/heads/master@{#393057}
  • Loading branch information
kojiishi authored and Commit bot committed May 11, 2016
1 parent c7c963d commit 27e0f01
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -197,12 +197,12 @@ TEST_F('CvoxContentEditableExtractorUnitTest', 'WordWrap', function() {

var extractor = new cvox.ContentEditableExtractor();
extractor.update(textbox);
assertEquals('One two three', extractor.getText());
assertEquals('One\ntwo\nthree', extractor.getText());
assertEquals(0, extractor.getLineStart(0));
assertEquals(3, extractor.getLineEnd(0));
assertEquals(3, extractor.getLineStart(1));
assertEquals(7, extractor.getLineEnd(1));
assertEquals(7, extractor.getLineStart(2));
assertEquals(4, extractor.getLineEnd(0));
assertEquals(4, extractor.getLineStart(1));
assertEquals(8, extractor.getLineEnd(1));
assertEquals(8, extractor.getLineStart(2));
assertEquals(13, extractor.getLineEnd(2));

// Test all possible cursor positions.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
<!DOCTPYE html>
<script src="../../../resources/testharness.js"></script>
<script src="../../../resources/testharnessreport.js"></script>
<style>
html, body {
margin: 0;
}

#container > div {
font-size: 10px;
width: 5ch;
border: thin solid black;
}
</style>
<div id="log"></div>
<div id="container">
<div id="normal-wrap">0123 456</div>
<div id="normal-wrap-with-overflow-space">01234 567</div>
<div id="pre-wrap" style="white-space: pre-wrap">0123 45678</div>
<div id="pre-wrap-with-overflow-space" style="white-space: pre-wrap">01234 5678</div>
<div id="break-word" style="word-wrap: break-word">0123456789</div>
<div id="pre" style="white-space:pre">a
b<span></span>b</div>
</div>
<script>
var range = document.createRange();

testWhenFirstRectIsEmpty("normal-wrap", [4]);
testWhenFirstRectIsEmpty("normal-wrap-with-overflow-space", [5]);
testWhenFirstRectIsEmpty("pre-wrap", [3, 4, 5]);
testWhenFirstRectIsEmpty("pre-wrap-with-overflow-space", [4, 5, 6]);
testWhenFirstRectIsEmpty("break-word", [4, 5]);
testWhenFirstRectIsEmpty("pre", [0, 1, 2]);

function testWhenFirstRectIsEmpty(id, offsets) {
let element = document.getElementById(id);
let node = element.firstChild;
for (let offset of offsets) {
range.setStart(node, offset);
range.setEnd(node, offset + 1);
let rects = range.getClientRects();
let bounds = range.getBoundingClientRect();
if (rects.length <= 1)
continue;

if (!bounds.width || !bounds.height) {
// If all rects are empty, bounds should be equal to rects[0].
test(() => assert_equals_rect(bounds, rects[0]),
`${name}[${offset}]: ${rectToString(bounds)} should be equal to the first of ${rectsToString(rects)}`);
} else {
// Otherwise, it should be the union of rects excluding empty ones.
// Since we measure one character, it should have the same height as rects[0].
test(() => assert_equals(bounds.height, rects[0].height),
`${name}[${offset}]: Height of ${rectToString(bounds)} should match to the first of ${rectsToString(rects)}`);
}
}
}

function assert_equals_rect(actual, expected, description) {
for (let prop in expected)
assert_equals(actual[prop], expected[prop], description + "." + prop);
}
function rectsToString(rects) {
return "[" + Array.prototype.map.call(rects, rectToString).join() + "]";
}

function rectToString(r) {
return "(" + r.left + "," + r.top + "-" + r.width + "," + r.height + ")";
}
</script>
16 changes: 6 additions & 10 deletions third_party/WebKit/Source/core/dom/Range.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1582,16 +1582,12 @@ FloatRect Range::boundingRect() const
getBorderAndTextQuads(quads);

FloatRect result;
// As per section 10 in https://www.w3.org/TR/cssom-view/
// "Return a static DOMRect object describing the smallest rectangle that
// includes the first rectangle in list and all of the remaining rectangles
// of which the height or width is not zero."
for (const FloatQuad& quad : quads) {
if (result.isEmpty())
result.uniteIfNonZero(quad.boundingBox());
else
result.unite(quad.boundingBox()); // Skips empty rects.
}
for (const FloatQuad& quad : quads)
result.unite(quad.boundingBox()); // Skips empty rects.

// If all rects are empty, return the first rect.
if (result.isEmpty() && !quads.isEmpty())
return quads.first().boundingBox();

return result;
}
Expand Down

0 comments on commit 27e0f01

Please sign in to comment.