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

Web Animations: Fix bug with Document::getAnimations #10281

Merged
merged 1 commit into from Apr 4, 2018

Conversation

Projects
None yet
5 participants
@chromium-wpt-export-bot
Collaborator

chromium-wpt-export-bot commented Apr 3, 2018

Per spec, we should exclude effects that do not target an element in
the called document:

https://drafts.csswg.org/web-animations-1/#dom-document-getanimations

Bug: 828424
Change-Id: I41405d82184b17c1185931e34735a5f946573844
Reviewed-on: https://chromium-review.googlesource.com/992812
Commit-Queue: Stephen McGruer smcgruer@chromium.org
Reviewed-by: Xida Chen xidachen@chromium.org
Cr-Commit-Position: refs/heads/master@{#548156}

@wpt-pr-bot

Already reviewed downstream.

@w3c-bots

This comment has been minimized.

w3c-bots commented Apr 3, 2018

Build PASSED

Started: 2018-04-04 23:00:27
Finished: 2018-04-04 23:09:07

View more information about this build on:

@birtles

This comment has been minimized.

Contributor

birtles commented Apr 4, 2018

As mentioned in #webanimations this change should not be necessary and might mask bugs in implementations.

createDiv() should register a cleanup function that runs at the end of the test and removes the div from the document and if the div is not in the document its animations shouldn't be returned by document.getAnimations()

@birtles

We need to work out if these changes are really necessary. From what I can tell, they are not and they could be harmful.

Web Animations: Fix bug with Document::getAnimations
Per spec, we should exclude effects that do not target an element in
the called document:

https://drafts.csswg.org/web-animations-1/#dom-document-getanimations

Bug: 828424
Change-Id: I41405d82184b17c1185931e34735a5f946573844
Reviewed-on: https://chromium-review.googlesource.com/992812
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548156}

Test was fixed to not make changes birtles@ was concerned about

@chromium-wpt-export-bot chromium-wpt-export-bot merged commit c148004 into master Apr 4, 2018

1 check passed

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

@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-cl-992812 branch Apr 4, 2018

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