1689719 - Table as flex item has different width after bug 1689045

Status: RESOLVED FIXED

(Core :: Layout: Flexbox, defect)

Description

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

Attachment: table-as-flex-item-with-percentage-td.html

This is a reduced testcase of bug 1689045.

Comment 1

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

It looks like before bug 1689045, we ignore the flex-grow property on <table>, and the flex container grows its width anyway. I feel our current behavior is correct and the rendering matches Google Chrome's. Thunderbird (Bug 1689045) may depend on the wrong behavior of Firefox.

Daniel, what do you think?

Comment 2

[Daniel Holbert [:dholbert]]

(In reply to Ting-Yu Lin [:TYLin] (UTC-8) from comment #1)

It looks like before bug 1689045, we ignore the flex-grow property on <table>

Strictly speaking, that's probably not what was happening under the hood -- it's unlikely that we were actually ignoring flex-grow:0, since it defaults to 0 (so ignoring it would still leave you with 0).

I'll bet that really this had to do with how we were measuring the table's content-width (to resolve the default flex-basis:auto;width:auto into a width). We were probably doing this in such a way that was allowing the td's 100% width to resolve against the flex container's explicitly-specified width, or something like that.

In any case: I think the expectation here is that <table> and <table style="width:max-content> should render the same as flex items. It seems they didn't used to do so, but they do now (and that's brought us into compat with Chrome on the attached testcase). So I think you're right that this is a step towards correctness, and Thunderbird was just accidentally depending on a Gecko bug before.

So I don't think there are any code changes to make here. I've got an idea for a reftest locally that I'll post as a patch for review from you, though, to be sure we don't regress this.

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: Bug 1689719: Add a WPT to assert that a table with a content-based flex-basis should be sized as if it had a 'max-content' size in a block. r?TYLin

Chrome and Safari fail this WPT because they mishandle the max-content sizing
in the reference case. That's tracked in these bugs:
https://bugs.chromium.org/p/chromium/issues/detail?id=1172498
https://bugs.webkit.org/show_bug.cgi?id=221165

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/061da9f1f7d0
Add a WPT to assert that a table with a content-based flex-basis should be sized as if it had a 'max-content' size in a block. r=TYLin

Comment 5

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

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

Comment 6

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e505da53360d
Backed out changeset 061da9f1f7d0 for failures on browser_bug676619.js.

Comment 7

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

Upstream PR was closed without merging

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0acd315f0a70
Add a WPT to assert that a table with a content-based flex-basis should be sized as if it had a 'max-content' size in a block. r=TYLin

Comment 9

[Atila Butkovits]

The push was wrongfully backed out. It was relanded: https://hg.mozilla.org/integration/autoland/rev/0acd315f0a70

Comment 10

[Daniel Holbert [:dholbert]]

Thanks!

Comment 11

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/0acd315f0a70

Comment 12

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

Upstream PR merged by moz-wptsync-bot