Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compat spec section on "-webkit-box" isn't actually interoperable #87

Open
dholbert opened this issue Nov 1, 2017 · 4 comments
Open

Comments

@dholbert
Copy link

dholbert commented Nov 1, 2017

The compat spec documents the way I initially implemented Mozilla's -webkit-box emulation in https://bugzilla.mozilla.org/show_bug.cgi?id=1208635 -- but that turned out to be problematic, so our impl has changed and the compat-spec text probably needs to change too.

Specifically: our impl changed in https://bugzilla.mozilla.org/show_bug.cgi?id=1257688 (see comment 0 there for the concerns that motivated the change). Unfortunately (from a speccability perspective), the resulting impl does not simply alias standard CSS properties/values (and that's intentional, due to the concerns expressed in that bug -- we don't want a page that has a crufty * { -webkit-box-pack: center } CSS rule to actually affect the modern/standard equivalent justify-content property on every single element in the page, particularly not once we've implemented justify-content for block layout. And similar for the other -webkit-box-* properties.)

Additionally, we have a few behavioral compatibility quirks that we only apply for -webkit-box and not for modern flexbox, for handling of abspos children and inline children. See https://hg.mozilla.org/mozilla-central/rev/5dcb4fe44668 and https://hg.mozilla.org/integration/autoland/rev/f42dc6b82a4d for those changes.

@gsnedders
Copy link
Member

CC @bfgeek, given IIRC Blink no longer has LayoutDeprecatedFlexibleBox.

@bfgeek
Copy link
Member

bfgeek commented Jun 7, 2022

Right - we map the majority of "display: -webkit-box" over to "display: flex". There are a few small adjustments:

  • min-width/min-height: auto resolves to zero (instead of the auto min-size).
  • an auto flex basis resolves to fit-content
  • (there are some other things but I think they were for some chrome specific use cases).

We also don't use display:flex for -webkit-line-clamp , instead this maps over to block layout (which will be more interoperable for the max-lines property when we can support it).

@andreubotella
Copy link
Member

andreubotella commented Jul 25, 2022

There's also the fact that this section uses the terms "keyword mapping" and "property mapping", which aren't defined anywhere. While keyword mappings would seem simple enough (don't map them at parse-time as for legacy value aliases but still treat them as the mapped property for everything else), it's far from clear how property mappings interact with their mapped properties when both end up present in the same CSS box after the cascade. In fact, as far as I can tell, these properties seem to be used instead of the mapped properties if display's computed value is -webkit-box (but not -webkit-flex), and they're ignored otherwise.

Still, keyword mappings don't behave exactly like their mapped keywords. Blink doesn't yet seem to support multiple keywords in the display property, but in both Gecko and Webkit, display: inline -webkit-box is not valid, even though display: inline flex is. So it would seem better to just augment the definition of display to include these keywords under <display-legacy>.

There's also the fact that the CSS overflow spec defines the legacy -webkit-line-clamp, switching some of its behavior based on whether the computed value of display is -webkit-box or -webkit-inline-box, and on whether the computed value of -webkit-box-orient is vertical. So these behave like the mapped keyword/property, unless some CSS spec adds further requirements.

@gsnedders
Copy link
Member

see also: w3c/csswg-drafts#1946

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants