Implement aspect-ratio for flex
Categories
(Core :: Layout: Flexbox, enhancement, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox83 | --- | fixed |
People
(Reporter: boris, Assigned: boris)
References
(Blocks 2 open bugs)
Details
(Keywords: dev-doc-complete)
Attachments
(8 files, 3 obsolete files)
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
740 bytes,
text/html
|
Details | |
47 bytes,
text/x-phabricator-request
|
Details | Review |
Implement aspect-ratio on display: flex.
At least we have to pass the wpt: testing/web-platform/tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-00x.tentative.html
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 1•4 years ago
•
|
||
It seems we have to focus on
- flex basis size algorithm.
- transferred size suggestion. Looks like we don't handle this well for non-replaced-element flex items.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 2•4 years ago
|
||
One of the current tentative wpts depends on Bug 1056959. I'd like to keep it expected failure for now.
Assignee | ||
Comment 3•4 years ago
|
||
This patch focus on applying aspect-ratio property to flex item (basic blocks).
For replaced elements, we should rely on its IntrinsicRatio() function.
Assignee | ||
Comment 4•4 years ago
|
||
This depends on github.com/w3c/csswg-drafts/issues/5406 acutally.
I tried this tentative way to take aspect-ratio into account for
handling min main size auto case on the flex container.
(https://drafts.csswg.org/css-sizing-4/#aspect-ratio-minimum)
Basically, we treat the main size as auto if it is ratio-dependent axis.
So we can put as many as flex items in this line. And then we compare the
largest line length of this to the main size calculated by aspect-ratio, and
use the larger one as the input for computing the final main size.
Assignee | ||
Comment 5•4 years ago
|
||
This tweaks the retrieval of aspect-ratio for Automatic Minimum Size of
Flex Items.
Assignee | ||
Comment 6•4 years ago
|
||
Assignee | ||
Updated•4 years ago
|
Updated•4 years ago
|
Assignee | ||
Comment 7•4 years ago
•
|
||
I notice there are some new wpts (Bug 1660056) added yesterday we don't pass based on my current patches. I'm looking at them. May add new extra patches or file a different bug, depends on where the root cause happens.
Assignee | ||
Comment 8•4 years ago
|
||
For the cross size determination of each item we
determine the hypothetical cross size of each item by performing layout as if it were an in-flow block-level box with the used main size and the given available space, treating auto as fit-content.
So perhaps this is the part I didn't address in the patch series. If we need to do this as in-flow block-level box, then it makes sense to use the used main size and the aspect-ratio
to get the hypothetical cross size for the flex items.
Assignee | ||
Comment 9•4 years ago
|
||
(In reply to Boris Chiou [:boris] from comment #8)
For the cross size determination of each item we
determine the hypothetical cross size of each item by performing layout as if it were an in-flow block-level box with the used main size and the given available space, treating auto as fit-content.
So perhaps this is the part I didn't address in the patch series. If we need to do this as in-flow block-level box, then it makes sense to use the used main size and the
aspect-ratio
to get the hypothetical cross size for the flex items.
OK. So it seems the reasonable way to fix this is to do the similar things as the imposed main size for both replaced elements and non-replaced elements (i.e. also in nsIFrame::ComputeSize
) if we have the non-auto aspect-ratio for this flex item and have set the frame property, nsIFrame::FlexItemMainSizeOverride()
.
Updated•4 years ago
|
Updated•4 years ago
|
Assignee | ||
Comment 11•4 years ago
|
||
Assignee | ||
Comment 12•4 years ago
|
||
Address aspect-ratio cases for non-replaced elements, for
https://drafts.csswg.org/css-flexbox-1/#cross-sizing.
Assignee | ||
Comment 13•4 years ago
|
||
Updated•4 years ago
|
Assignee | ||
Comment 14•4 years ago
|
||
Updated•4 years ago
|
Comment 15•4 years ago
|
||
Per the comment in the patch: if we're given a bogus range, we can technically
end up with a single PrintedSheetFrame that contains no displayable
pages. Luckily, that's a situation that the frontend should detect & recover
from quickly & gracefully, so it's not really a problem. So: in that scenario,
we'll now spam a non-fatal warning; and we'll fatally assert if any sheet
beyond the first page has zero displayable pages. (That still shouldn't be
possible.)
Comment 16•4 years ago
|
||
Comment on attachment 9174560 [details]
Bug 1646096: Add back assertion to ensure we don't create printed sheets with no pages on them (softened for the first sheet). r?hiro
Revision D89537 was moved to bug 1662940. Setting attachment 9174560 [details] to obsolete.
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Comment 17•4 years ago
•
|
||
Here's a testcase to illustrate why we can't just leave mainAxisCoord
untouched as noted in my inline comment in phabricator.
This is a variant of the existing web-platform test https://searchfox.org/mozilla-central/source/testing/web-platform/tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-001.tentative.html -- I've moved the test's styles to a <style>
block, and added my own tweaks in inline styles.
With your current first patch in the patch stack here, the third chunk in this testcase renders as a skinny green bar, rather than a green square. But really, all three chunks should render the same. (Chrome "agrees" with us right now simply because they don't support the explicit content
keyword for flex-basis
, so they're not a trustworthy source on the rendering of this third example)
Updated•4 years ago
|
Assignee | ||
Comment 18•4 years ago
|
||
So we have the better test coverage for apsect-ratio on flex layout.
Those wpts should be passed already because we handle aspect-ratio
property well in GetIntrinisicRatio() for all the replaced elements.
Assignee | ||
Comment 19•4 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=48bb933db3a99811d134bba8d590459c196e2f82
Comment 20•4 years ago
|
||
Pushed by bchiou@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5ae44731baab Add tests for replaced-element flex items with aspect-ratio. r=dholbert https://hg.mozilla.org/integration/autoland/rev/6683051983ae Support aspect-ratio for flex items when computing their sizes for ReflowInput. r=dholbert https://hg.mozilla.org/integration/autoland/rev/bbc133587c17 Handle aspect-ratio for Automatic Minimum Size of Flex items. r=dholbert https://hg.mozilla.org/integration/autoland/rev/1f41a35021e9 Mark bug number for flex-aspect-ratio-009.tentative.html. r=dholbert https://hg.mozilla.org/integration/autoland/rev/1f474090b6e8 Take aspect-ratio into account in FlexItem::mIntrinsicRatio. r=dholbert,TYLin https://hg.mozilla.org/integration/autoland/rev/e3eb1e60ee1f Fix the calculation of the flex item's cross size with respect to aspect-ratio. r=dholbert https://hg.mozilla.org/integration/autoland/rev/fd47099e9a65 Mark bug number for the incorrect calculation of the cross size for column-oriented flex container. r=dholbert
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/25664 for changes under testing/web-platform/tests
Upstream PR merged by moz-wptsync-bot
Comment 23•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5ae44731baab
https://hg.mozilla.org/mozilla-central/rev/6683051983ae
https://hg.mozilla.org/mozilla-central/rev/bbc133587c17
https://hg.mozilla.org/mozilla-central/rev/1f41a35021e9
https://hg.mozilla.org/mozilla-central/rev/1f474090b6e8
https://hg.mozilla.org/mozilla-central/rev/e3eb1e60ee1f
https://hg.mozilla.org/mozilla-central/rev/fd47099e9a65
Comment 24•4 years ago
|
||
I've updated BCD for this partial implementation of the feature.
Description
•