<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[171882] trunk</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/171882">171882</a></dd>
<dt>Author</dt> <dd>abucur@adobe.com</dd>
<dt>Date</dt> <dd>2014-07-31 12:50:45 -0700 (Thu, 31 Jul 2014)</dd>
</dl>

<h3>Log Message</h3>
<pre>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.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderMultiColumnSetcpp">trunk/Source/WebCore/rendering/RenderMultiColumnSet.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderMultiColumnSeth">trunk/Source/WebCore/rendering/RenderMultiColumnSet.h</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastmulticolcontentboundingboxrtlexpectedtxt">trunk/LayoutTests/fast/multicol/content-bounding-box-rtl-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastmulticolcontentboundingboxrtlhtml">trunk/LayoutTests/fast/multicol/content-bounding-box-rtl.html</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (171881 => 171882)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog        2014-07-31 19:47:40 UTC (rev 171881)
+++ trunk/LayoutTests/ChangeLog        2014-07-31 19:50:45 UTC (rev 171882)
</span><span class="lines">@@ -1,3 +1,18 @@
</span><ins>+2014-07-31  Andrei Bucur  &lt;abucur@adobe.com&gt;
+
+        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.
+
</ins><span class="cx"> 2014-07-31  Bear Travis  &lt;betravis@adobe.com&gt;
</span><span class="cx"> 
</span><span class="cx">         [CSS Font Loading] Test expectations should show success
</span></span></pre></div>
<a id="trunkLayoutTestsfastmulticolcontentboundingboxrtlexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/multicol/content-bounding-box-rtl-expected.txt (0 => 171882)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,2 @@
</span><ins>+Test for https://bugs.webkit.org/show_bug.cgi?id=135452
+PASS
</ins></span></pre></div>
<a id="trunkLayoutTestsfastmulticolcontentboundingboxrtlhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/multicol/content-bounding-box-rtl.html (0 => 171882)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,71 @@
</span><ins>+&lt;!DOCTYPE html&gt;
+&lt;html&gt;
+&lt;head&gt;
+    &lt;style&gt;
+        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;
+        }
+    &lt;/style&gt;
+&lt;/head&gt;
+&lt;body&gt;
+    &lt;div id=&quot;multicol&quot; dir=&quot;rtl&quot;&gt;
+        &lt;span class=&quot;overlay-target&quot;&gt;first&lt;/span&gt; &lt;span class=&quot;overlay-target relative&quot;&gt;second&lt;/span&gt; &lt;span class=&quot;overlay-target&quot;&gt;third&lt;/span&gt;&lt;br&gt;
+        &lt;span class=&quot;overlay-target&quot;&gt;fourth&lt;/span&gt; &lt;span class=&quot;overlay-target&quot;&gt;fifth&lt;/span&gt; &lt;span class=&quot;overlay-target absolute&quot;&gt;sixth&lt;/span&gt;
+    &lt;/div&gt;
+    &lt;script type=&quot;text/javascript&quot;&gt;
+        if (window.testRunner)
+            window.testRunner.dumpAsText();
+
+        var boxes = document.getElementsByClassName(&quot;overlay-target&quot;);
+        var expectedResults = [{&quot;top&quot;:0,&quot;left&quot;:450,&quot;width&quot;:150,&quot;height&quot;:30},
+                            {&quot;top&quot;:30,&quot;left&quot;:420,&quot;width&quot;:180,&quot;height&quot;:30},
+                            {&quot;top&quot;:0,&quot;left&quot;:233.34375,&quot;width&quot;:150,&quot;height&quot;:30},
+                            {&quot;top&quot;:30,&quot;left&quot;:203.34375,&quot;width&quot;:180,&quot;height&quot;:30},
+                            {&quot;top&quot;:0,&quot;left&quot;:16.6875,&quot;width&quot;:150,&quot;height&quot;:30},
+                            {&quot;top&quot;:150,&quot;left&quot;:450,&quot;width&quot;:150,&quot;height&quot;:30}];
+
+        var errorString = null;
+        for (var i = 0; i &lt; 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) &amp;&amp; (bboxObject.top === expected.top)
+                &amp;&amp; (bboxObject.width === expected.width) &amp;&amp; (bboxObject.height === expected.height))
+                continue;
+
+            errorString = (errorString || &quot;&quot;) + &quot;Expected: &quot; + JSON.stringify(expected) + &quot; Got: &quot; + JSON.stringify(bboxObject) + &quot;&lt;br&gt;&quot;;
+        }
+
+        var prefix = &quot;Test for https://bugs.webkit.org/show_bug.cgi?id=135452&lt;br&gt;&quot;;
+        if (errorString)
+            document.body.innerHTML = prefix + &quot;FAIL&lt;br&gt;&quot; + errorString;
+        else
+            document.body.innerHTML = prefix + &quot;PASS&quot;;
+    &lt;/script&gt;
+&lt;/body&gt;
+&lt;/html&gt;
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (171881 => 171882)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,27 @@
</span><ins>+2014-07-31  Andrei Bucur  &lt;abucur@adobe.com&gt;
+
+        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:
+
</ins><span class="cx"> 2014-07-31  Martin Hodovan  &lt;mhodovan.u-szeged@partner.samsung.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Eliminate &quot;FractionConversion&quot; from CSSPrimitiveValue::convertToLength
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderMultiColumnSetcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderMultiColumnSet.cpp (171881 => 171882)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -444,32 +444,52 @@
</span><span class="cx">     return count;
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-LayoutRect RenderMultiColumnSet::columnRectAt(unsigned index) const
</del><ins>+LayoutUnit RenderMultiColumnSet::columnLogicalLeft(unsigned index) const
</ins><span class="cx"> {
</span><span class="cx">     LayoutUnit colLogicalWidth = computedColumnWidth();
</span><del>-    LayoutUnit colLogicalHeight = computedColumnHeight();
-    LayoutUnit colLogicalTop = borderAndPaddingBefore();
</del><span class="cx">     LayoutUnit colLogicalLeft = borderAndPaddingLogicalLeft();
</span><span class="cx">     LayoutUnit colGap = columnGap();
</span><del>-    
</del><ins>+
</ins><span class="cx">     bool progressionReversed = multiColumnFlowThread()-&gt;progressionIsReversed();
</span><span class="cx">     bool progressionInline = multiColumnFlowThread()-&gt;progressionIsInline();
</span><del>-    
</del><ins>+
</ins><span class="cx">     if (progressionInline) {
</span><span class="cx">         if (style().isLeftToRightDirection() ^ progressionReversed)
</span><span class="cx">             colLogicalLeft += index * (colLogicalWidth + colGap);
</span><span class="cx">         else
</span><span class="cx">             colLogicalLeft += contentLogicalWidth() - colLogicalWidth - index * (colLogicalWidth + colGap);
</span><del>-    } else {
</del><ins>+    }
+
+    return colLogicalLeft;
+}
+
+LayoutUnit RenderMultiColumnSet::columnLogicalTop(unsigned index) const
+{
+    LayoutUnit colLogicalHeight = computedColumnHeight();
+    LayoutUnit colLogicalTop = borderAndPaddingBefore();
+    LayoutUnit colGap = columnGap();
+
+    bool progressionReversed = multiColumnFlowThread()-&gt;progressionIsReversed();
+    bool progressionInline = multiColumnFlowThread()-&gt;progressionIsInline();
+
+    if (!progressionInline) {
</ins><span class="cx">         if (!progressionReversed)
</span><span class="cx">             colLogicalTop += index * (colLogicalHeight + colGap);
</span><span class="cx">         else
</span><span class="cx">             colLogicalTop += contentLogicalHeight() - colLogicalHeight - index * (colLogicalHeight + colGap);
</span><span class="cx">     }
</span><del>-    
</del><ins>+
+    return colLogicalTop;
+}
+
+LayoutRect RenderMultiColumnSet::columnRectAt(unsigned index) const
+{
+    LayoutUnit colLogicalWidth = computedColumnWidth();
+    LayoutUnit colLogicalHeight = computedColumnHeight();
+
</ins><span class="cx">     if (isHorizontalWritingMode())
</span><del>-        return LayoutRect(colLogicalLeft, colLogicalTop, colLogicalWidth, colLogicalHeight);
-    return LayoutRect(colLogicalTop, colLogicalLeft, colLogicalHeight, colLogicalWidth);
</del><ins>+        return LayoutRect(columnLogicalLeft(index), columnLogicalTop(index), colLogicalWidth, colLogicalHeight);
+    return LayoutRect(columnLogicalTop(index), columnLogicalLeft(index), colLogicalHeight, colLogicalWidth);
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> unsigned RenderMultiColumnSet::columnIndexAtOffset(LayoutUnit offset, ColumnIndexCalculationMode mode) const
</span><span class="lines">@@ -766,6 +786,7 @@
</span><span class="cx">                 inlineOffset += contentLogicalWidth() - colLogicalWidth;
</span><span class="cx">         }
</span><span class="cx">         translationOffset.setWidth(inlineOffset);
</span><ins>+
</ins><span class="cx">         LayoutUnit blockOffset = initialBlockOffset + logicalTop() - flowThread()-&gt;logicalTop() + (isHorizontalWritingMode() ? -flowThreadPortion.y() : -flowThreadPortion.x());
</span><span class="cx">         if (!progressionIsInline) {
</span><span class="cx">             if (!progressionReversed)
</span><span class="lines">@@ -807,7 +828,6 @@
</span><span class="cx">     unsigned startColumn = columnIndexAtOffset(offset);
</span><span class="cx">     
</span><span class="cx">     LayoutUnit colGap = columnGap();
</span><del>-    LayoutUnit colLogicalWidth = computedColumnWidth();
</del><span class="cx">     
</span><span class="cx">     LayoutRect flowThreadPortion = flowThreadPortionRectAt(startColumn);
</span><span class="cx">     LayoutPoint translationOffset;
</span><span class="lines">@@ -817,15 +837,8 @@
</span><span class="cx"> 
</span><span class="cx">     LayoutUnit initialBlockOffset = initialBlockOffsetForPainting();
</span><span class="cx">     
</span><del>-    LayoutUnit inlineOffset = progressionIsInline ? startColumn * (colLogicalWidth + colGap) : LayoutUnit();
-    
-    bool leftToRight = style().isLeftToRightDirection() ^ progressionReversed;
-    if (!leftToRight) {
-        inlineOffset = -inlineOffset;
-        if (progressionReversed)
-            inlineOffset += contentLogicalWidth() - colLogicalWidth;
-    }
-    translationOffset.setX(inlineOffset);
</del><ins>+    translationOffset.setX(columnLogicalLeft(startColumn));
+
</ins><span class="cx">     LayoutUnit blockOffset = initialBlockOffset - (isHorizontalWritingMode() ? flowThreadPortion.y() : flowThreadPortion.x());
</span><span class="cx">     if (!progressionIsInline) {
</span><span class="cx">         if (!progressionReversed)
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderMultiColumnSeth"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderMultiColumnSet.h (171881 => 171882)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -160,6 +160,9 @@
</span><span class="cx">     LayoutUnit calculateMaxColumnHeight() const;
</span><span class="cx">     LayoutUnit columnGap() const;
</span><span class="cx"> 
</span><ins>+    LayoutUnit columnLogicalLeft(unsigned) const;
+    LayoutUnit columnLogicalTop(unsigned) const;
+
</ins><span class="cx">     LayoutRect flowThreadPortionRectAt(unsigned index) const;
</span><span class="cx">     LayoutRect flowThreadPortionOverflowRect(const LayoutRect&amp; flowThreadPortion, unsigned index, unsigned colCount, LayoutUnit colGap);
</span><span class="cx"> 
</span></span></pre>
</div>
</div>

</body>
</html>