[css-grid] Fix positioned children in RTL direction

third_party/WebKit/Source/core/layout/LayoutGrid.cpp

Index: third_party/WebKit/Source/core/layout/LayoutGrid.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutGrid.cpp b/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
index 9f538821e67418d11cc5f8fc6fef418f1c31d3ea..97fc714a322e73bd4e0b3a9c100e126c40924712 100644
--- a/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
@@ -1522,23 +1522,38 @@ void LayoutGrid::offsetAndBreadthForPositionedChild(const LayoutBox& child, Grid
         || (endLine < firstExplicitLine)
         || (endLine > lastExplicitLine);
 
-    LayoutUnit start = startIsAuto ? LayoutUnit() : isForColumns ?  m_columnPositions[startLine] : m_rowPositions[startLine];
-    LayoutUnit end = endIsAuto ? isForColumns ? logicalWidth() : logicalHeight() : isForColumns ?  m_columnPositions[endLine] : m_rowPositions[endLine];
-
-    breadth = end - start;
-
-    if (startIsAuto)
-        breadth -= isForColumns ? borderStart() : borderBefore();
-    else
-        start -= isForColumns ? borderStart() : borderBefore();
+    LayoutUnit start;
+    if (startIsAuto) {
+        start = LayoutUnit();
+    } else {
+        start = isForColumns ? m_columnPositions[startLine] : m_rowPositions[startLine];
+        start -= isForColumns ? m_columnPositions[0] : m_rowPositions[0];
+        start += isForColumns ? paddingStart() : paddingBefore();

[svillar, 2016/03/23 10:43:55]

Better than the nested ternary but still difficult to follow the flow (also you
don't need the startIsAuto branch). I suggest

LayoutUnit start;
if (!startIsAuto) {
    if (isForColumns)
        start = m_columnPositions[startLine]  - m_columnPositions[0] +
paddingStart();
    else
        start = m_rowPositions[startLine] - m_rowPositions[0] + paddingBefore();
}

[Manuel Rego, 2016/03/23 11:20:52]

Done.

+    }
 
+    LayoutUnit end;
     if (endIsAuto) {
-        breadth -= isForColumns ? borderEnd() : borderAfter();
-        breadth -= scrollbarLogicalWidth();
+        end = isForColumns ? clientLogicalWidth() : clientLogicalHeight();
+    } else {
+        end = isForColumns ? m_columnPositions[endLine] : m_rowPositions[endLine];
+        end -= isForColumns ? m_columnPositions[0] : m_rowPositions[0];
+        end += isForColumns ? paddingStart() : paddingBefore();
     }

[svillar, 2016/03/23 10:43:55]

Same comment than for the start block

[Manuel Rego, 2016/03/23 11:20:52]

Done.

 
+    breadth = end - start;
     offset = start;
 
+    if (isForColumns && !styleRef().isLeftToRightDirection() && !child.styleRef().hasStaticInlinePosition(child.isHorizontalWritingMode())) {
+        // If the child doesn't have a static inline position, we need to calculate the offset from the left (even if we're in RTL).
+        if (endIsAuto) {
+            offset = LayoutUnit();
+        } else {
+            LayoutUnit alignmentOffset =  m_columnPositions[0] - borderAndPaddingStart();
+            LayoutUnit offsetFromLastLine = m_columnPositions[m_columnPositions.size() - 1] - m_columnPositions[endLine];
+            offset = paddingLeft() +  alignmentOffset + offsetFromLastLine;
+        }

[svillar, 2016/03/23 10:43:55]

I don't get this. This CL is about fixing the behavior of positioning children
in RTL and you add a block to handle non-static items?

[Manuel Rego, 2016/03/23 11:20:52]

I think you're mixing concepts here.

Of course this CL only has to do with positioned children.
This code is only executed for those elements.

The thing that is probably confusing you is the term "static position"
(https://www.w3.org/TR/CSS21/visudet.html#static-position)
and the "static" value in "position: static" vs "postiion: absolute".

Let's use an example of a regular block:
<div style="position: relative; border: 5px solid; padding: 10px;">
  <div style="position: absolute; background: lime;">test</div>
</div>

By default "left: auto;", the test element will use the static position,
it'll be placed in the padding: 15px (5px + 10px).
In this case hasStaticInlinePosition() returns TRUE.

However if you set "left: 0px" then it'd be placed in the border:
5px (5px + 0px). Here hasStaticInlinePosition() returns FALSE.

+    }
+
     if (child.parent() == this && !startIsAuto) {
         // If column/row start is "auto" the static position has been already set in prepareChildForPositionedLayout().
         PaintLayer* childLayer = child.layer();
@@ -1547,6 +1562,7 @@ void LayoutGrid::offsetAndBreadthForPositionedChild(const LayoutBox& child, Grid
         else
             childLayer->setStaticBlockPosition(borderBefore() + offset);
     }
+

[svillar, 2016/03/23 10:43:55]

Extra line.

[Manuel Rego, 2016/03/23 11:20:52]

Acknowledged.

 }
 
 GridArea LayoutGrid::cachedGridArea(const LayoutBox& gridItem) const