1638937 - Firefox resolves the default "flex-basis" to 100% on svg elements that have viewBox (and no intrinsic size attributes)

Status: RESOLVED FIXED

(Core :: Layout: Flexbox, defect, P3)

Description

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

I'm not entirely sure who's correct here, but there's an interop issue at least, so I'm filing this bug to cover it.

STR:

1. Load attached testcase.
2. Compare our rendering to Chrome.

ACTUAL RESULTS:
The last piece ("Just viewBox") differs. We render the element as if it had flex-basis:100%. Chrome renders it as if it had flex-basis:0.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: testcase 2: using a block wrapper instead of a flex wrapper

Here's a second testcase with display:block instead of display:flex on the container.

This doesn't change our behavior* as compared to testcase 1, whereas it does change Chrome's behavior -- they change to match us.

• (aside from a minor line-box-sizing-based difference on a short element, where the block version is a bit taller [interoperably] than the flex version)

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: testcase 3: using a block wrapper *and also* "width: max-content" on the SVG elements

If I explicitly add width:max-content to the SVG elements (which perhaps is a better analogy for what the default content-based flex-basis is supposed to do), then both Firefox and Chrome change their behavior (but neither of them quite end up matching the flexbox behavior from testcase 1, so it's not in fact an exact analogy).

• Firefox triggers a size-to-availableWidth behavior which seems somewhat broken (the SVG ends up as wide as your window is when you reload) - there might be a bug on that already?

• Chrome uses the CSS fallback replaced-element intrinsic width of 300px.

Comment 3

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #0)

Created attachment 9149930 [details]
testcase 1
[...]
ACTUAL RESULTS:
The last piece ("Just viewBox") differs. We render the element as if it had flex-basis:100%. Chrome renders it as if it had flex-basis:0.

Update: Firefox and Chrome have both changed how they handle this^ testcase (testcase 1). We changed on the first part (diverging from Chrome), and Chrome changed on the last part (to agree with us).

I'm testing two old browser versions from roughly when this bug was filed (Firefox 78 & Chrome 86), to compare modern dev versions when describing changes in my notes below.

FIRST SECTION (labeled "no intrinsic attributes"):
"Old" Firefox 78 & Chrome 86, and "current" Chrome 106dev all render the first section with the teal SVG being half the width of its container.
"Current" Firefox 105 Nightly renders that first section with the teal SVG being the full border-box width of its container (which makes it barely overflow beyond the parent's right border a bit.

LAST SECTION (labeled "Just viewBox"):
"Old" Chrome 86 rendered this one with the teal SVG being 0x0.
"Old" Firefox 78 and "Current" Chrome 106dev and Firefox 105 Nightly render that last section with the teal area being 600px wide (filling the flex container's width) by 150px tall.

So there's still an interop issue here, but it's just with a different part of the testcase. :)

Also:

• Chrome also renders testcases 2 and 3 the same way as they render testcase 1 (modulo the line-height thing I mentioned in comment 1)
• Firefox hasn't changed it's rendering on testcase 2 (we match Chrome there) or testcase 3 (we have a weird size-to-fit-the-window behavior there which is likely broken, mentioned in comment 2).

Comment 4

[Daniel Holbert [:dholbert]]

So, put another way:

• part of the interop issue here (the difference I called out in comment 0) has been mitigated due to a change in Chrome.
• but there's a new interop issue with that original testcase due to a Firefox change (probably need to bisect to find out why & confirm whether that's unwanted)
• and testcase 3 still shows huge/broken behavior, though that's mostly the issue that's covered in bug 1651754 I think.

Comment 5

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

I bisect our behavior change for FIRST SECTION (labeled "no intrinsic attributes") in testcase 1. Mozregression finds this range, and bug 1686603 is likely the suspect behind it.

Comment 6

[Daniel Holbert [:dholbert]]

Thanks!

I think I'll spin off a new bug for that behavior-change (I think it's a regression), and I'll turn the testcases here into checkLayout-based WPTs that we can land with some known-failure annotations. (The known failure in the first testcase will be covered by the bug that I'm about to spin off, and the known failures in testcase 3 will be covered by bug 1651754.)

And then we can close this bug as WORKSFORME since the original thing it was tracking was addressed elsewhere, and the remaining bits are covered by other more-specific bugs.

Comment 7

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #6)

I think I'll spin off a new bug for that behavior-change (I think it's a regression)

Filed bug 1786610.

Comment 8

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

I think I'll spin off a new bug for that behavior-change (I think it's a regression), and I'll turn the testcases here into checkLayout-based WPTs that we can land with some known-failure annotations. (The known failure in the first testcase will be covered by the bug that I'm about to spin off, and the known failures in testcase 3 will be covered by bug 1651754.)

I've written a patch to convert all the testcases to wpt. Will post it shortly.

Comment 9

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1638937 - Add wpt tests for svg intrinsic sizes. r?dholbert

The three wpt tests are adapted from dholbert's testcases in the bug, and
written in checkLayout flavor. They should have the same rendering.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ab0e9b9eb8e8
Add wpt tests for svg intrinsic sizes. r=dholbert

Comment 11

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/ab0e9b9eb8e8

Comment 12

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/35733 for changes under testing/web-platform/tests

Comment 13

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot