browser-compat-data icon indicating copy to clipboard operation
browser-compat-data copied to clipboard

Update Firefox versions for AnimationPlaybackEvent API

Open queengooborg opened this issue 3 years ago • 4 comments

This PR updates and corrects the real values for Firefox and Firefox Android for the AnimationPlaybackEvent API, based upon results from the mdn-bcd-collector project (v5.0.0).

Tests Used: https://mdn-bcd-collector.appspot.com/tests/api/AnimationPlaybackEvent

Check out the collector's guide on how to review this PR.

queengooborg avatar Apr 15 '22 12:04 queengooborg

This pull request has merge conflicts that must be resolved before it can be merged.

github-actions[bot] avatar Jun 15 '22 21:06 github-actions[bot]

This doesn't match the other web animation APIs, and the collector is reporting the animation event handlers as supported in Firefox 51. I also don't see any evidence of a prefix. I'll have to do some more digging to see what's going on here.

queengooborg avatar Jun 19 '22 12:06 queengooborg

Okay, so after further analysis, I found the following:

  • The WebIDL was added in https://hg.mozilla.org/mozilla-central/rev/98c90fc9a0bbdb27d894da3c3005e30ccdeef0bc, which indicates Fx 42. In the IDL, nsDocument::IsWebAnimationsEnabled is referenced.
    • The WebIDL also has a constructor which implies the interface should be exposed.
  • Looking at the source code, the function checks StaticPrefs::dom_animations_api_core_enabled(), which translates to the dom.animations-api.core.enabled flag.
  • Checking Firefox 48 through 62, the flag is not enabled by default. Enabling the flag enables this feature.
  • In Firefox 48, enabling or disabling the feature doesn't seem to affect other animation APIs.

This tells me that "63" is accurate for this interface. It probably should have been exposed when the rest of the animation APIs were, but was kept disabled on accident.

queengooborg avatar Sep 06 '22 13:09 queengooborg

Yep, seems like a bunch of things should be marked as shipping in Firefox 63, I found this relating to "dom.animations-api.core.enabled":

  • https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/63#dom
  • https://bugzilla.mozilla.org/show_bug.cgi?id=1476158

However, there doesn't seem to be anything about events here. Per spec it seems like the events using this interface are "cancel", "finish" and "remove". But the "finish" and "cancel" events are marked as supported since Firefox 48.


As a more general point, it would facilitate quick review and ensure high quality data to always look into related API surfaces and say something about them in a PR description. Changing one piece in isolation risks making the overall data inconsistent, and looking around usually reveals other things in need of updating.

foolip avatar Sep 13 '22 14:09 foolip

I checked through the remainder of the interfaces and it looks like this is the only one left that should be set to "63", per the linked release notes -- KeyframeEffect and the properties on Animation are set to "63" as well!

queengooborg avatar Sep 23 '22 08:09 queengooborg

I don't think that we'll have to worry about that, as the collector has still consistently reported "63"!

queengooborg avatar Sep 23 '22 09:09 queengooborg

Oh, so this wasn't a case of an interface object exposed too early. Got it!

foolip avatar Sep 23 '22 09:09 foolip