- Revision
- 171882
- Author
- [email protected]
- Date
- 2014-07-31 12:50:45 -0700 (Thu, 31 Jul 2014)
Log Message
REGRESSION: Search highlight is broken in RTL multicolumn content
https://bugs.webkit.org/show_bug.cgi?id=135452
Reviewed by Simon Fraser.
Source/WebCore:
The offsets for elements inside RTL multi-column elements are incorrectly computed because
the columns don't calculate their left position according to the writing direction.
The patch extracts the column position computation in two helper functions (for top and left)
so they can be used when needed in different parts of the code. In our case, the |columnLogicalLeft|
function should be used inside |columnTranslationForOffset|.
Test: fast/multicol/content-bounding-box-rtl.html
* rendering/RenderMultiColumnSet.cpp:
(WebCore::RenderMultiColumnSet::columnLogicalLeft): Return the logical left of a column relative to the set.
(WebCore::RenderMultiColumnSet::columnLogicalTop): Return the logical top of a column relative to the set.
(WebCore::RenderMultiColumnSet::columnRectAt): Split the code between columnLogicalLeft and columnLogicalTop.
(WebCore::RenderMultiColumnSet::collectLayerFragments): Make code clearer by adding a new line.
(WebCore::RenderMultiColumnSet::columnTranslationForOffset): Use columnLogicalLeft instead of duplicating logic.
* rendering/RenderMultiColumnSet.h:
LayoutTests:
A test that verifies the bounding boxes for content inside a RTL multi-column element are correctly computed:
- for static elements
- for relative positioned elements
- for absolutely positioned elements
* fast/multicol/content-bounding-box-rtl-expected.txt: Added.
* fast/multicol/content-bounding-box-rtl.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (171881 => 171882)
--- trunk/LayoutTests/ChangeLog 2014-07-31 19:47:40 UTC (rev 171881)
+++ trunk/LayoutTests/ChangeLog 2014-07-31 19:50:45 UTC (rev 171882)
@@ -1,3 +1,18 @@
+2014-07-31 Andrei Bucur <[email protected]>
+
+ REGRESSION: Search highlight is broken in RTL multicolumn content
+ https://bugs.webkit.org/show_bug.cgi?id=135452
+
+ Reviewed by Simon Fraser.
+
+ A test that verifies the bounding boxes for content inside a RTL multi-column element are correctly computed:
+ - for static elements
+ - for relative positioned elements
+ - for absolutely positioned elements
+
+ * fast/multicol/content-bounding-box-rtl-expected.txt: Added.
+ * fast/multicol/content-bounding-box-rtl.html: Added.
+
2014-07-31 Bear Travis <[email protected]>
[CSS Font Loading] Test expectations should show success
Added: trunk/LayoutTests/fast/multicol/content-bounding-box-rtl-expected.txt (0 => 171882)
--- trunk/LayoutTests/fast/multicol/content-bounding-box-rtl-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/multicol/content-bounding-box-rtl-expected.txt 2014-07-31 19:50:45 UTC (rev 171882)
@@ -0,0 +1,2 @@
+Test for https://bugs.webkit.org/show_bug.cgi?id=135452
+PASS
Added: trunk/LayoutTests/fast/multicol/content-bounding-box-rtl.html (0 => 171882)
--- trunk/LayoutTests/fast/multicol/content-bounding-box-rtl.html (rev 0)
+++ trunk/LayoutTests/fast/multicol/content-bounding-box-rtl.html 2014-07-31 19:50:45 UTC (rev 171882)
@@ -0,0 +1,71 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ html, body {
+ margin: 0px;
+ padding: 0px;
+ }
+
+ .relative {
+ position: relative;
+ }
+
+ .absolute {
+ position: absolute;
+ }
+
+ #multicol {
+ position: absolute;
+ left: 0;
+ top: 0px;
+ width: 600px;
+ -webkit-column-count: 3;
+ font-size: 30px;
+ font-family: Ahem;
+ -webkit-column-gap: 50px;
+ }
+ </style>
+</head>
+<body>
+ <div id="multicol" dir="rtl">
+ <span class="overlay-target">first</span> <span class="overlay-target relative">second</span> <span class="overlay-target">third</span><br>
+ <span class="overlay-target">fourth</span> <span class="overlay-target">fifth</span> <span class="overlay-target absolute">sixth</span>
+ </div>
+ <script type="text/_javascript_">
+ if (window.testRunner)
+ window.testRunner.dumpAsText();
+
+ var boxes = document.getElementsByClassName("overlay-target");
+ var expectedResults = [{"top":0,"left":450,"width":150,"height":30},
+ {"top":30,"left":420,"width":180,"height":30},
+ {"top":0,"left":233.34375,"width":150,"height":30},
+ {"top":30,"left":203.34375,"width":180,"height":30},
+ {"top":0,"left":16.6875,"width":150,"height":30},
+ {"top":150,"left":450,"width":150,"height":30}];
+
+ var errorString = null;
+ for (var i = 0; i < boxes.length; i++) {
+ var bbox = boxes[i].getBoundingClientRect();
+ bboxObject = {
+ top: bbox.top,
+ left: bbox.left,
+ width: bbox.width,
+ height: bbox.height
+ }
+ var expected = expectedResults[i];
+ if ((bboxObject.left === expected.left) && (bboxObject.top === expected.top)
+ && (bboxObject.width === expected.width) && (bboxObject.height === expected.height))
+ continue;
+
+ errorString = (errorString || "") + "Expected: " + JSON.stringify(expected) + " Got: " + JSON.stringify(bboxObject) + "<br>";
+ }
+
+ var prefix = "Test for https://bugs.webkit.org/show_bug.cgi?id=135452<br>";
+ if (errorString)
+ document.body.innerHTML = prefix + "FAIL<br>" + errorString;
+ else
+ document.body.innerHTML = prefix + "PASS";
+ </script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (171881 => 171882)
--- trunk/Source/WebCore/ChangeLog 2014-07-31 19:47:40 UTC (rev 171881)
+++ trunk/Source/WebCore/ChangeLog 2014-07-31 19:50:45 UTC (rev 171882)
@@ -1,3 +1,27 @@
+2014-07-31 Andrei Bucur <[email protected]>
+
+ REGRESSION: Search highlight is broken in RTL multicolumn content
+ https://bugs.webkit.org/show_bug.cgi?id=135452
+
+ Reviewed by Simon Fraser.
+
+ The offsets for elements inside RTL multi-column elements are incorrectly computed because
+ the columns don't calculate their left position according to the writing direction.
+
+ The patch extracts the column position computation in two helper functions (for top and left)
+ so they can be used when needed in different parts of the code. In our case, the |columnLogicalLeft|
+ function should be used inside |columnTranslationForOffset|.
+
+ Test: fast/multicol/content-bounding-box-rtl.html
+
+ * rendering/RenderMultiColumnSet.cpp:
+ (WebCore::RenderMultiColumnSet::columnLogicalLeft): Return the logical left of a column relative to the set.
+ (WebCore::RenderMultiColumnSet::columnLogicalTop): Return the logical top of a column relative to the set.
+ (WebCore::RenderMultiColumnSet::columnRectAt): Split the code between columnLogicalLeft and columnLogicalTop.
+ (WebCore::RenderMultiColumnSet::collectLayerFragments): Make code clearer by adding a new line.
+ (WebCore::RenderMultiColumnSet::columnTranslationForOffset): Use columnLogicalLeft instead of duplicating logic.
+ * rendering/RenderMultiColumnSet.h:
+
2014-07-31 Martin Hodovan <[email protected]>
Eliminate "FractionConversion" from CSSPrimitiveValue::convertToLength
Modified: trunk/Source/WebCore/rendering/RenderMultiColumnSet.cpp (171881 => 171882)
--- trunk/Source/WebCore/rendering/RenderMultiColumnSet.cpp 2014-07-31 19:47:40 UTC (rev 171881)
+++ trunk/Source/WebCore/rendering/RenderMultiColumnSet.cpp 2014-07-31 19:50:45 UTC (rev 171882)
@@ -444,32 +444,52 @@
return count;
}
-LayoutRect RenderMultiColumnSet::columnRectAt(unsigned index) const
+LayoutUnit RenderMultiColumnSet::columnLogicalLeft(unsigned index) const
{
LayoutUnit colLogicalWidth = computedColumnWidth();
- LayoutUnit colLogicalHeight = computedColumnHeight();
- LayoutUnit colLogicalTop = borderAndPaddingBefore();
LayoutUnit colLogicalLeft = borderAndPaddingLogicalLeft();
LayoutUnit colGap = columnGap();
-
+
bool progressionReversed = multiColumnFlowThread()->progressionIsReversed();
bool progressionInline = multiColumnFlowThread()->progressionIsInline();
-
+
if (progressionInline) {
if (style().isLeftToRightDirection() ^ progressionReversed)
colLogicalLeft += index * (colLogicalWidth + colGap);
else
colLogicalLeft += contentLogicalWidth() - colLogicalWidth - index * (colLogicalWidth + colGap);
- } else {
+ }
+
+ return colLogicalLeft;
+}
+
+LayoutUnit RenderMultiColumnSet::columnLogicalTop(unsigned index) const
+{
+ LayoutUnit colLogicalHeight = computedColumnHeight();
+ LayoutUnit colLogicalTop = borderAndPaddingBefore();
+ LayoutUnit colGap = columnGap();
+
+ bool progressionReversed = multiColumnFlowThread()->progressionIsReversed();
+ bool progressionInline = multiColumnFlowThread()->progressionIsInline();
+
+ if (!progressionInline) {
if (!progressionReversed)
colLogicalTop += index * (colLogicalHeight + colGap);
else
colLogicalTop += contentLogicalHeight() - colLogicalHeight - index * (colLogicalHeight + colGap);
}
-
+
+ return colLogicalTop;
+}
+
+LayoutRect RenderMultiColumnSet::columnRectAt(unsigned index) const
+{
+ LayoutUnit colLogicalWidth = computedColumnWidth();
+ LayoutUnit colLogicalHeight = computedColumnHeight();
+
if (isHorizontalWritingMode())
- return LayoutRect(colLogicalLeft, colLogicalTop, colLogicalWidth, colLogicalHeight);
- return LayoutRect(colLogicalTop, colLogicalLeft, colLogicalHeight, colLogicalWidth);
+ return LayoutRect(columnLogicalLeft(index), columnLogicalTop(index), colLogicalWidth, colLogicalHeight);
+ return LayoutRect(columnLogicalTop(index), columnLogicalLeft(index), colLogicalHeight, colLogicalWidth);
}
unsigned RenderMultiColumnSet::columnIndexAtOffset(LayoutUnit offset, ColumnIndexCalculationMode mode) const
@@ -766,6 +786,7 @@
inlineOffset += contentLogicalWidth() - colLogicalWidth;
}
translationOffset.setWidth(inlineOffset);
+
LayoutUnit blockOffset = initialBlockOffset + logicalTop() - flowThread()->logicalTop() + (isHorizontalWritingMode() ? -flowThreadPortion.y() : -flowThreadPortion.x());
if (!progressionIsInline) {
if (!progressionReversed)
@@ -807,7 +828,6 @@
unsigned startColumn = columnIndexAtOffset(offset);
LayoutUnit colGap = columnGap();
- LayoutUnit colLogicalWidth = computedColumnWidth();
LayoutRect flowThreadPortion = flowThreadPortionRectAt(startColumn);
LayoutPoint translationOffset;
@@ -817,15 +837,8 @@
LayoutUnit initialBlockOffset = initialBlockOffsetForPainting();
- LayoutUnit inlineOffset = progressionIsInline ? startColumn * (colLogicalWidth + colGap) : LayoutUnit();
-
- bool leftToRight = style().isLeftToRightDirection() ^ progressionReversed;
- if (!leftToRight) {
- inlineOffset = -inlineOffset;
- if (progressionReversed)
- inlineOffset += contentLogicalWidth() - colLogicalWidth;
- }
- translationOffset.setX(inlineOffset);
+ translationOffset.setX(columnLogicalLeft(startColumn));
+
LayoutUnit blockOffset = initialBlockOffset - (isHorizontalWritingMode() ? flowThreadPortion.y() : flowThreadPortion.x());
if (!progressionIsInline) {
if (!progressionReversed)
Modified: trunk/Source/WebCore/rendering/RenderMultiColumnSet.h (171881 => 171882)
--- trunk/Source/WebCore/rendering/RenderMultiColumnSet.h 2014-07-31 19:47:40 UTC (rev 171881)
+++ trunk/Source/WebCore/rendering/RenderMultiColumnSet.h 2014-07-31 19:50:45 UTC (rev 171882)
@@ -160,6 +160,9 @@
LayoutUnit calculateMaxColumnHeight() const;
LayoutUnit columnGap() const;
+ LayoutUnit columnLogicalLeft(unsigned) const;
+ LayoutUnit columnLogicalTop(unsigned) const;
+
LayoutRect flowThreadPortionRectAt(unsigned index) const;
LayoutRect flowThreadPortionOverflowRect(const LayoutRect& flowThreadPortion, unsigned index, unsigned colCount, LayoutUnit colGap);
