-
Notifications
You must be signed in to change notification settings - Fork 75
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 features for @import
and @supports
#1353
Conversation
features/feature-queries.yml.dist
Outdated
# edge: "122" | ||
# firefox: "115" | ||
# firefox_android: "115" | ||
- css.at-rules.import.supports |
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 and api.CSSImportRule.supportsText are the same feature, and any differences are either errors in BCD, or mishaps in shipping. The chromestatus entry is https://chromestatus.com/feature/5899007704694784.
@danielsakhapov, does it make sense that the API part shipped first in Chrome 121?
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.
Looking at mdn/browser-compat-data#21603 and mdn/browser-compat-data#22164 and https://issues.chromium.org/issues/40244647#comment15 it's also possible that it all shipped in 122?
(It's unclear to me whether the BCD collector correctly found supportsText
in Chrome 121 beta, or if @Elchi3 added it because Chrome Status incorrectly said it was shipping.)
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.
Hmm, 121 is the correct version according to https://chromiumdash.appspot.com/commit/f9bbbf13060df98c24385bc83da7693d33f3650d
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.
@danielsakhapov helped me figure out what happened here. https://chromiumdash.appspot.com/commit/6ca8ac7e347b73c62609643164a444719f9d26f7 is the commit that actually shipped the feature, in Chrome 122.
The CSSOM bits were accidentally exposed in Chrome 121 and don't do anything. That's actually my fault, as the reviewer of that CL.
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.
For Safari, we'd need to check if the feature is enabled, or if they made the same mistake of exposing the API first.
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.
Hmm, there's is a mismatch in Firefox too...
@ddbeck I'd suggest just putting this subfeature to the side for now.
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.
OK, I've commented it out and left a note back to this discussion. We can get it to when we're in the long tail, I guess?
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.
Identifier should change, but OK to go after that.
features/feature-queries.yml
Outdated
@@ -0,0 +1,16 @@ | |||
name: Feature queries |
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.
Let's make the identifier support
. I'm not sure about the name, whatever seems most common for web developers is OK.
OK, I changed the |
I picked these up because
api.CSSImportRule.supportsText
is newly available.