Closed
Bug 1456394
Opened 6 years ago
Closed 6 years ago
Remove *ReadOnly and AnimationEffectTiming animation interfaces
Categories
(Core :: DOM: Animation, enhancement, P3)
Core
DOM: Animation
Tracking
()
RESOLVED
FIXED
mozilla62
People
(Reporter: birtles, Assigned: birtles)
References
()
Details
(Keywords: dev-doc-complete)
Attachments
(15 files)
59 bytes,
text/x-review-board-request
|
bzbarsky
:
review+
hiro
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
hiro
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
bzbarsky
:
review+
hiro
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
bzbarsky
:
review+
hiro
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
bzbarsky
:
review+
hiro
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
bzbarsky
:
review+
hiro
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
bzbarsky
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
bzbarsky
:
review+
hiro
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
hiro
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
bzbarsky
:
review+
hiro
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
hiro
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
daisuke
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
hiro
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
bzbarsky
:
review+
hiro
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
hiro
:
review+
|
Details |
We need to update our effect API to match the spec changes here: https://github.com/w3c/csswg-drafts/pull/2432 The WPT tests for this have already been merged into mozilla-central (bug 1445877).
Assignee | ||
Comment 1•6 years ago
|
||
Here's a roughly complete patch queue (although most of the commits are hidden by treeherder): https://treeherder.mozilla.org/#/jobs?repo=try&revision=344071a296fb9023eb33f3531c91fd9d961bb60b Although this will bitrot quickly it's probably best to land this after the merge (and after getting back from PTO/Golden Week). I'll fix the inevitable test failures, and rebase on top of 1456688 tomorrow, but then I'll probably defer putting this up for review until May 7.
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → bbirtles
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•6 years ago
|
||
Updated try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=53fa8c0f1e5c238592d18271c4f70828c4a043fc I'll pick this up again after May 7. Hopefully it doesn't bitrot too badly by then.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Updated•6 years ago
|
Comment 18•6 years ago
|
||
mozreview-review |
Comment on attachment 8973604 [details] Bug 1456394 - Update reference to AnimationEffectTiming in wpt tests; https://reviewboard.mozilla.org/r/241986/#review247806
Attachment #8973604 -
Flags: review?(hikezoe) → review+
Comment 19•6 years ago
|
||
mozreview-review |
Comment on attachment 8973601 [details] Bug 1456394 - Replace a few references to 'timing' in DevTools with 'getComputedTiming()' and 'updateTiming()'; https://reviewboard.mozilla.org/r/241980/#review247808
Attachment #8973601 -
Flags: review?(dakatsuka) → review+
Comment 20•6 years ago
|
||
mozreview-review |
Comment on attachment 8973590 [details] Bug 1456394 - Move KeyframeEffect attribute setters to KeyframeEffectReadOnly; https://reviewboard.mozilla.org/r/241958/#review247810 I assume that the test cases marked as FAIL will be resolved in a later patch in this series.
Attachment #8973590 -
Flags: review?(hikezoe) → review+
Comment 21•6 years ago
|
||
mozreview-review |
Comment on attachment 8973591 [details] Bug 1456394 - Moving timing updates to KeyframeEffectReadOnly; https://reviewboard.mozilla.org/r/241960/#review247812
Attachment #8973591 -
Flags: review?(hikezoe) → review+
Comment 22•6 years ago
|
||
mozreview-review |
Comment on attachment 8973592 [details] Bug 1456394 - Move KeyframeEffect constructors to KeyframeEffectReadOnly.{h,cpp}; https://reviewboard.mozilla.org/r/241962/#review247814
Attachment #8973592 -
Flags: review?(hikezoe) → review+
Comment 23•6 years ago
|
||
mozreview-review |
Comment on attachment 8973593 [details] Bug 1456394 - Rename KeyframeEffectReadOnly.{h,cpp} to KeyframeEffect.{h,cpp}; https://reviewboard.mozilla.org/r/241964/#review247816 Looks fine, I haven't audited there is no other usage which didn't appear in this patch, though.
Attachment #8973593 -
Flags: review?(hikezoe) → review+
Comment 24•6 years ago
|
||
mozreview-review |
Comment on attachment 8973594 [details] Bug 1456394 - Merge KeyframeEffectReadOnly and KeyframeEffect; https://reviewboard.mozilla.org/r/241966/#review247818 ::: dom/animation/EffectSet.h:139 (Diff revision 1) > MOZ_ASSERT(!Done()); > mHashIterator.Next(); > return *this; > } > > - dom::KeyframeEffectReadOnly* operator* () > + dom::KeyframeEffect* operator* () Very very minor nit. Do we have a space between operator-blah and () in our style? ::: dom/animation/KeyframeEffect.cpp:664 (Diff revision 1) > // > // In all non-Xray cases, `aGlobal` matches the current Realm, so this > // matches the spec behavior. > // > // In Xray case, the new objects should be created using the document of > - // the target global, but KeyframeEffect and KeyframeEffectReadOnly > + // the target global, but the KeyframeEffect constructors is called in the I am having trouble understanding this comment. I think 'the KeyframeEffect constructors' means now the constructor which is exposed in script, doesn't mean KeyframeEffect constructors in C++, right? Then it should be 'constructor', not prural. Oops. After reading this patch further, there is a copy constructor for KeyframeEffect. So, 'constructors are called'? ::: dom/animation/KeyframeEffect.cpp:707 (Diff revision 1) > return nullptr; > } > > - // Create a new KeyframeEffectReadOnly object with aSource's target, > + // Create a new KeyframeEffect object with aSource's target, > // iteration composite operation, composite operation, and spacing mode. > - // The constructor creates a new AnimationEffect(ReadOnly) object by > + // The constructor creates a new AnimationEffectTiming object by Oops. the comment was originally wrong, that was reviwed by me. :/
Attachment #8973594 -
Flags: review?(hikezoe) → review+
Comment 25•6 years ago
|
||
mozreview-review |
Comment on attachment 8973595 [details] Bug 1456394 - Rename AnimationEffectReadOnly to AnimationEffect; https://reviewboard.mozilla.org/r/241968/#review247822
Attachment #8973595 -
Flags: review?(hikezoe) → review+
Comment 26•6 years ago
|
||
mozreview-review |
Comment on attachment 8973597 [details] Bug 1456394 - Add AnimationEffect.getTiming(); https://reviewboard.mozilla.org/r/241972/#review247826
Attachment #8973597 -
Flags: review?(hikezoe) → review+
Comment 27•6 years ago
|
||
mozreview-review |
Comment on attachment 8973598 [details] Bug 1456394 - Factor out TimingParams::FromEffectTiming; https://reviewboard.mozilla.org/r/241974/#review247830
Attachment #8973598 -
Flags: review?(hikezoe) → review+
Comment 28•6 years ago
|
||
mozreview-review |
Comment on attachment 8973597 [details] Bug 1456394 - Add AnimationEffect.getTiming(); https://reviewboard.mozilla.org/r/241972/#review247834 ::: commit-message-7b54b:1 (Diff revision 1) > +Bug 1456394 - Add AnimationEffect.getTiming(); r?hiro,bz We should put the link to the spec here (especially for bz).
Comment 29•6 years ago
|
||
mozreview-review |
Comment on attachment 8973600 [details] Bug 1456394 - Make TimingParameters::operator== compare end delay values; https://reviewboard.mozilla.org/r/241978/#review247840
Attachment #8973600 -
Flags: review?(hikezoe) → review+
Comment 30•6 years ago
|
||
mozreview-review |
Comment on attachment 8973602 [details] Bug 1456394 - Use updateTiming() in animation tests; https://reviewboard.mozilla.org/r/241982/#review247842
Attachment #8973602 -
Flags: review?(hikezoe) → review+
Comment 31•6 years ago
|
||
mozreview-review |
Comment on attachment 8973599 [details] Bug 1456394 - Add AnimationEffect.updateTiming(); https://reviewboard.mozilla.org/r/241976/#review247832 I don't quite understand why we don't use mDocument for nsAutoAnimationMutationBatch but I believe bz will audit the part. ::: commit-message-cb9f9:1 (Diff revision 1) > +Bug 1456394 - Add AnimationEffect.updateTiming(); r?hiro,bz Should we put the link to the spec of updateTiming() here in this commit message? ::: dom/animation/AnimationEffect.cpp:85 (Diff revision 1) > + Maybe<nsAutoAnimationMutationBatch> mb; > + if (AsKeyframeEffect() && AsKeyframeEffect()->GetTarget()) { > + mb.emplace(AsKeyframeEffect()->GetTarget()->mElement->OwnerDoc()); > + } This part is completely out of my knowledge. Can you please add a comment to explain why we don't use mDocument instead? I guess it's related to bug 1414674 or some others, but I don't quite understand them.
Attachment #8973599 -
Flags: review?(hikezoe) → review+
Comment 32•6 years ago
|
||
mozreview-review |
Comment on attachment 8973603 [details] Bug 1456394 - Drop AnimationEffectTiming(ReadOnly) interfaces; https://reviewboard.mozilla.org/r/241984/#review247854 ::: dom/animation/AnimationEffect.cpp:341 (Diff revision 1) > SetSpecifiedTiming(timing); > } > > AnimationEffect::~AnimationEffect() > { > - // mTiming is cycle collected, so we have to do null check first even though > + // This is just here so Animation can be an incomplete type in the header. Just curious, where is/are the place(s) that requires Animation is an incomplete type?
Attachment #8973603 -
Flags: review?(hikezoe) → review+
Assignee | ||
Updated•6 years ago
|
Summary: Remove *ReadOnly animation interfaces → Remove *ReadOnly and AnimationEffectTiming animation interfaces
Assignee | ||
Updated•6 years ago
|
Priority: -- → P3
Assignee | ||
Comment 33•6 years ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #20) > Comment on attachment 8973590 [details] > Bug 1456394 - Move KeyframeEffect attribute setters to > KeyframeEffectReadOnly; > > https://reviewboard.mozilla.org/r/241958/#review247810 > > I assume that the test cases marked as FAIL will be resolved in a later > patch in this series. Yes, that's right. (In reply to Hiroyuki Ikezoe (:hiro) from comment #24) > Comment on attachment 8973594 [details] > Bug 1456394 - Merge KeyframeEffectReadOnly and KeyframeEffect; > > https://reviewboard.mozilla.org/r/241966/#review247818 > > ::: dom/animation/EffectSet.h:139 > (Diff revision 1) > > MOZ_ASSERT(!Done()); > > mHashIterator.Next(); > > return *this; > > } > > > > - dom::KeyframeEffectReadOnly* operator* () > > + dom::KeyframeEffect* operator* () > > Very very minor nit. Do we have a space between operator-blah and () in our > style? Looking at our codebase, it seems we don't normally. Fixed now in this patch. > ::: dom/animation/KeyframeEffect.cpp:664 > (Diff revision 1) > > // > > // In all non-Xray cases, `aGlobal` matches the current Realm, so this > > // matches the spec behavior. > > // > > // In Xray case, the new objects should be created using the document of > > - // the target global, but KeyframeEffect and KeyframeEffectReadOnly > > + // the target global, but the KeyframeEffect constructors is called in the > > I am having trouble understanding this comment. I think 'the > KeyframeEffect constructors' means now the constructor which is exposed in > script, doesn't mean KeyframeEffect constructors in C++, right? Then it > should be 'constructor', not prural. > > Oops. After reading this patch further, there is a copy constructor for > KeyframeEffect. So, 'constructors are called'? Fixed.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 49•6 years ago
|
||
Rebased but mercurial handles rebasing with file renames so poorly the chance I broke something in the process is pretty high.
Assignee | ||
Comment 50•6 years ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #32) > Comment on attachment 8973603 [details] > Bug 1456394 - Drop AnimationEffectTiming(ReadOnly) interfaces; > > https://reviewboard.mozilla.org/r/241984/#review247854 > > ::: dom/animation/AnimationEffect.cpp:341 > (Diff revision 1) > > SetSpecifiedTiming(timing); > > } > > > > AnimationEffect::~AnimationEffect() > > { > > - // mTiming is cycle collected, so we have to do null check first even though > > + // This is just here so Animation can be an incomplete type in the header. > > Just curious, where is/are the place(s) that requires Animation is an > incomplete type? There aren't any anymore! I wrote these patches a couple of months ago and there were at that time but it looks like they're gone now.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 53•6 years ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #31) > Comment on attachment 8973599 [details] > Bug 1456394 - Add AnimationEffect.updateTiming(); > ... > ::: dom/animation/AnimationEffect.cpp:85 > (Diff revision 1) > > + Maybe<nsAutoAnimationMutationBatch> mb; > > + if (AsKeyframeEffect() && AsKeyframeEffect()->GetTarget()) { > > + mb.emplace(AsKeyframeEffect()->GetTarget()->mElement->OwnerDoc()); > > + } > > This part is completely out of my knowledge. Can you please add a comment > to explain why we don't use mDocument instead? I guess it's related to bug > 1414674 or some others, but I don't quite understand them. mDocument is the document from the current realm where the KeyframeEffect was constructed. However, for animation observers we always use the owner document for the target element of the effect.
Comment 54•6 years ago
|
||
(In reply to Brian Birtles (:birtles) from comment #53) > (In reply to Hiroyuki Ikezoe (:hiro) from comment #31) > > Comment on attachment 8973599 [details] > > Bug 1456394 - Add AnimationEffect.updateTiming(); > > ... > > ::: dom/animation/AnimationEffect.cpp:85 > > (Diff revision 1) > > > + Maybe<nsAutoAnimationMutationBatch> mb; > > > + if (AsKeyframeEffect() && AsKeyframeEffect()->GetTarget()) { > > > + mb.emplace(AsKeyframeEffect()->GetTarget()->mElement->OwnerDoc()); > > > + } > > > > This part is completely out of my knowledge. Can you please add a comment > > to explain why we don't use mDocument instead? I guess it's related to bug > > 1414674 or some others, but I don't quite understand them. > > mDocument is the document from the current realm where the KeyframeEffect > was constructed. > > However, for animation observers we always use the owner document for the > target element of the effect. Oh I see. Thanks for the explanation. It would be nice to have a test case for our animation observer tests at some point. :)
Comment 55•6 years ago
|
||
mozreview-review |
Comment on attachment 8973590 [details] Bug 1456394 - Move KeyframeEffect attribute setters to KeyframeEffectReadOnly; https://reviewboard.mozilla.org/r/241958/#review249084 r=me on the webidl bits
Attachment #8973590 -
Flags: review?(bzbarsky) → review+
Comment 56•6 years ago
|
||
mozreview-review |
Comment on attachment 8973592 [details] Bug 1456394 - Move KeyframeEffect constructors to KeyframeEffectReadOnly.{h,cpp}; https://reviewboard.mozilla.org/r/241962/#review249086 ::: commit-message-930df:5 (Diff revision 2) > +Bug 1456394 - Move KeyframeEffect constructors to KeyframeEffectReadOnly.{h,cpp}; r?hiro,bz > + > +By doing this we will have all the KeyframeEffect* related code in > +KeyframeEffectReadOnly.{h,cpp} so we can rename them to KeyframeEffect.{h,cpp} > +in the next patch and make it easier the history for the bulk of this code. "make it easier to examine the history"?
Attachment #8973592 -
Flags: review?(bzbarsky) → review+
Comment 57•6 years ago
|
||
mozreview-review |
Comment on attachment 8973593 [details] Bug 1456394 - Rename KeyframeEffectReadOnly.{h,cpp} to KeyframeEffect.{h,cpp}; https://reviewboard.mozilla.org/r/241964/#review249088
Attachment #8973593 -
Flags: review?(bzbarsky) → review+
Comment 58•6 years ago
|
||
mozreview-review |
Comment on attachment 8973594 [details] Bug 1456394 - Merge KeyframeEffectReadOnly and KeyframeEffect; https://reviewboard.mozilla.org/r/241966/#review249090 r=me on the webidl and test_interfaces changes. I didn't read the rest very carefully, though I did skim it some.
Attachment #8973594 -
Flags: review?(bzbarsky) → review+
Comment 59•6 years ago
|
||
mozreview-review |
Comment on attachment 8973595 [details] Bug 1456394 - Rename AnimationEffectReadOnly to AnimationEffect; https://reviewboard.mozilla.org/r/241968/#review249092 r=me on the webidl and test_interfaces changes.
Attachment #8973595 -
Flags: review?(bzbarsky) → review+
Comment 60•6 years ago
|
||
mozreview-review |
Comment on attachment 8973596 [details] Bug 1456394 - Rename animation timing dictionaries; https://reviewboard.mozilla.org/r/241970/#review249094 r=me
Attachment #8973596 -
Flags: review?(bzbarsky) → review+
Comment 61•6 years ago
|
||
mozreview-review |
Comment on attachment 8973597 [details] Bug 1456394 - Add AnimationEffect.getTiming(); https://reviewboard.mozilla.org/r/241972/#review249096 r=me on the webidl bits
Attachment #8973597 -
Flags: review?(bzbarsky) → review+
Comment 62•6 years ago
|
||
mozreview-review |
Comment on attachment 8973599 [details] Bug 1456394 - Add AnimationEffect.updateTiming(); https://reviewboard.mozilla.org/r/241976/#review249112 r=me on the webidl bits. ::: dom/animation/TimingParams.h:82 (Diff revision 2) > nsIDocument* aDocument, ErrorResult& aRv); > static TimingParams FromEffectTiming( > const dom::EffectTiming& aEffectTiming, > nsIDocument* aDocument, > ErrorResult& aRv); > + static TimingParams MergeOptionalEffectTiming( This should really be documented. What does it do?
Attachment #8973599 -
Flags: review?(bzbarsky) → review+
Comment 63•6 years ago
|
||
mozreview-review |
Comment on attachment 8973603 [details] Bug 1456394 - Drop AnimationEffectTiming(ReadOnly) interfaces; https://reviewboard.mozilla.org/r/241984/#review249114 r=me on the webidl and test_interfaces changes.
Attachment #8973603 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 64•6 years ago
|
||
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #62) > Comment on attachment 8973599 [details] > Bug 1456394 - Add AnimationEffect.updateTiming(); > > https://reviewboard.mozilla.org/r/241976/#review249112 > > r=me on the webidl bits. > > ::: dom/animation/TimingParams.h:82 > (Diff revision 2) > > nsIDocument* aDocument, ErrorResult& aRv); > > static TimingParams FromEffectTiming( > > const dom::EffectTiming& aEffectTiming, > > nsIDocument* aDocument, > > ErrorResult& aRv); > > + static TimingParams MergeOptionalEffectTiming( > > This should really be documented. What does it do? Updated with the following comment: // Returns a copy of |aSource| where each timing property in |aSource| that // is also specified in |aEffectTiming| is replaced with the value from // |aEffectTiming|. // // If any of the values in |aEffectTiming| are invalid, |aRv.Failed()| will be // true and an unmodified copy of |aSource| will be returned.
Assignee | ||
Comment 65•6 years ago
|
||
Thanks for the reviews!
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 81•6 years ago
|
||
Pushed by bbirtles@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/45e028b105e5 Move KeyframeEffect attribute setters to KeyframeEffectReadOnly; r=bz,hiro https://hg.mozilla.org/integration/autoland/rev/831f34bd841d Moving timing updates to KeyframeEffectReadOnly; r=hiro https://hg.mozilla.org/integration/autoland/rev/5f8a3f4a435a Move KeyframeEffect constructors to KeyframeEffectReadOnly.{h,cpp}; r=bz,hiro https://hg.mozilla.org/integration/autoland/rev/ede7f6c30b62 Rename KeyframeEffectReadOnly.{h,cpp} to KeyframeEffect.{h,cpp}; r=bz,hiro https://hg.mozilla.org/integration/autoland/rev/238d0908de38 Merge KeyframeEffectReadOnly and KeyframeEffect; r=bz,hiro https://hg.mozilla.org/integration/autoland/rev/7eae2c106d30 Rename AnimationEffectReadOnly to AnimationEffect; r=bz,hiro https://hg.mozilla.org/integration/autoland/rev/97256bd7ef99 Rename animation timing dictionaries; r=bz https://hg.mozilla.org/integration/autoland/rev/995676902281 Add AnimationEffect.getTiming(); r=bz,hiro https://hg.mozilla.org/integration/autoland/rev/2d36f43a3807 Factor out TimingParams::FromEffectTiming; r=hiro https://hg.mozilla.org/integration/autoland/rev/ea71864d99b4 Add AnimationEffect.updateTiming(); r=bz,hiro https://hg.mozilla.org/integration/autoland/rev/ab4634b0c0b7 Make TimingParameters::operator== compare end delay values; r=hiro https://hg.mozilla.org/integration/autoland/rev/0ab592b16b04 Replace a few references to 'timing' in DevTools with 'getComputedTiming()' and 'updateTiming()'; r=daisuke https://hg.mozilla.org/integration/autoland/rev/e85f004907be Use updateTiming() in animation tests; r=hiro https://hg.mozilla.org/integration/autoland/rev/99694c74c0f4 Drop AnimationEffectTiming(ReadOnly) interfaces; r=bz,hiro https://hg.mozilla.org/integration/autoland/rev/8e9a4a323f0c Update reference to AnimationEffectTiming in wpt tests; r=hiro
Created web-platform-tests PR https://github.com/w3c/web-platform-tests/pull/10961 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Comment 84•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/45e028b105e5 https://hg.mozilla.org/mozilla-central/rev/831f34bd841d https://hg.mozilla.org/mozilla-central/rev/5f8a3f4a435a https://hg.mozilla.org/mozilla-central/rev/ede7f6c30b62 https://hg.mozilla.org/mozilla-central/rev/238d0908de38 https://hg.mozilla.org/mozilla-central/rev/7eae2c106d30 https://hg.mozilla.org/mozilla-central/rev/97256bd7ef99 https://hg.mozilla.org/mozilla-central/rev/995676902281 https://hg.mozilla.org/mozilla-central/rev/2d36f43a3807 https://hg.mozilla.org/mozilla-central/rev/ea71864d99b4 https://hg.mozilla.org/mozilla-central/rev/ab4634b0c0b7 https://hg.mozilla.org/mozilla-central/rev/0ab592b16b04 https://hg.mozilla.org/mozilla-central/rev/e85f004907be https://hg.mozilla.org/mozilla-central/rev/99694c74c0f4 https://hg.mozilla.org/mozilla-central/rev/8e9a4a323f0c
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Upstream PR merged
Updated•6 years ago
|
Updated•6 years ago
|
Keywords: dev-doc-needed
Comment 86•6 years ago
|
||
Here's a great comment that summarizes these changes: https://github.com/w3c/csswg-drafts/pull/2432#issue-174270202 For MDN, I did: Compat data updates: https://github.com/mdn/browser-compat-data/pull/2481 https://github.com/mdn/browser-compat-data/pull/2482 https://github.com/mdn/browser-compat-data/pull/2483 https://github.com/mdn/browser-compat-data/pull/2485 Navigation updates: https://github.com/mdn/kumascript/pull/742 Compat table updates and navigation updates will be deployed to MDN soon, but it's not there yet as of now (will be later this week). I went ahead and did update a bunch of MDN wiki pages for this already, though: Updated MDN pages for AnimationEffectReadOnly -> AnimationEffect: https://developer.mozilla.org/en-US/docs/Web/API/AnimationEffect https://developer.mozilla.org/en-US/docs/Web/API/AnimationEffect/getTiming https://developer.mozilla.org/en-US/docs/Web/API/AnimationEffect/getComputedTiming https://developer.mozilla.org/en-US/docs/Web/API/AnimationEffect/updateTiming (and AnimationEffectReadOnly pages now redirect to AnimationEffect) AnimationEffectTiming(ReadOnly) are gone, so these pages are deleted now: https://developer.mozilla.org/en-US/docs/Web/API/AnimationEffectTiming https://developer.mozilla.org/en-US/docs/Web/API/AnimationEffectTimingReadOnly https://developer.mozilla.org/en-US/docs/Web/API/AnimationEffectTimingReadOnly/delay https://developer.mozilla.org/en-US/docs/Web/API/AnimationEffectTimingReadOnly/direction https://developer.mozilla.org/en-US/docs/Web/API/AnimationEffectTimingReadOnly/duration https://developer.mozilla.org/en-US/docs/Web/API/AnimationEffectTimingReadOnly/easing https://developer.mozilla.org/en-US/docs/Web/API/AnimationEffectTimingReadOnly/endDelay https://developer.mozilla.org/en-US/docs/Web/API/AnimationEffectTimingReadOnly/fill https://developer.mozilla.org/en-US/docs/Web/API/AnimationEffectTimingReadOnly/iterations https://developer.mozilla.org/en-US/docs/Web/API/AnimationEffectTimingReadOnly/iterationStart Updated MDN pages for AnimationEffectTimingProperties -> EffectTiming https://developer.mozilla.org/en-US/docs/Web/API/EffectTiming https://developer.mozilla.org/en-US/docs/Web/API/EffectTiming/delay https://developer.mozilla.org/en-US/docs/Web/API/EffectTiming/direction https://developer.mozilla.org/en-US/docs/Web/API/EffectTiming/duration https://developer.mozilla.org/en-US/docs/Web/API/EffectTiming/easing https://developer.mozilla.org/en-US/docs/Web/API/EffectTiming/endDelay https://developer.mozilla.org/en-US/docs/Web/API/EffectTiming/fill https://developer.mozilla.org/en-US/docs/Web/API/EffectTiming/iterations https://developer.mozilla.org/en-US/docs/Web/API/EffectTiming/iterationStart (and AnimationEffectTimingProperties pages now redirect to EffectTiming) KeyframeEffectReadOnly is gone, so pages redirect to KeyframeEffect and these have been updated: https://developer.mozilla.org/en-US/docs/Web/API/KeyframeEffect https://developer.mozilla.org/docs/Web/API/KeyframeEffect/KeyframeEffect https://developer.mozilla.org/docs/Web/API/KeyframeEffect/composite https://developer.mozilla.org/docs/Web/API/KeyframeEffect/iterationComposite https://developer.mozilla.org/docs/Web/API/KeyframeEffect/getKeyframes https://developer.mozilla.org/docs/Web/API/KeyframeEffect/setKeyframes https://developer.mozilla.org/docs/Web/API/KeyframeEffect/target Reviews and fixes to these MDN wiki pages are welcome, but let me know if I got something terribly wrong and I will fix it up.
Assignee | ||
Comment 87•6 years ago
|
||
Looks perfect, thanks Florian!
Comment 88•6 years ago
|
||
Thanks, the navigation and the compat tables are updated now as well.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•