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

Add missing css.properties.display.flow feature #24386

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

queengooborg
Copy link
Contributor

This PR adds the missing flow member of the display 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

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
@queengooborg queengooborg added the data:css Compat data for CSS features. https://developer.mozilla.org/docs/Web/CSS label Sep 9, 2024
Copy link
Contributor

@caugner caugner left a 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.

Comment on lines +239 to +241
"firefox": {
"version_added": "≤72"
},
Copy link
Contributor

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 to display:block in Firefox,

Copy link
Contributor

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>)

Copy link
Contributor

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. 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@github-actions github-actions bot added the size:m [PR only] 25-100 LoC changed label Nov 22, 2024
ShineWellman

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:css Compat data for CSS features. https://developer.mozilla.org/docs/Web/CSS size:m [PR only] 25-100 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants