Bug 136041 – Safari (WebKit) doesn't wrap element within flex when width comes below min-width

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Kenan Sulayman]

Created attachment 236765 [details]
Screenshot: Safari, Chrome, Opera, Firefox.

Safari doesn't wrap an element when flex is applied (1 0 25%) and the width comes below the set min-width. This is different in Firefox, Chrome (Blink) and Opera (Blink). That is, I assume it was already fixed in that fork and not incorrectly implemented in Gecko at the first place.

Changing

flex: 1 0 25%;

to

flex: 1 0 40%;

Seems to be a bit of a workaround, doesn't fix it, though.

In the attachment (Screenshot), from left to right: Safari, Chrome, Opera, Firefox.

Since I am unable to attach two attachments, see this link for the example used in the screenshot: http://data.sly.mn/text/163h0C2x2f3O/raw.

[Kenan Sulayman]

Created attachment 236805 [details]
Example

[Mikol Graves]

See also:

* http://codepen.io/anon/pen/qEKwXX (Does not work as expected.)
* http://codepen.io/anon/pen/yyqLGr (Works around actual behavior.)
* https://github.com/angular/material/issues/1720 (Resulting Angular Material issue.)

Steps to Reproduce:
1. Create a container with CSS declarations `-webkit-flex-direction: row; -webkit-flex-wrap: wrap;` and three child elements.
2. Each child should include the CSS declarations `-webkit-flex: 1; min-width: 320px;`.
3. View the page with Safari on OS X (10.9.5) or iOS (I used 8.x on both a real and simulated iPhone 6).


Expected Results:
Each child element should shrink to its minimum width as the viewport gets narrower and wrap as the viewport becomes narrower than their combined minimum width.

Setting `flex: 1 1 320px` achieves the expected results.

Actual Results:
The child elements shrink to their mnimum width when the viewport width is narrow, but they do not wrap.

User Agent: 
Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_5) AppleWebKit/600.3.18 (KHTML, like Gecko) Version/7.1.3 Safari/537.85.12

[Philip Walton]

I've added this bug to the Flexbugs list I maintain:
https://github.com/philipwalton/flexbugs#11-min-and-max-size-declarations-are-ignored-when-wrapping-flex-items

I'll copy the bug and description here to make it easier to reference:

## Min and max size declarations are ignored in Safari when wrapping flex items

Bug: http://codepen.io/anon/pen/BNrGwN
Workaround: http://codepen.io/anon/pen/RPMqjz

Safari uses min/max width/height declarations for actually rendering the size of flex items, but it ignores those values when calculating how many items should be on a single line of a multi-line flex container. Instead, it simply uses the item's `flex-basis` value, or its width if the flex basis is set to `auto`.

This is problematic when using the `flex: 1` shorthand because that sets the flex basis to `0%`, and an infinite number of flex items could fit on a single line if the browser thinks their widths are all zero. The bug above (http://codepen.io/philipwalton/pen/BNrGwN) show an example of this happening.

This is also problematic when creating fluid layouts where you want your flex items to be no bigger than X but no smaller than Y. Since Safari ignores those values when determining how many items fit on a line, that strategy won't work.

[Radar WebKit Bug Importer]

<rdar://problem/25569370>

[Jon Lee]

*** Bug 145402 has been marked as a duplicate of this bug. ***

This is flex basic behaviour, works as expected in all browsers except Safari. It's unbeliable that since 2014, no one in the Safari dev team has even bothered to look at this.

Can we get some sorf of update on this please?

Thanks.

[Martin Kadlec]

This is really giving us trouble right now and basically preventing us from using flexbox effectively on production. Any chance for a fix?

[Maël Lavault]

It is a shame that this is not fixed, it's taking us so much time working around this. flex-wrap is something we should expect to work in every recent browsers.

+1 for this still being a problem; I can't get any workaround to work.

[Daniel]

This is a problem for me too.

"Safari uses min/max width/height declarations for actually rendering the size of flex items, but it ignores those values when calculating how many items should be on a single line of a multi-line flex container. Instead, it simply uses the item's `flex-basis` value, or its width if the flex basis is set to `auto`."

The resulting behavior in my case is that the flex items overflow their container. Safari *should* use the min- and max-width values when calculating how many items should be on a single line of a multi-line flex container.

[Phy]

This is causing problems for UI devs all over; please prioritize it.  It's a shame to have Safari be the exception for web design architecture.

[Luke Jackson]

+1 on this issue, have had to use a workaround which is far from ideal

[zalan]

Created attachment 295540 [details]
Patch

[zalan]

Created attachment 295551 [details]
Patch

[Daniel Bates]

Comment on attachment 295551 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=295551&action=review

I did not review this patch for correctness. I noticed a minor style nit.

> Source/WebCore/rendering/RenderFlexibleBox.cpp:996
> +        LayoutUnit childMainAxisExtentWithMinWidthConstrain = childMainAxisExtent;

Should this local be called childMainAxisExtentWithMinWidthConstraint? (Notice the 't' at the end of the name).

[zalan]

(In reply to comment #15)
> Comment on attachment 295551 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=295551&action=review
> 
> I did not review this patch for correctness. I noticed a minor style nit.
> 
> > Source/WebCore/rendering/RenderFlexibleBox.cpp:996
> > +        LayoutUnit childMainAxisExtentWithMinWidthConstrain = childMainAxisExtent;
> 
> Should this local be called childMainAxisExtentWithMinWidthConstraint?
> (Notice the 't' at the end of the name).
:( It should. Thanks!

[zalan]

Created attachment 295573 [details]
Patch

[Darin Adler]

Comment on attachment 295551 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=295551&action=review

> Source/WebCore/rendering/RenderFlexibleBox.cpp:999
> +            auto minWidthForChild = computeMainAxisExtentForChild(*child, MinSize, child->style().logicalMinWidth());
> +            if (minWidthForChild)

Could define this variable inside the if:

    if (auto minWidthForChild = ...)

> Source/WebCore/rendering/RenderFlexibleBox.cpp:1000
> +                childMainAxisExtentWithMinWidthConstrain = std::max(childMainAxisExtent, minWidthForChild.value());

Seems like there ought to be a function like std::max that ignores optional values; then we would not need an if statement.

[Darin Adler]

Comment on attachment 295573 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=295573&action=review

See my comments from the previous version of this patch.

> Source/WebCore/rendering/RenderFlexibleBox.cpp:1002
> +        preferredMainAxisExtentWithMinWidthConstraint += (childMainAxisExtentWithMinWidthConstraint + borderMarginAndPaddingSpace);

Not sure the parentheses here are helpful.

> Source/WebCore/rendering/RenderFlexibleBox.cpp:1009
> +        preferredMainAxisExtent += (childMainAxisExtent + borderMarginAndPaddingSpace);

Not sure the parentheses here are helpful.

> Source/WebCore/rendering/RenderFlexibleBox.cpp:1014
> +        minMaxAppliedMainAxisExtent += (childMinMaxAppliedMainAxisExtent + borderMarginAndPaddingSpace);

Not sure the parentheses here are helpful.

[zalan]

(In reply to comment #18)
> Comment on attachment 295551 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=295551&action=review
> 
> > Source/WebCore/rendering/RenderFlexibleBox.cpp:999
> > +            auto minWidthForChild = computeMainAxisExtentForChild(*child, MinSize, child->style().logicalMinWidth());
> > +            if (minWidthForChild)
> 
> Could define this variable inside the if:
> 
>     if (auto minWidthForChild = ...)
> 
> > Source/WebCore/rendering/RenderFlexibleBox.cpp:1000
> > +                childMainAxisExtentWithMinWidthConstrain = std::max(childMainAxisExtent, minWidthForChild.value());
> 
> Seems like there ought to be a function like std::max that ignores optional
> values; then we would not need an if statement.
I've seen people doing this instead -> std::max(a, b.value_or(a)) but I find it even worse.

[zalan]

Created attachment 295574 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 295574 [details]
Patch

Clearing flags on attachment: 295574

Committed r209068: <http://trac.webkit.org/changeset/209068>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.