Closed Bug 1630745 Opened 4 years ago Closed 4 years ago

[wpt-sync] Sync PR 23032 - Animations modified with setKeyframes must not mask !important

Categories

(Core :: CSS Transitions and Animations, task, P4)

Product:
Core
Component:
CSS Transitions and Animations
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla77
Tracking Flags:
Tracking Status
firefox77 --- fixed

People

(Reporter: mozilla.org, Unassigned)

References

(
URL
)

Details

(Whiteboard: [wptsync downstream])

Sync web-platform-tests PR 23032 into mozilla-central (this bug is closed when the sync is complete).

PR: https://github.com/web-platform-tests/wpt/pull/23032
Details from upstream follow.

Anders Hartvoll Ruud <andruud@chromium.org> wrote:

Animations modified with setKeyframes must not mask !important

CSS declarations that are !important have higher priority in the
cascade than animation effects. Unfortunately the information about
which declarations were and weren't important is lost once the
StyleCascade disappears. Specifically, it's not stored on the
ComputedStyle. This causes a problem (once again) for the base computed
style optimization, since we can't naively add animation effects on
top of the base anymore. We might be overwriting something in the base
that was important.

The previous attempt at fixing this (flag on ElementAnimations) doesn't
work properly. For example, it fails to detect the case where an
animation initially doesn't conflict with important declarations, but
then suddenly does via setKeyframes.

To solve this, this CL stores a bitset containing information about
important declarations alongside the base ComputedStyle on
ElementAnimations. When we're considering whether the base can be used
or not, we then check if there's any animation matching the set of
important declarations.

Persisting that many bits is slightly uncomfortable, but the only
viable alternative I see is disabling the optimization when any
important declaration exists in the base, which is probably a worse
option.

Sidenote: Initially I tried to always use the base, even when there
were conflicts with important declarations. The bitset of important
declarations is effectively a set of animations to be skipped, so we
should still be able to use the base if we just don't apply the
properties present in the set. However, it unfortunately didn't work
due to visited/unvisited colors being animated together
(crbug.com/1062217).

Bug: 552085
Change-Id: I39e2879af8a858ce1bd97eaa2ceb6e222591df79
Reviewed-on: https://chromium-review.googlesource.com/2152449
WPT-Export-Revision: 9eafdadb5e2877bd22f2f8f0126889d680019b78

Component: web-platform-tests → CSS Transitions and Animations
Product: Testing → Core

CI Results

Ran 13 Firefox configurations based on mozilla-central, and Firefox, Chrome, and Safari on GitHub CI

Total 60 tests

Status Summary

Firefox

PASS : 59
ERROR: 1

Chrome

OK : 1
PASS : 6
FAIL : 1

Safari

ERROR: 1

Links

Gecko CI (Treeherder)
GitHub PR Head
GitHub PR Base

Details

New Tests That Don't Pass

/css/css-animations/animation-important-001.html: ERROR (Chrome: OK, Safari: ERROR)

CI Results

Ran 13 Firefox configurations based on mozilla-central, and Firefox, Chrome, and Safari on GitHub CI

Total 60 tests

Status Summary

Firefox

PASS : 59
ERROR: 1

Chrome

OK : 1
PASS : 6
FAIL : 1

Safari

ERROR: 1

Links

Gecko CI (Treeherder)
GitHub PR Head
GitHub PR Base

Details

New Tests That Don't Pass

/css/css-animations/animation-important-001.html: ERROR (Chrome: OK, Safari: ERROR)

Pushed by wptsync@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/97b1855713ee
[wpt PR 23032] - Animations modified with setKeyframes must not mask !important, a=testonly
https://hg.mozilla.org/integration/autoland/rev/8ad29f64900b
[wpt PR 23032] - Update wpt metadata, a=testonly
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/8e727e285adb
[wpt PR 23032] - Animations modified with setKeyframes must not mask !important, a=testonly
https://hg.mozilla.org/integration/autoland/rev/6a7be38cb8f3
[wpt PR 23032] - Update wpt metadata, a=testonly

https://hg.mozilla.org/mozilla-central/rev/8e727e285adb
https://hg.mozilla.org/mozilla-central/rev/6a7be38cb8f3

Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox77: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla77
You need to log in before you can comment on or make changes to this bug.