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 at-rule @keyframes compat data #476

Merged
merged 7 commits into from
Oct 8, 2017
Merged

Conversation

ddbeck
Copy link
Collaborator

@ddbeck ddbeck commented Oct 5, 2017

This PR adds one at-rule, @keyframes. Since it's the first of it's kind, I figured I should open a PR for this one alone.

@teoli2003 teoli2003 added the data:css Compat data for CSS features. https://developer.mozilla.org/docs/Web/CSS label Oct 5, 2017
Copy link
Contributor

@teoli2003 teoli2003 left a comment

Choose a reason for hiding this comment

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

Really good, we just need to get CSS Animations prefixes straight. (And we need to tell this grim and scary story ;-) )

},
{
"prefix": "-o-",
"version_added": "12"
Copy link
Contributor

Choose a reason for hiding this comment

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

"version_removed": "15"

},
"opera": [
{
"version_added": "12.10"
Copy link
Contributor

Choose a reason for hiding this comment

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

"version_removed": "15"

(Opera lost support for unprefixed CSS Animations when moving to Chromium/Blink

"version_added": "12"
}
],
"opera_android": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as Opera.

}
],
"safari_ios": {
"version_added": null
Copy link
Contributor

Choose a reason for hiding this comment

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

Set 'true'. Also add a prefix "-webkit-" version, also set to 'true'

"version_added": "10"
},
"ie_mobile": {
"version_added": null
Copy link
Contributor

Choose a reason for hiding this comment

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

Set to 'true'

"opera": [
{
"version_added": "12.10"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Add an extra entry for the -webkit- prefixing story:
{
"version_added": "15",
"prefix": "-webkit-"
},

{
"version_added": "16"
},
{
Copy link
Contributor

@teoli2003 teoli2003 Oct 6, 2017

Choose a reason for hiding this comment

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

We need to add two {} blocks to describe the story of -webkit- prefixes in Firefox (added in 44 behind a pref, without a pref in 49)

{
  "version_added": "49",
  "prefix": "-webkit-"
},
{
  "version_added": "44",
  "prefix": "-webkit-",
  "flag": {
    "type": "preference",
    "name": "layout.css.prefixes.webkit",
    "value_to_set": "true"
  }
},

"version_added": "5"
}
],
"firefox_android": [
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as Firefox (-webkit- prefixes)

"__compat": {
"mdn_url": "https://developer.mozilla.org/docs/Web/CSS/@keyframes",
"support": {
"webview_android": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Added unprefixed version in "43" too.

"version_added": true
}
],
"chrome_android": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Also supported (same as "chrome")

"safari_ios": {
"version_added": null
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a "status" box. (This is standard behaviour; the spec says 'Animations override all normal rules, but are overriden by !important rules. ')

Copy link
Contributor

@teoli2003 teoli2003 left a comment

Choose a reason for hiding this comment

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

This one is really close to land :-)

@teoli2003
Copy link
Contributor

@Elchi3 Quick question: is it ok to have a dash in the identifier (and in the directory name):

"css": {
    "at-rules": {
      "@keyframes": {

?

@Elchi3
Copy link
Member

Elchi3 commented Oct 6, 2017

Yes, that should be alright. The mdn/data repo also uses "at-rules" and accessing the data using "css.at-rules.@keyframes" should work just fine.

@ddbeck
Copy link
Collaborator Author

ddbeck commented Oct 7, 2017

OK, I think this one is ready for another look, @teoli2003.

This did remind me of a question, too: I've seen tables with Android Webview version numbers that seem to be Android version numbers (e.g., 4.3, 4.4, 5, etc.) and some that correspond to Chrome version numbers (e.g., 36, 43, etc.)? What's the right way to migrate Webview? Keep the number regardless of the sequence they belong to? Go to true when it's an Android version number?

@ddbeck
Copy link
Collaborator Author

ddbeck commented Oct 7, 2017

Also, I agree this is a grim and scary story, but at least it's the right time of year for it. 🎃

@teoli2003
Copy link
Contributor

Before Android 4.4, the Webview wasn't based on Chromium and has only the Android version. From Android 4.4, it is based on Chromium (like 4.4 is based on Chromium 30). So Webview should not be the Android version for 4.4 or larger.

@Elchi3 is adding version validation and I think the Webview versions will added to these checks at some point. Meanwhile, I would say to not lose information (I prefer a 5 that we will convert to a Chrome-based version later to a true that will stay like this forever).

Copy link
Contributor

@teoli2003 teoli2003 left a comment

Choose a reason for hiding this comment

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

LGTM, r+

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants