-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add missing css.properties.display.flow feature #24386
base: main
Are you sure you want to change the base?
Add missing css.properties.display.flow feature #24386
Conversation
This PR adds the missing `flow` member of the `display` CSS property. The data comes from the [mdn-bcd-collector](https://mdn-bcd-collector.gooborg.com) project (v10.12.3). _Check out the [collector's guide on how to review this PR](https://github.com/openwebdocs/mdn-bcd-collector#reviewing-bcd-changes)._ Tests Used: https://mdn-bcd-collector.gooborg.com/tests/css/properties/display/flow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we'd need to test manually that display: flow
has the specified effect.
"firefox": { | ||
"version_added": "≤72" | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to https://bugzil.la/1657594, Firefox may not actually support it:
The Compatibility panel should indicate that
display: flow
is converted todisplay:block
in Firefox,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed with Vinyl, and the question is whether it's always display:block
, or if it is converted to the default value for the element (e.g. display: inline
for <span>
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I generated a test case with ChatGPT: https://developer.mozilla.org/de/play?id=h%2BLlW3ImqADHCJYkMw2xE%2BjFSvlfWApNyUj1hSyEa2VSWUgXOu84ObYg3ftPQZ74pE8GnTGvbE18Lg0V
Tested in Chrome 131, Firefox Nightly 134.0a1 (2024-11-21), and Safari 18.1.1.
All browsers do not seem to support flow
, i.e. the computed value is block
.
But maybe that's in accordance to the spec:
Otherwise it generates a block container box.
So I came up with another (ChatGPT-generated) test case that sets style.display
and reads it again: https://developer.mozilla.org/en-US/play?id=UGBq0JaWyjIGJz8nqIEbFux3a3rDR5mS3uppv7pl%2Bn03MxTNx0%2B%2BXNGDjtIzkD0ICNQbWIjuTJQKhQJm
And this test also fails in all three browsers engines. 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR adds the missing
flow
member of thedisplay
CSS property. The data comes from the mdn-bcd-collector project (v10.12.3).Check out the collector's guide on how to review this PR.
Tests Used: https://mdn-bcd-collector.gooborg.com/tests/css/properties/display/flow