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

Opera still supports @keyframes #3227

Merged
merged 2 commits into from Dec 21, 2018

Conversation

Projects
None yet
2 participants
@antross
Copy link
Contributor

antross commented Dec 18, 2018

Looks like a typo in the data. The examples from https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Animations/Using_CSS_animations still work in the latest version of Opera.

@ddbeck
Copy link
Collaborator

ddbeck left a comment

Thanks for opening this PR, @antross, and welcome to BCD! This is a good catch. The data for Opera on this feature is a bit messy and I think we can improve it significantly.

@@ -84,8 +84,7 @@
},
"opera": [
{
"version_added": "12.1",
"version_removed": "15"
"version_added": "12.1"

This comment has been minimized.

@ddbeck

ddbeck Dec 20, 2018

Collaborator

I think the data here isn't quite wrong, but it's definitely not complete. It looks like, in the transition from Presto to Blink, Opera did indeed remove support for unprefixed @keyframes at version 15. Chrome unprefixed @keyframes in version 43, which suggests that Opera unprefixed in version 30 (i.e., Chrome version number - 13). This is corroborated by the Chrome Platform Status entry.

Instead, I think we need to restructure the Opera data with support statements in this order:

  1. (first position) A new entry with only "version_added": "30"
  2. Move the -webkit- data to this position
  3. Revert the changes you made to the 12.1 to 15 data
  4. (last position) The existing -o- prefix data

Also, while we're at it, it would be nice to see the indentation fixed to align with the other data in this file (the "opera" line seems to have started the misalignment). This isn't from your change, just an existing quirk.

I realize this feedback is a bit bigger than you probably had in mind, @antross! If you're keen to push a new commit or two to make these changes, I think it would be a big help! Thank you!

This comment has been minimized.

@antross

antross Dec 20, 2018

Author Contributor

I realize this feedback is a bit bigger than you probably had in mind, @antross! If you're keen to push a new commit or two to make these changes, I think it would be a big help!

Absolutely happy to make the additional changes here and thanks @ddbeck for the detailed explanation! I'll try to update the PR later today.

This comment has been minimized.

@antross

antross Dec 20, 2018

Author Contributor

Also, while we're at it, it would be nice to see the indentation fixed to align with the other data in this file (the "opera" line seems to have started the misalignment). This isn't from your change, just an existing quirk.

@ddbeck I'm not sure I see the misalignment you referred to. The data immediately prior to "opera" for "ie" is at a slightly different indent, but that's because it's composed of a single version entry as opposed to an array containing multiple entries.

This comment has been minimized.

@ddbeck

ddbeck Dec 21, 2018

Collaborator

@antross Sorry, I looked at the actual file on my machine and the indentation is fine. I think it was a quirk of how GitHub shows the expanded context. My mistake!

screen shot 2018-12-21 at 1 52 06 pm

@ddbeck

ddbeck approved these changes Dec 21, 2018

Copy link
Collaborator

ddbeck left a comment

Excellent update, @antross. Thank you! 🎉

@ddbeck ddbeck merged commit 8d168ca into mdn:master Dec 21, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

antross added a commit to antross/hint that referenced this pull request Jan 24, 2019

Chore: Remove obsolete failing test
The `keyframes` property is supported by Opera 15, but was
previously incorrectly reported as unsupported due to a bug in
the MDN data. This bug was fixed in
mdn/browser-compat-data#3227.

No other un-prefixed CSS feature exists which is both included
in the MDN data and has been removed by a browser so removing
this test.

@antross antross referenced this pull request Jan 24, 2019

Closed

Chore: Remove obsolete failing test #1760

3 of 3 tasks complete

alrra added a commit to webhintio/hint that referenced this pull request Jan 24, 2019

Chore: Remove obsolete failing test
The `keyframes` property is supported by Opera 15, but was
previously incorrectly reported as unsupported due to a bug
in the MDN data. This bug was fixed in
mdn/browser-compat-data#3227.

No other un-prefixed CSS feature exists which is both included
in the MDN data and has been removed by a browser so removing
this test.

Close #1760
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment