1783934 - Add a way to incrementally migrate -moz-box layout to modern flexbox

Status: RESOLVED FIXED

(Core :: Layout, task)

Description

[Emilio Cobos Álvarez (:emilio)]

We have layout.css.emulate-moz-box-with-flex, which is ultimately a one-off switch to replace all of legacy box layout with flexbox-based layout.

Recently, I've come across various broken XUL / block interactions that wouldn't be broken if they were Flex / block interactions. See the "see also" field for some examples that I believe this would fix.

These bugs are really annoying to workaround, but we're nowhere close to ship layout.css.emulate-moz-box-with-flex all at once. However, I think we could take incremental steps towards that.

So, what do y'all think about an inherited -moz-box-layout: legacy | flex property, that allows us to toggle legacy / flex based layout on a per-subtree basis?

The idea would be to start using it where possible, and eventually probably switch the default value to "flex". That hopefully provides a path towards removing legacy XUL incrementally.

cc'ing / ni'ing some potentially interested parties for feedback, but I'll try to take a look at this next week unless someone thinks it's a bad idea for some reason.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1783934 - Add a chrome-only -moz-box-layout: legacy | flex, and use that to implement flexbox emulation. r=dholbert

Depends on D154152

Comment 2

[:Gijs (he/him)]

Do we have a reasonable way of querying for nodes that use either model once we introduce this? I'm a bit worried about that given it's CSS based and the markup will be elsewhere. Without an automated querying/replacement system I'm nervous that this step:

The idea would be to start using it where possible, and eventually probably switch the default value to "flex".

Is going to not be very feasible until we get very close to finishing this. Or am I missing something obvious here?

Comment 3

[Emilio Cobos Álvarez (:emilio)]

(In reply to :Gijs (he/him) from comment #2)

Do we have a reasonable way of querying for nodes that use either model once we introduce this? I'm a bit worried about that given it's CSS based and the markup will be elsewhere.

Can you elaborate? It should show up in devtools like other properties. We could expose this in getComputedStyle or via WebIDL to chrome code or so if needed, though it's a bit unclear in why would it be needed, because ideally the models are not that different. In practice the differences I'm aware of are:

• The new "flex" model interacts better with other HTML layout modes (like block / etc).
• The old model looks not only at CSS but also XUL "flex" attributes.

Other than those, for the most part they should behave equivalently.

Without an automated querying/replacement system I'm nervous that this step:

The idea would be to start using it where possible, and eventually probably switch the default value to "flex".

Is going to not be very feasible until we get very close to finishing this. Or am I missing something obvious here?

I think that's right, but I guess I'm not sure how that's necessarily related to the querying?

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/449fe2d946ea
Add a chrome-only -moz-box-layout: legacy | flex, and use that to implement flexbox emulation. r=dholbert

Comment 5

[Marian-Vasile Laza]

Backed out changeset 449fe2d946ea (Bug 1783934) for causing mochitest failures on test_animation-type-longhand.html.
Backout link
Push with failures <--> c1
Failure Log
Also 5 Failure Log

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Grr, I swear I thought I had uploaded the version with the fix for that test, but apparently I had not...

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/23c5d9491a56
Add a chrome-only -moz-box-layout: legacy | flex, and use that to implement flexbox emulation. r=dholbert

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b9d5a4b375f6
Fix newly introduced test to actually flex the item.

Comment 9

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/23c5d9491a56
https://hg.mozilla.org/mozilla-central/rev/b9d5a4b375f6

Comment 10

[:Gijs (he/him)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)

(In reply to :Gijs (he/him) from comment #2)

Do we have a reasonable way of querying for nodes that use either model once we introduce this? I'm a bit worried about that given it's CSS based and the markup will be elsewhere.

Can you elaborate? It should show up in devtools like other properties. We could expose this in getComputedStyle or via WebIDL to chrome code or so if needed, though it's a bit unclear in why would it be needed, because ideally the models are not that different. In practice the differences I'm aware of are:

• The new "flex" model interacts better with other HTML layout modes (like block / etc).
• The old model looks not only at CSS but also XUL "flex" attributes.

Other than those, for the most part they should behave equivalently.

Without an automated querying/replacement system I'm nervous that this step:

The idea would be to start using it where possible, and eventually probably switch the default value to "flex".

Is going to not be very feasible until we get very close to finishing this. Or am I missing something obvious here?

I think that's right, but I guess I'm not sure how that's necessarily related to the querying?

Well, if there was a straightforward way of automatically finding (querying for) instances of using the non-default model in the source tree, that would make it easier to make a switch like this? We could automatically transform such nodes - e.g. if we had an opt-in attribute for the new model, we could remove those once we switch to that model by default, and/or add new opt-out attributes for things that need to keep using the old model for the moment. It would also help in terms of assessing where to focus QA effort when making these changes.

I don't think that manually auditing via devtools scales for the size of our project - even just browser/ has some 80 HTML markup files we'd need to open and then check with devtools...

Does that make more sense of my concern?

Comment 11

[Emilio Cobos Álvarez (:emilio)]

I see. Sure, when switching the behavior of something like that, what we've done in the past is adding assertions before landing (and ensuring they're not hit on automation by fixing the relevant styles). Bug 1580012 is an example of that. Doing such thing should be feasible here to make sure we're not changing behavior. Does that alleviate your concern?

Comment 12

[:Gijs (he/him)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #11)

I see. Sure, when switching the behavior of something like that, what we've done in the past is adding assertions before landing (and ensuring they're not hit on automation by fixing the relevant styles). Bug 1580012 is an example of that. Doing such thing should be feasible here to make sure we're not changing behavior. Does that alleviate your concern?

Yeah, that helps. I think we'll still miss things because some parts of Firefox are, let's say, better-tested than others... but automation should be enough for the 95% cases.

Comment 13

[Dão Gottwald [:dao]]

I'm late to the party, but yeah this was / is a good idea. :)