-
Notifications
You must be signed in to change notification settings - Fork 102
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 keys from BCD 5.6.6 release to existing features #1972
Conversation
# baseline: high | ||
# baseline_low_date: 2020-01-15 | ||
# baseline_high_date: 2022-07-15 | ||
# support: | ||
# chrome: "50" | ||
# chrome_android: "50" | ||
# edge: "79" | ||
# firefox: "50" | ||
# firefox_android: "50" | ||
# safari: "9" | ||
# safari_ios: "9" | ||
- css.properties.column-width.auto |
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.
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'm confused here. The browser versions for the
auto
value on MDN doesn't match what we have here:
Interesting. It looks like column-width
was prefixed in Chrome until version 50, and compute-baseline
is able to pick up that the auto
keyword would have been only usable with a prefixed property. So I think what is in the dist here is correct, but I'm not sure if BCD needs an update of some sort.
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.
That's right. compute-baseline
checks ancestor compat features (we turned this on in #1485). This is an oddity of BCD's data model. It can't directly express the idea that a feature is, for example, behind a prefix when some parent of that feature is behind a prefix.
features/object-position.yml
Outdated
@@ -3,5 +3,8 @@ description: The `object-position` CSS property places images, videos, and other | |||
spec: https://drafts.csswg.org/css-images-3/#the-object-position | |||
# object-fit on caniuse includes both object-fit and object-position. | |||
# `caniuse: object-fit` is set in object-fit.yml | |||
status: | |||
compute_from: css.properties.object-position |
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.
The description says "images, videos, and other replaced elements", but now we're ignoring iframes, which is also a replaced element. So I think we should do something like this: call out the iframe exception in the description, and add a TODO comment about removing the callout once iframes are taken into account.
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 really dislike the way this information has been represented in BCD. I think the addition of the applies_to_iframe_elements
key ought to be reverted (and left a comment saying so). Until mdn/browser-compat-data#23586 is resolved, I'd suggest:
- Undoing all the changes in this feature so far
- Adding a comment mentioning
css.properties.object-position.applies_to_iframe_elements
and css.properties.object-position - BCD on this doesn't cover support in iframe mdn/browser-compat-data#23586 and saying that the compat data for which elementsobject-position
works with is unclear and needs to be sorted out upstream.
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.
Updated in e95bb19
I'm going to move head with this. @captainbrosset, if you still have suggestions or concerns, would you open a follow up PR or issue? Thank you! |
New keys introduced by BCD 5.6.6:
css.properties.white-space-collapse.preserve-spaces
(added compute_from to avoid baseline impact)css.properties.column-width.auto
(no impact to baseline)css.properties.object-position.applies_to_iframe_elements
(added compute_from to avoid baseline impact)