Rename AnimationTimeline to DocumentTimeline

RESOLVED FIXED in Firefox 40

Status

()

Core
DOM
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: birtles, Assigned: birtles)

Tracking

({dev-doc-needed})

Trunk
mozilla40
dev-doc-needed
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(3 attachments)

(Assignee)

Description

3 years ago
The Web Animations spec has been updated such that AnimationTimeline is now an abstract interface for representing potentially many different types of timelines. The concrete timeline used by default for CSS Animations etc., i.e. the document timeline, is called DocumentTimeline.

We don't plan to implement other types of timelines yet but we should rename the existing names to match the spec for now.
(Assignee)

Updated

3 years ago
Depends on: 1152619
(Assignee)

Comment 1

3 years ago
Created attachment 8590624 [details] [diff] [review]
part 1 - Remove AnimationTimeline IDL tests from dom/animation/tests

The tests in dom/animation/tests/ use an old version of idlharness.js that
doesn't support inherited interfaces. As discussed in bug 1152619 we're not
looking at updating these old tests (under dom/imptests) at the moment which
means we won't be able to update the IDL tests in dom/animation/tests/ to
continue passing once we introduce DocumentTimeline as a subinterface of
AnimationTimeline.

As a result, this patch simply the removes the IDL tests for this interface from
dom/animation/tests. However, we have a test for this interface in
web-platform-tests where I've set up a pull request to apply the required
renaming so we should eventually get test coverage for this renaming.

  https://github.com/w3c/web-platform-tests/pull/1748

In the long run, all the tests in dom/animation/tests should end up in
web-platform-tests. The main reason they aren't there yet is that most of them
test the mapping between the Web Animations API and CSS and there's currently no
spec defining that so there's no place to put them in the web-platform-tests
repository.

There are a few tests for animation timeline which could be landed in
web-platform-tests (and then removed from dom/animation/tests) but we need to
discuss with Google if this is the desired behavior or not first. For the time
being I have a branch setup for that and I'm leaving the tests in
dom/animation/tests so we continue to test what *we* think the behavior should
be in the meantime. That branch is here:

  https://github.com/birtles/web-platform-tests/compare/rename-animation-timeline...birtles:add-hidden-iframe-tests
Attachment #8590624 - Flags: review?(james)
(Assignee)

Comment 2

3 years ago
Created attachment 8590627 [details] [diff] [review]
part 2 - Rename AnimationTimeline to DocumentTimeline

And then re-add AnimationTimeline has an abstract super-class of
DocumentTimeline.
Attachment #8590627 - Flags: review?(bugs)
(Assignee)

Comment 3

3 years ago
Created attachment 8590628 [details] [diff] [review]
part 3 - Update web-platform-tests expectations

Until the renaming PR (https://github.com/w3c/web-platform-tests/pull/1748)
lands upstream and we update, this test will fail.
Attachment #8590628 - Flags: review?(james)
(Assignee)

Comment 4

3 years ago
Try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ad41307c0c01
Comment on attachment 8590624 [details] [diff] [review]
part 1 - Remove AnimationTimeline IDL tests from dom/animation/tests

Review of attachment 8590624 [details] [diff] [review]:
-----------------------------------------------------------------

I think Ms2ger should review things in imptest
Attachment #8590624 - Flags: review?(james) → review?(Ms2ger)
Attachment #8590627 - Flags: review?(bugs) → review+
Attachment #8590628 - Flags: review?(james) → review+
Comment on attachment 8590624 [details] [diff] [review]
part 1 - Remove AnimationTimeline IDL tests from dom/animation/tests

Review of attachment 8590624 [details] [diff] [review]:
-----------------------------------------------------------------

rs=me
Attachment #8590624 - Flags: review?(Ms2ger) → review+
(Assignee)

Comment 7

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c0766ab1022
https://hg.mozilla.org/integration/mozilla-inbound/rev/9296539ae800
https://hg.mozilla.org/integration/mozilla-inbound/rev/df60406c8e62
https://hg.mozilla.org/mozilla-central/rev/8c0766ab1022
https://hg.mozilla.org/mozilla-central/rev/9296539ae800
https://hg.mozilla.org/mozilla-central/rev/df60406c8e62
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox40: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
(Assignee)

Updated

3 years ago
Blocks: 1154615

Updated

3 years ago
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.