[css-align] New CSS Value 'normal' for Self Alignment

third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp

Index: third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp
diff --git a/third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp b/third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp
index 5d363848d3c5ab03df6601b0635173eddeff537e..8a3a251c2dec6c590fc76ca27d5c632173a34478 100644
--- a/third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp
+++ b/third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp
@@ -243,7 +243,10 @@ void StyleAdjuster::adjustComputedStyle(ComputedStyle& style, const ComputedStyl
         if (isSVGTextElement(*element))
             style.clearMultiCol();
     }
-    adjustStyleForAlignment(style, parentStyle);
+    if (element && element->layoutObject() && element->layoutObject()->containingBlock()->isAnonymous())
+        adjustStyleForAlignment(style, element->layoutObject()->containingBlock()->styleRef());
+    else
+        adjustStyleForAlignment(style, parentStyle);

[rune, 2016/06/13 12:24:19]

I still have a hard time understanding the spec here. The spec says align-items
applies to block level elements, yet the computed value is still the specified
value, and align-items does not affect the element itself. align-self refers to
the computed value for the parent align-items, which means the align-self:auto
can resolve its auto-value from an inline parent. I'm not sure if that is what
the spec was meant to say, but that means this code is wrong if you have:

<span style="align-items: center">
  <span style="align-self: auto"></span>
</span>

and the outer span for some reason is wrapped into an anonymous block box.

Another thing is that anonymous boxes are often created after you calculate the
style for its children. In the case of tables, you calculate the style of a
direct child of a table, if it's not some table-* display type you need to wrap
it in an anonymous table cell, anonymous row, etc. But, you don't know if you
have to do that before you have calculated and adjusted the style for that
element.

[jfernandez, 2016/06/13 13:00:36]

That's how I understand the spec, yes. An element's align-items property is
intended to
be used its children's align-self property to resolve any 'auto' value. I guess
this applies
as well to inline hierarchies, indeed. 
After Tab's reply to my question (posted in an earlier comment in this review)
"Parent
element" refers to the element tree; it doesn't care about box-tree structure."
So, inner span's 'auto' value will be resolved from its original parent element.
The code above is intended to be triggered DURING layout, AFTER the style was
calculated. The idea
is that wrapping layout objects into Anonymous boxes will imply a style recalc
on 
those objects, so only in that case the new parent will be used. Clearly the
spec does
not mention that case, but we indeed want that behavior eg. FullScreen). 

The problem I see, with my current knowledge, with the Full Screen case is that
we
want a new parent to propagate its align-items value to its new children. We
could
resolve the opposite case (new children. anonymous or not, resolving its value
to
its new parent) but not a re-parenting case. The reason is that getter methods
for the
ComputedStyle are supposed to be const, as Layout is not the place to do this
kind
of style calculations. We could copy the styles of all the children, modifying
this new
ComputedStyle instance and assign it back again, but I guess it's a brute force
approach
with many problems.   
So far, Alignment is only implemented in Flexible Box and Grid layout models.
Even that
parsing and computed style resolution should be the common logic, I guess
specific
layout models will have to implement it at some point and dealing with its
particularities.

 }
 
 void StyleAdjuster::adjustStyleForEditing(ComputedStyle& style)
@@ -273,38 +276,31 @@ void StyleAdjuster::adjustStyleForFirstLetter(ComputedStyle& style)
 
 void StyleAdjuster::adjustStyleForAlignment(ComputedStyle& style, const ComputedStyle& parentStyle)
 {
-    bool isFlexOrGrid = style.isDisplayFlexibleOrGridBox();
-    bool absolutePositioned = style.position() == AbsolutePosition;
+    // To avoid needing to copy the RareNonInheritedData, we repurpose the 'auto' flag
+    // to not just mean 'auto' prior to running the StyleAdjuster but also mean 'normal'
+    // after running it.
 
-    // If the inherited value of justify-items includes the legacy keyword, 'auto'
+    // If the inherited value of justify-items includes the 'legacy' keyword, 'auto'
     // computes to the the inherited value.
-    // Otherwise, auto computes to:
-    //  - 'stretch' for flex containers and grid containers.
-    //  - 'start' for everything else.
+    // Otherwise, 'auto' computes to 'normal'.
     if (style.justifyItemsPosition() == ItemPositionAuto) {
         if (parentStyle.justifyItemsPositionType() == LegacyPosition)
             style.setJustifyItems(parentStyle.justifyItems());
-        else if (isFlexOrGrid)
-            style.setJustifyItemsPosition(ItemPositionStretch);
     }
 
-    // The 'auto' keyword computes to 'stretch' on absolutely-positioned elements,
-    // and to the computed value of justify-items on the parent (minus
-    // any legacy keywords) on all other boxes.
+    // The 'auto' keyword computes the computed value of justify-items on the parent (minus
+    // any legacy keywords), or 'normal' if the box has no parent.
     if (style.justifySelfPosition() == ItemPositionAuto) {
-        if (absolutePositioned)
-            style.setJustifySelfPosition(ItemPositionStretch);
-        else
+        if (parentStyle.justifyItemsPositionType() == LegacyPosition)
+            style.setJustifySelfPosition(parentStyle.justifyItemsPosition());
+        else if (parentStyle.justifyItemsPosition() != ItemPositionAuto)
             style.setJustifySelf(parentStyle.justifyItems());
     }
 
-    // The 'auto' keyword computes to:
-    //  - 'stretch' for flex containers and grid containers,
-    //  - 'start' for everything else.
-    if (style.alignItemsPosition() == ItemPositionAuto) {
-        if (isFlexOrGrid)
-            style.setAlignItemsPosition(ItemPositionStretch);
-    }
+    // The 'auto' keyword computes the computed value of align-items on the parent
+    // or 'normal' if the box has no parent.
+    if (style.alignSelfPosition() == ItemPositionAuto && parentStyle.alignItemsPosition() != ItemPositionNormal)
+        style.setAlignSelf(parentStyle.alignItems());
 }
 
 void StyleAdjuster::adjustStyleForHTMLElement(ComputedStyle& style, const ComputedStyle& parentStyle, HTMLElement& element)