Closed
Bug 972595
Opened 11 years ago
Closed 9 years ago
Flex containers should use "flex-basis" instead of "width" for computing intrinsic widths of flex items
Categories
(Core :: Layout, defect, P4)
Core
Layout
Tracking
()
RESOLVED
DUPLICATE
of bug 1055887
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
Attachments
(3 files, 1 obsolete file)
690 bytes,
text/html
|
Details | |
1.76 KB,
patch
|
Details | Diff | Splinter Review | |
594 bytes,
text/html
|
Details |
Per http://lists.w3.org/Archives/Public/www-style/2014Feb/0483.html , the flexbox spec section on "Intrinsic Sizes" [1] says that each flex item's contribution is its "hypothetical main size".
And the definition of "hypothetical main size"[2] is based on flex-basis, rather than width/height. (though "auto" may fall back to the width/height value)
Currently, we use IntrinsicForContainer to compute each item's contribution, and that does use the "width" property. But we should change that to use flex-basis for horizontal flex items (with non-'auto' flex-basis).
[1] http://dev.w3.org/csswg/css-flexbox/#intrinsic-sizes
[2] http://dev.w3.org/csswg/css-flexbox/#hypothetical-main-size
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → dholbert
Priority: -- → P4
Assignee | ||
Comment 1•11 years ago
|
||
Assignee | ||
Comment 2•11 years ago
|
||
Here's a first-pass fix; this fixes the attached testcase. I'll bet we can more elegantly share code with the existing "width or flex-basis" chunks of nsLayoutUtils.cpp, though, so this stands to be improved a bit.
Assignee | ||
Comment 3•11 years ago
|
||
Note: Chrome/Blink matches our (broken) current rendering, incorrectly relying on 'width'.
Opera 12.16. (from local testing) and IE (according to the www-style post) rely on 'flex-basis' here and hence do not match our current rendering.
Comment 4•10 years ago
|
||
If I am correct Daniel, would this change make this test work correctly in FF and match Blink/IE: http://jsfiddle.net/944tL115/4/
Also, we have seen Blink at times take flex-basis into account while others they do not.
Assignee | ||
Comment 5•10 years ago
|
||
I don't think that's related to this bug -- I think we're overflowing there because of "min-width:auto" imposing a large minimum width on the <input>. It's basically an instance of bug 1088586.
Blink matches us on that testcase if I add this CSS rule:
input { min-width: -webkit-min-content }
which makes it "fake" min-width:auto support.
Comment 6•10 years ago
|
||
We have implemented all of the aspects of the latest spec and due to the width of the input the flex container should most definitely include that width of the input (whether the input is large or not) to calculate the hypothetical main-size of the items. I don't want to hijack this thread for this bug so if you could follow up with me in email.
Assignee | ||
Comment 7•10 years ago
|
||
Ah, I see what you mean. tl;dr: yes, that testcase would be fixed (in that we'd grow the containers to be large enough for the contents) if we fixed this bug.
I was confused because I only had Blink to compare to locally (since I run Linux), and Blink does *not* grow the containers -- instead, it shrinks the <input> to accomodate the incorrectly-too-small container. (as I noted in comment 5) So it sort of looks right, but it's not (and I imagine it differs from IE, if IE does this right).
I'm attaching a testcase that's based on your fiddle, but with "flex:none" on the <input>, to prevent Blink from shrinking it & looking correct. This testcase compares "flex:0 0 80px" to "flex:none;width:80px", which *should* be equivalent, but aren't currently in Firefox or Chrome.
Can you confirm that the two examples in this testcase look the same (and don't overflow) in IE?
Flags: needinfo?(gwhit)
Comment 8•10 years ago
|
||
Yes, IE12 renders per spec in this test case while Chrome/FF allow the input to overflow. Glad you are aware of it. Thanks.
Flags: needinfo?(gwhit)
I would like to add one more test case.
From it you can see, that 0px flex-basis is ignored, if flex item has oversized content, unless overflow:auto is specified (uncomment corresponding css on .flex2 to see correction). Blink cuts the overflowed content as expected. Presto is bugged.
Spec says (http://www.w3.org/TR/css-flexbox-1/#hypothetical-main-size), that hypothetic main size is flex-basis if it is defined.
Assignee | ||
Comment 10•9 years ago
|
||
@ettavolt: Thanks for the testcase. It's actually unrelated to this bug, though, and Firefox is doing the right thing there. What you're running into is the flexbox spec's "implied minimum size of flex items", described here: http://dev.w3.org/csswg/css-flexbox/#min-size-auto
"overflow:auto" does indeed disable that feature (as required by the spec); but a simpler way to disable it (if you want to disable it) is to simply set "min-height:0" or "min-width:0".
(Microsoft Edge matches Firefox's rendering of your testcase, too, which increases my confidence that we're doing the right thing & Blink's the odd one out. :))
Assignee | ||
Updated•9 years ago
|
Attachment #8626503 -
Attachment is obsolete: true
Comment 11•9 years ago
|
||
@dholbert
I opened a bug against Blink for this: https://code.google.com/p/chromium/issues/detail?id=504931&thanks=504931&ts=1435338782
Feel free to chime in if necessary, I'll ping Christian offline to ensure that he knows about this interop difference.
Assignee | ||
Comment 12•9 years ago
|
||
Thanks, Greg!
Comment 13•9 years ago
|
||
Aha, seems I've missed the statement "hypothetical main size is the item’s flex base size _clamped_ according to its _min_ and _max_ main size properties.".
So the new meaning for min and max sizes is the most confusing thing in this case.
Assignee | ||
Comment 14•9 years ago
|
||
Resolving as INVALID, since it seems there's no Firefox bug here. Thanks for filing, in any case; hopefully we'll get some investigation/interop on the Chrome/blink side here, via Greg's bug.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → INVALID
Assignee | ||
Comment 15•9 years ago
|
||
Sorry, my bad -- I resolved this as INVALID with only comment 9 through 13 in mind -- but those comments are about something different from what was originally being discussed here.
The discussion before comment 9 still represents a firefox bug. Though it's ultimately about the "intrinsic sizes" section of the flexbox spec, which has been rewritten since this bug was filed.
So: I'm duping this forward to bug bug 1055887, which is on implementing the newer language there.
Resolution: INVALID → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•