Review css/css-sizing/aspect-ratio/flex-aspect-ratio-004.html #139
Comments
nt1m
added
the
test-change-proposal
|
This is the actual screen in all browsers:
|
|
This is subject to the ongoing discussion in w3c/csswg-drafts#6794 |
|
The CSSWG issue seems to have a resolution and is closed, do the tests need to be updated? |
|
It has a resolution for an ancillary point. The underlying issue is not resolved. See the very bottom of the IRC log: |
|
We talked about this briefly in #140. @davidsgrogan, to ensure the CSSWG are aware of this specific issue, can you describe it in w3c/csswg-drafts#6794, or file a new one if it's not quite the right issue? We'll probably keep this issue open until the spec discussion has happened, but if it takes a very long time we'll probably drop the test from Interop 2021. |
|
I looked at this test some more. Both of the competing spec proposals in w3c/csswg-drafts#6794 dictate this test's rendering to match all the current implementations. So I will just change the test to match all current implementations. (Yay!) But both proposals also make flex-aspect-ratio-002.html's rendering match Firefox's rendering, not Blink and WebKit's. Currently Blink and WebKit pass flex-aspect-ratio-002.html and Firefox fails it. So when I change -004 to match the behavior dictated by both proposals, then following the exact same principle, I'll change -002 to match the behavior dictated by both proposals. This will result in no net score difference for Blink and WebKit and a minor score increase for Firefox. -002 and -004 test the same idea, just that -002 is for row flexboxes and -004 is for column flexboxes. Firefox's behavior in -002 is what the spec authors originally intended. There are no champions for the Blink/WebKit behavior among the commenters in w3c/csswg-drafts#6794. As for mentioning this case in the WG issue, the CSSWG is already well aware of it. There are just competing proposals how to best fix it, with debate around the effects each proposal would have on cases other than -002 and -004. |
New expectations incorporate an item's min-intrinsic size as part of flex's automatic minimum sizing. The new expectations are arguably what the spec currently calls for AND what the spec authors originally intended AND are consistent with both relevant proposals in w3c/csswg-drafts#6794 Also discussed at web-platform-tests/interop#139 (comment) Change-Id: I3c6ecdf66fbb4ee745c88b6b2a4dbecfb9913908
New expectations incorporate an item's min-intrinsic size as part of flex's automatic minimum sizing. The new expectations are arguably what the spec currently calls for AND what the spec authors originally intended AND are consistent with both relevant proposals in w3c/csswg-drafts#6794 Also discussed at web-platform-tests/interop#139 (comment) Change-Id: I3c6ecdf66fbb4ee745c88b6b2a4dbecfb9913908 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3924134 Commit-Queue: David Grogan <dgrogan@chromium.org> Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Cr-Commit-Position: refs/heads/main@{#1052123}
New expectations incorporate an item's min-intrinsic size as part of flex's automatic minimum sizing. The new expectations are arguably what the spec currently calls for AND what the spec authors originally intended AND are consistent with both relevant proposals in w3c/csswg-drafts#6794 Also discussed at web-platform-tests/interop#139 (comment) Change-Id: I3c6ecdf66fbb4ee745c88b6b2a4dbecfb9913908 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3924134 Commit-Queue: David Grogan <dgrogan@chromium.org> Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Cr-Commit-Position: refs/heads/main@{#1052123}
New expectations incorporate an item's min-intrinsic size as part of flex's automatic minimum sizing. The new expectations are arguably what the spec currently calls for AND what the spec authors originally intended AND are consistent with both relevant proposals in w3c/csswg-drafts#6794 Also discussed at web-platform-tests/interop#139 (comment) Change-Id: I3c6ecdf66fbb4ee745c88b6b2a4dbecfb9913908 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3924134 Commit-Queue: David Grogan <dgrogan@chromium.org> Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Cr-Commit-Position: refs/heads/main@{#1052123}
|
Since the test now passes in all browsers I'll go ahead and close this. @nt1m please reopen if you think there's more we need to do here. |
|
And of course, big thanks @davidsgrogan for sorting out the tests! |
…pect-ratio tests, a=testonly Automatic update from web-platform-tests [css-flex] Change expectations of two aspect-ratio tests New expectations incorporate an item's min-intrinsic size as part of flex's automatic minimum sizing. The new expectations are arguably what the spec currently calls for AND what the spec authors originally intended AND are consistent with both relevant proposals in w3c/csswg-drafts#6794 Also discussed at web-platform-tests/interop#139 (comment) Change-Id: I3c6ecdf66fbb4ee745c88b6b2a4dbecfb9913908 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3924134 Commit-Queue: David Grogan <dgrogan@chromium.org> Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Cr-Commit-Position: refs/heads/main@{#1052123} -- wpt-commits: 8160da618bad50684232b7865e956827efe18072 wpt-pr: 36113
New expectations incorporate an item's min-intrinsic size as part of flex's automatic minimum sizing. The new expectations are arguably what the spec currently calls for AND what the spec authors originally intended AND are consistent with both relevant proposals in w3c/csswg-drafts#6794 Also discussed at web-platform-tests/interop#139 (comment) Change-Id: I3c6ecdf66fbb4ee745c88b6b2a4dbecfb9913908 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3924134 Commit-Queue: David Grogan <dgrogan@chromium.org> Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Cr-Commit-Position: refs/heads/main@{#1052123} NOKEYCHECK=True GitOrigin-RevId: cc4f488e5e9e8a496e5135e6d6fcea62eac77088
…pect-ratio tests, a=testonly Automatic update from web-platform-tests [css-flex] Change expectations of two aspect-ratio tests New expectations incorporate an item's min-intrinsic size as part of flex's automatic minimum sizing. The new expectations are arguably what the spec currently calls for AND what the spec authors originally intended AND are consistent with both relevant proposals in w3c/csswg-drafts#6794 Also discussed at web-platform-tests/interop#139 (comment) Change-Id: I3c6ecdf66fbb4ee745c88b6b2a4dbecfb9913908 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3924134 Commit-Queue: David Grogan <dgrogan@chromium.org> Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Cr-Commit-Position: refs/heads/main@{#1052123} -- wpt-commits: 8160da618bad50684232b7865e956827efe18072 wpt-pr: 36113
…lex items. https://bugs.webkit.org/show_bug.cgi?id=246755 rdar://101346126 Reviewed by Rob Buis. There has been a lot of conversation on computing the min-content size for flex items when computing the content size suggestion. In particular, there were potential issues when computing the min-content size for non-replaced flex items that were given an aspect-ratio property. This issue was brought up in the following CSSWG conversation: w3c/csswg-drafts#6794 When the conversation was initially created, browsers all had different behaviors. A patch was added to make WebKit behavior similar to the behavior of Blink: WebKit#3025 The patch added behavior to take the aspect ratio into consideration when computing these sizes. This behavior ended up being incorrect by the time consensus was reached. The final decision seems to be that we should be computing the min-intrinsic size, which does not take into consideration the aspect-ratio and is just based off of the content. The idea of the min-intrinsic size was introduced here: w3c/csswg-drafts#5305 Since our initial behavior was close to the behavior that was eventually agreed upon, this patch ends up being mostly a revert. It is not completely a revert, however, since there are still some pieces left in from the initial patch. If the item is a replaced element, we will start take into consideration the aspect-ratio and compute the size using computeMainSizeFromAspectRatioUsing. If the item is a non-replaced element, instead we will compute the size using computeMainAxisExtentForChild. This will compute the min-intrinsic size by calling either child.computeLogicalWidthInFragmentUsing or child.computeContentLogicalHeight. Spec reference: https://drafts.csswg.org/css-flexbox-1/#min-size-auto Follow up discussion: web-platform-tests/interop#139 * LayoutTests/TestExpectations: * LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-002.html: * LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-004.html: Newest version of the tests * Source/WebCore/rendering/RenderFlexibleBox.cpp: (WebCore::RenderFlexibleBox::computeFlexItemMinMaxSizes): Canonical link: https://commits.webkit.org/255858@main
Test List
Rationale
Not suggesting anything specific here, but mostly a review of that test, given it fails in the same identical way across the 3 engines.
cc @davidsgrogan @aethanyc who are the authors of this test.
The text was updated successfully, but these errors were encountered: