Description was changed from ========== Rename AnimationPlayerEvent as AnimationPlaybackEvent The Web Animations spec now uses ...
4 years, 3 months ago
(2016-09-14 07:40:08 UTC)
#3
Description was changed from
==========
Rename AnimationPlayerEvent as AnimationPlaybackEvent
The Web Animations spec now uses the name AnimationPlaybackEvent instead of
AnimationPlayerEvent:
http://w3c.github.io/web-animations/#the-animationplaybackevent-interface
BUG=624639
==========
to
==========
Rename AnimationPlayerEvent as AnimationPlaybackEvent
The Web Animations spec now uses the name AnimationPlaybackEvent instead of
AnimationPlayerEvent:
http://w3c.github.io/web-animations/#the-animationplaybackevent-interface
This patch also makes the timelineTime property of AnimationPlaybackEvent
nullable, as specced.
BUG=624639
==========
suzyh_UTC10 (ex-contributor)
The CQ bit was checked by suzyh@chromium.org to run a CQ dry run
4 years, 3 months ago
(2016-09-14 07:41:02 UTC)
#4
lgtm https://codereview.chromium.org/2344473002/diff/20001/third_party/WebKit/Source/core/events/AnimationPlaybackEvent.cpp File third_party/WebKit/Source/core/events/AnimationPlaybackEvent.cpp (right): https://codereview.chromium.org/2344473002/diff/20001/third_party/WebKit/Source/core/events/AnimationPlaybackEvent.cpp#newcode21 third_party/WebKit/Source/core/events/AnimationPlaybackEvent.cpp:21: if (initializer.hasCurrentTime()) Per spec, but do you know ...
4 years, 3 months ago
(2016-09-15 15:49:17 UTC)
#10
lgtm
https://codereview.chromium.org/2344473002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/events/AnimationPlaybackEvent.cpp (right):
https://codereview.chromium.org/2344473002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/events/AnimationPlaybackEvent.cpp:21: if
(initializer.hasCurrentTime())
Per spec, but do you know if the only reason that these are nullable on event
interface and dict is so that scripts don't have to provide a value? If the UA
itself can never fire events where either of these are null, I'd suggest
changing the spec to make the dict and its members required and everything
non-nullable. This file would shrink a bit.
4 years, 3 months ago
(2016-09-15 15:59:03 UTC)
#12
bindings/ LGTM
suzyh_UTC10 (ex-contributor)
On 2016/09/15 at 15:49:17, foolip wrote: > lgtm > > https://codereview.chromium.org/2344473002/diff/20001/third_party/WebKit/Source/core/events/AnimationPlaybackEvent.cpp > File third_party/WebKit/Source/core/events/AnimationPlaybackEvent.cpp (right): ...
4 years, 3 months ago
(2016-09-16 00:32:00 UTC)
#13
On 2016/09/15 at 15:49:17, foolip wrote:
> lgtm
>
>
https://codereview.chromium.org/2344473002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/events/AnimationPlaybackEvent.cpp (right):
>
>
https://codereview.chromium.org/2344473002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/events/AnimationPlaybackEvent.cpp:21: if
(initializer.hasCurrentTime())
> Per spec, but do you know if the only reason that these are nullable on event
interface and dict is so that scripts don't have to provide a value? If the UA
itself can never fire events where either of these are null, I'd suggest
changing the spec to make the dict and its members required and everything
non-nullable. This file would shrink a bit.
I checked with Shane and he pointed out that it's possible for currentTime to be
null, and so the point you make does not apply in this case.
On 2016/09/15 at 15:51:41, foolip wrote:
> On 2016/09/14 21:34:29, suzyh wrote:
> > haraken: please review bindings/
> >
> > foolip: please review core/
> > One thing I'm not entirely sure about is some references to the old name in
a
> > histogram/UseCounter added in
> >
https://chromium.googlesource.com/chromium/src/+/b864804d2c75a5024c4b6d57ee23...
> > and removed from UseCounter.h by you in
> >
https://chromium.googlesource.com/chromium/src/+/b2343c3b045d6ac978a4e9b003a2...
>
> Things in histograms.xml just stay forever, that's fine.
>
> > plus another reference that seems to be related to this, in
> >
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/imported/...
> > Is there anything additional I'd need to do here?
>
> Continuing to test the old name would be odd, so I'd either rename or remove
the line. It doesn't matter really since new events aren't getting this
treatment any more.
Since this is an imported test, I'll send a PR to WPT directly to remove this
line. Thanks!
suzyh_UTC10 (ex-contributor)
The CQ bit was checked by suzyh@chromium.org
4 years, 3 months ago
(2016-09-16 00:32:09 UTC)
#14
4 years, 3 months ago
(2016-09-16 04:48:00 UTC)
#16
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
commit-bot: I haz the power
Description was changed from ========== Rename AnimationPlayerEvent as AnimationPlaybackEvent The Web Animations spec now uses ...
4 years, 3 months ago
(2016-09-16 04:49:48 UTC)
#17
Message was sent while issue was closed.
Description was changed from
==========
Rename AnimationPlayerEvent as AnimationPlaybackEvent
The Web Animations spec now uses the name AnimationPlaybackEvent instead of
AnimationPlayerEvent:
http://w3c.github.io/web-animations/#the-animationplaybackevent-interface
This patch also makes the timelineTime property of AnimationPlaybackEvent
nullable, as specced.
BUG=624639
==========
to
==========
Rename AnimationPlayerEvent as AnimationPlaybackEvent
The Web Animations spec now uses the name AnimationPlaybackEvent instead of
AnimationPlayerEvent:
http://w3c.github.io/web-animations/#the-animationplaybackevent-interface
This patch also makes the timelineTime property of AnimationPlaybackEvent
nullable, as specced.
BUG=624639
Committed: https://crrev.com/fb139e4ad08c64b1940c5ce4eed21025184d0a1e
Cr-Commit-Position: refs/heads/master@{#419103}
==========
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/fb139e4ad08c64b1940c5ce4eed21025184d0a1e Cr-Commit-Position: refs/heads/master@{#419103}
4 years, 3 months ago
(2016-09-16 04:49:49 UTC)
#18
Issue 2344473002: Rename AnimationPlayerEvent as AnimationPlaybackEvent
(Closed)
Created 4 years, 3 months ago by suzyh_UTC10 (ex-contributor)
Modified 4 years, 3 months ago
Reviewers: foolip, haraken
Base URL:
Comments: 1