- Revision
- 91763
- Author
- [email protected]
- Date
- 2011-07-26 10:47:03 -0700 (Tue, 26 Jul 2011)
Log Message
RenderText::absoluteRectsForRange() and absoluteQuadsForRange() have nearly duplicate code
https://bugs.webkit.org/show_bug.cgi?id=62478
Reviewed by Simon Fraser.
Source/WebCore:
Test: fast/dom/Range/getClientRects.html
* platform/graphics/FloatRect.h:
(WebCore::FloatRect::isZero):
Add izZero method, unlike isEmpty this checks if both the width and the
height are zero.
* rendering/RenderText.cpp:
(WebCore::absoluteQuadForTextBox):
(WebCore::RenderText::absoluteRectsForRange):
(WebCore::RenderText::absoluteQuadsForRange):
Break duplicate code out of absoluteRectsForRange and
absoluteQuadsForRange into shared static function.
Fix what I presume to be a bug in the absoluteQuadsForRange where the
logicalHeight was always used to set the size in absoluteQuadForTextBox
as oppsued to the absoluteRectsForRange implementation that uses the
logicalWidth or the logicalHeight depending on whether it's horizontal or
vertical.
LayoutTests:
* fast/dom/Range/getClientRects-expected.txt:
* fast/dom/Range/getClientRects.html:
Add test for vertical text.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (91762 => 91763)
--- trunk/LayoutTests/ChangeLog 2011-07-26 17:37:15 UTC (rev 91762)
+++ trunk/LayoutTests/ChangeLog 2011-07-26 17:47:03 UTC (rev 91763)
@@ -1,3 +1,14 @@
+2011-07-26 Emil A Eklund <[email protected]>
+
+ RenderText::absoluteRectsForRange() and absoluteQuadsForRange() have nearly duplicate code
+ https://bugs.webkit.org/show_bug.cgi?id=62478
+
+ Reviewed by Simon Fraser.
+
+ * fast/dom/Range/getClientRects-expected.txt:
+ * fast/dom/Range/getClientRects.html:
+ Add test for vertical text.
+
2011-07-26 Adrienne Walker <[email protected]>
[chromium] Mark media-controls-clone as flaky crashing on other platforms.
Modified: trunk/LayoutTests/fast/dom/Range/getClientRects-expected.txt (91762 => 91763)
--- trunk/LayoutTests/fast/dom/Range/getClientRects-expected.txt 2011-07-26 17:37:15 UTC (rev 91762)
+++ trunk/LayoutTests/fast/dom/Range/getClientRects-expected.txt 2011-07-26 17:47:03 UTC (rev 91763)
@@ -198,6 +198,24 @@
PASS rects[0].top is 1903
PASS rects[0].width is 0
PASS rects[0].height is 18
+Test 10
+PASS rects.length is 4
+PASS rects[0].left is 19
+PASS rects[0].top is 2088
+PASS rects[0].width is 18
+PASS rects[0].height is 393
+PASS rects[1].left is 59
+PASS rects[1].top is 2088
+PASS rects[1].width is 18
+PASS rects[1].height is 377
+PASS rects[2].left is 99
+PASS rects[2].top is 2088
+PASS rects[2].width is 18
+PASS rects[2].height is 372
+PASS rects[3].left is 139
+PASS rects[3].top is 2088
+PASS rects[3].width is 18
+PASS rects[3].height is 360
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/fast/dom/Range/getClientRects.html (91762 => 91763)
--- trunk/LayoutTests/fast/dom/Range/getClientRects.html 2011-07-26 17:37:15 UTC (rev 91762)
+++ trunk/LayoutTests/fast/dom/Range/getClientRects.html 2011-07-26 17:47:03 UTC (rev 91763)
@@ -27,6 +27,11 @@
-webkit-transform: translate(50px, 100px) rotate(50deg);
}
+#test10 {
+ height: 400px;
+ -webkit-writing-mode: vertical-lr;
+}
+
#console {
position:absolute;
left: 500px;
@@ -73,6 +78,10 @@
<div class="box" id="test9">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="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>
+
</div>
<script>
@@ -366,6 +375,30 @@
shouldBe("rects[0].width", "0");
shouldBe("rects[0].height", "18");
+ // Test 10
+ debug("Test 10");
+ var range10 = document.createRange();
+ range10.selectNodeContents(document.getElementById('test10'));
+ show(range10);
+ rects = range10.getClientRects();
+ shouldBe("rects.length", "4");
+ shouldBe("rects[0].left", "19");
+ shouldBe("rects[0].top", "2088");
+ shouldBe("rects[0].width", "18");
+ shouldBe("rects[0].height", "393");
+ shouldBe("rects[1].left", "59");
+ shouldBe("rects[1].top", "2088");
+ shouldBe("rects[1].width", "18");
+ shouldBe("rects[1].height", "377");
+ shouldBe("rects[2].left", "99");
+ shouldBe("rects[2].top", "2088");
+ shouldBe("rects[2].width", "18");
+ shouldBe("rects[2].height", "372");
+ shouldBe("rects[3].left", "139");
+ shouldBe("rects[3].top", "2088");
+ shouldBe("rects[3].width", "18");
+ shouldBe("rects[3].height", "360");
+
if (window.layoutTestController) {
var area = document.getElementById('testArea');
area.parentNode.removeChild(area);
Modified: trunk/Source/WebCore/ChangeLog (91762 => 91763)
--- trunk/Source/WebCore/ChangeLog 2011-07-26 17:37:15 UTC (rev 91762)
+++ trunk/Source/WebCore/ChangeLog 2011-07-26 17:47:03 UTC (rev 91763)
@@ -1,3 +1,30 @@
+2011-07-26 Emil A Eklund <[email protected]>
+
+ RenderText::absoluteRectsForRange() and absoluteQuadsForRange() have nearly duplicate code
+ https://bugs.webkit.org/show_bug.cgi?id=62478
+
+ Reviewed by Simon Fraser.
+
+ Test: fast/dom/Range/getClientRects.html
+
+ * platform/graphics/FloatRect.h:
+ (WebCore::FloatRect::isZero):
+ Add izZero method, unlike isEmpty this checks if both the width and the
+ height are zero.
+
+ * rendering/RenderText.cpp:
+ (WebCore::absoluteQuadForTextBox):
+ (WebCore::RenderText::absoluteRectsForRange):
+ (WebCore::RenderText::absoluteQuadsForRange):
+ Break duplicate code out of absoluteRectsForRange and
+ absoluteQuadsForRange into shared static function.
+
+ Fix what I presume to be a bug in the absoluteQuadsForRange where the
+ logicalHeight was always used to set the size in absoluteQuadForTextBox
+ as oppsued to the absoluteRectsForRange implementation that uses the
+ logicalWidth or the logicalHeight depending on whether it's horizontal or
+ vertical.
+
2011-07-26 Alexandru Chiculita <[email protected]>
[CSSRegions] Collect flowed elements in different render element
Modified: trunk/Source/WebCore/platform/graphics/FloatRect.h (91762 => 91763)
--- trunk/Source/WebCore/platform/graphics/FloatRect.h 2011-07-26 17:37:15 UTC (rev 91762)
+++ trunk/Source/WebCore/platform/graphics/FloatRect.h 2011-07-26 17:47:03 UTC (rev 91763)
@@ -102,6 +102,7 @@
void setHeight(float height) { m_size.setHeight(height); }
bool isEmpty() const { return m_size.isEmpty(); }
+ bool isZero() const { return m_size.isZero(); }
FloatPoint center() const { return FloatPoint(x() + width() / 2, y() + height() / 2); }
Modified: trunk/Source/WebCore/rendering/RenderText.cpp (91762 => 91763)
--- trunk/Source/WebCore/rendering/RenderText.cpp 2011-07-26 17:37:15 UTC (rev 91762)
+++ trunk/Source/WebCore/rendering/RenderText.cpp 2011-07-26 17:47:03 UTC (rev 91763)
@@ -272,6 +272,27 @@
rects.append(enclosingLayoutRect(FloatRect(accumulatedOffset + box->topLeft(), box->size())));
}
+static FloatRect absoluteQuadForTextBox(InlineTextBox* box, unsigned start, unsigned end, bool useSelectionHeight)
+{
+ unsigned realEnd = min(box->end() + 1, end);
+ IntRect r = box->selectionRect(start, realEnd);
+ if (r.height()) {
+ if (!useSelectionHeight) {
+ // Change the height and y position (or width and x for vertical text)
+ // because selectionRect uses selection-specific values.
+ if (box->isHorizontal()) {
+ r.setHeight(box->logicalHeight());
+ r.setY(box->y());
+ } else {
+ r.setWidth(box->logicalWidth());
+ r.setX(box->x());
+ }
+ }
+ return FloatRect(r);
+ }
+ return FloatRect();
+}
+
void RenderText::absoluteRectsForRange(Vector<IntRect>& rects, unsigned start, unsigned end, bool useSelectionHeight)
{
// Work around signed/unsigned issues. This function takes unsigneds, and is often passed UINT_MAX
@@ -300,21 +321,9 @@
}
rects.append(localToAbsoluteQuad(FloatQuad(r)).enclosingBoundingBox());
} else {
- unsigned realEnd = min(box->end() + 1, end);
- IntRect r = box->selectionRect(start, realEnd);
- if (!r.isEmpty()) {
- if (!useSelectionHeight) {
- // change the height and y position because selectionRect uses selection-specific values
- if (box->isHorizontal()) {
- r.setHeight(box->logicalHeight());
- r.setY(box->y());
- } else {
- r.setWidth(box->logicalWidth());
- r.setX(box->x());
- }
- }
- rects.append(localToAbsoluteQuad(FloatQuad(r)).enclosingBoundingBox());
- }
+ FloatRect rect = absoluteQuadForTextBox(box, start, end, useSelectionHeight);
+ if (!rect.isZero())
+ rects.append(localToAbsoluteQuad(rect).enclosingBoundingBox());
}
}
}
@@ -393,21 +402,9 @@
}
quads.append(localToAbsoluteQuad(FloatRect(r)));
} else {
- unsigned realEnd = min(box->end() + 1, end);
- IntRect r = box->selectionRect(start, realEnd);
- if (r.height()) {
- if (!useSelectionHeight) {
- // change the height and y position because selectionRect uses selection-specific values
- if (box->isHorizontal()) {
- r.setHeight(box->logicalHeight());
- r.setY(box->y());
- } else {
- r.setWidth(box->logicalHeight());
- r.setX(box->x());
- }
- }
- quads.append(localToAbsoluteQuad(FloatRect(r)));
- }
+ FloatRect rect = absoluteQuadForTextBox(box, start, end, useSelectionHeight);
+ if (!rect.isZero())
+ quads.append(localToAbsoluteQuad(rect));
}
}
}
