Changeset 195284 in webkit

Timestamp:

Jan 19, 2016 3:36:11 AM (8 years ago)

Author:

jfernandez@igalia.com

Message:

[css-grid][css-align] justify-self stretch is not applied for img elements
​https://bugs.webkit.org/show_bug.cgi?id=153206

Reviewed by Darin Adler.

Source/WebCore:

When computing the logical height, we check first if there is an override
height value set as a consequence of the stretching logic, so we use it
directly for any kind of element. However, in the case of the width
computation, we don't use such override value because it's the default
behavior of block-level boxes.

However, we consider some special cases which have to be treated as
replaced elements. Theses cases are evaluated first, so we don't let the
regular width computation logic to be executed, which is what we want
to implement the stretch behavior.

In order to let replaced elements, such images, to be stretched as a
consequence of the CSS alignment properties, we need to exclude grid
items from the cases to be treated as replaced elements during the width
computation.

Test: fast/css-grid-layout/grid-align-stretching-replaced-items.html

• rendering/RenderBox.cpp:

(WebCore::RenderBox::computeLogicalWidthInRegion):
(WebCore::RenderBox::hasStretchedLogicalWidth):
(WebCore::RenderBox::sizesLogicalWidthToFitContent):

• rendering/RenderBox.h:

LayoutTests:

Test to verify Replaced Elements, like images, are stretched in the inline
axis, when fulfilling the Box Alignment restrictions on this regard.

• fast/css-grid-layout/grid-align-stretching-replaced-items-expected.txt: Added.
• fast/css-grid-layout/grid-align-stretching-replaced-items.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/css-grid-layout/grid-align-stretching-replaced-items-expected.txt (added)
• LayoutTests/fast/css-grid-layout/grid-align-stretching-replaced-items.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderBox.cpp (modified) (3 diffs)
• Source/WebCore/rendering/RenderBox.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-01-19  Javier Fernandez  <jfernandez@igalia.com>
+
+        [css-grid][css-align] justify-self stretch is not applied for img elements
+        https://bugs.webkit.org/show_bug.cgi?id=153206
+
+        Reviewed by Darin Adler.
+
+        Test to verify Replaced Elements, like images, are stretched in the inline
+        axis, when fulfilling the Box Alignment restrictions on this regard.
+
+        * fast/css-grid-layout/grid-align-stretching-replaced-items-expected.txt: Added.
+        * fast/css-grid-layout/grid-align-stretching-replaced-items.html: Added.
+
 2016-01-19  Ryosuke Niwa  <rniwa@webkit.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-01-19  Javier Fernandez  <jfernandez@igalia.com>
+
+        [css-grid][css-align] justify-self stretch is not applied for img elements
+        https://bugs.webkit.org/show_bug.cgi?id=153206
+
+        Reviewed by Darin Adler.
+
+        When computing the logical height, we check first if there is an override
+        height value set as a consequence of the stretching logic, so we use it
+        directly for any kind of element. However, in the case of the width
+        computation, we don't use such override value because it's the default
+        behavior of block-level boxes.
+
+        However, we consider some special cases which have to be treated as
+        replaced elements. Theses cases are evaluated first, so we don't let the
+        regular width computation logic to be executed, which is what we want
+        to implement the stretch behavior.
+
+        In order to let replaced elements, such images, to be stretched as a
+        consequence of the CSS alignment properties, we need to exclude grid
+        items from the cases to be treated as replaced elements during the width
+        computation.
+
+        Test: fast/css-grid-layout/grid-align-stretching-replaced-items.html
+
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::computeLogicalWidthInRegion):
+        (WebCore::RenderBox::hasStretchedLogicalWidth):
+        (WebCore::RenderBox::sizesLogicalWidthToFitContent):
+        * rendering/RenderBox.h:
+
 2016-01-19  Ryosuke Niwa  <rniwa@webkit.org>
 

trunk/Source/WebCore/rendering/RenderBox.cpp

@@ -2370 +2370 @@
     bool inVerticalBox = parent()->isDeprecatedFlexibleBox() && (parent()->style().boxOrient() == VERTICAL);
     bool stretching = (parent()->style().boxAlign() == BSTRETCH);
+    // FIXME: Stretching is the only reason why we don't want the box to be treated as a replaced element, so we could perhaps
+    // refactor all this logic, not only for flex and grid since alignment is intended to be applied to any block.
     bool treatAsReplaced = shouldComputeSizeAsReplaced() && (!inVerticalBox || !stretching);
+#if ENABLE(CSS_GRID_LAYOUT)
+    treatAsReplaced = treatAsReplaced && (!isGridItem() || !hasStretchedLogicalWidth());
+#endif
 
     const RenderStyle& styleToUse = style();
@@ -2518 +2523 @@
 }
 
+// FIXME: Can/Should we move this inside specific layout classes (flex. grid)? Can we refactor columnFlexItemHasStretchAlignment logic?
+bool RenderBox::hasStretchedLogicalWidth() const
+{
+    auto& style = this->style();
+    if (!style.logicalWidth().isAuto() || style.marginStart().isAuto() || style.marginEnd().isAuto())
+        return false;
+    RenderBlock* containingBlock = this->containingBlock();
+    if (!containingBlock) {
+        // We are evaluating align-self/justify-self, which default to 'normal' for the root element.
+        // The 'normal' value behaves like 'start' except for Flexbox Items, which obviously should have a container.
+        return false;
+    }
+    if (containingBlock->isHorizontalWritingMode() != isHorizontalWritingMode())
+        return RenderStyle::resolveAlignment(containingBlock->style(), style, ItemPositionStretch) == ItemPositionStretch;
+    return RenderStyle::resolveJustification(containingBlock->style(), style, ItemPositionStretch) == ItemPositionStretch;
+}
+
 bool RenderBox::sizesLogicalWidthToFitContent(SizeType widthType) const
 {
@@ -2530 +2552 @@
 
 #if ENABLE(CSS_GRID_LAYOUT)
-    if (parent()->isRenderGrid()) {
-        bool allowedToStretchChildAlongRowAxis = style().logicalWidth().isAuto() && !style().marginStartUsing(&parent()->style()).isAuto() && !style().marginEndUsing(&parent()->style()).isAuto();
-        return !allowedToStretchChildAlongRowAxis || RenderStyle::resolveJustification(parent()->style(), style(), ItemPositionStretch) != ItemPositionStretch;
-    }
+    if (isGridItem())
+        return !hasStretchedLogicalWidth();
 #endif
 

trunk/Source/WebCore/rendering/RenderBox.h

@@ -424 +424 @@
     bool sizesLogicalWidthToFitContent(SizeType) const;
 
+    bool hasStretchedLogicalWidth() const;
+
     LayoutUnit shrinkLogicalWidthToAvoidFloats(LayoutUnit childMarginStart, LayoutUnit childMarginEnd, const RenderBlock* cb, RenderRegion*) const;