Commit e1a347fb authored by haozhe's avatar haozhe Committed by Commit Bot

Order animations of same class by tree-order of their owning elements

Currently, getAnimation sort the result by animation class and creation
time.

Sort composite order for animations of the same animation class by tree
order of their owning element according to spec.
(https://drafts.csswg.org/web-animations/#dom-document-getanimations).

This patch align the implementation with the spec.

Bug: 993365

Change-Id: I8478b2985c0ab473db33c176fc69516ea0222ee1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2008325
Commit-Queue: Hao Sheng <haozhes@chromium.org>
Reviewed-by: default avatarKevin Ellis <kevers@chromium.org>
Reviewed-by: default avatarRobert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735966}
parent 15273e85
...@@ -68,6 +68,8 @@ namespace blink { ...@@ -68,6 +68,8 @@ namespace blink {
namespace { namespace {
enum PseudoPriority { kMarker, kBefore, kOther, kAfter };
unsigned NextSequenceNumber() { unsigned NextSequenceNumber() {
static unsigned next = 0; static unsigned next = 0;
return ++next; return ++next;
...@@ -93,6 +95,16 @@ double Min(base::Optional<double> a, double b) { ...@@ -93,6 +95,16 @@ double Min(base::Optional<double> a, double b) {
return b; return b;
} }
PseudoPriority ConvertStringtoPriority(const String& pseudo) {
if (pseudo == "::marker")
return PseudoPriority::kMarker;
if (pseudo == "::before")
return PseudoPriority::kBefore;
if (pseudo == "::after")
return PseudoPriority::kAfter;
return PseudoPriority::kOther;
}
Animation::AnimationClassPriority AnimationPriority( Animation::AnimationClassPriority AnimationPriority(
const Animation* animation) { const Animation* animation) {
// According to the spec: // According to the spec:
...@@ -444,6 +456,68 @@ bool Animation::HasLowerCompositeOrdering(const Animation* animation1, ...@@ -444,6 +456,68 @@ bool Animation::HasLowerCompositeOrdering(const Animation* animation1,
AnimationClassPriority priority2 = AnimationPriority(animation2); AnimationClassPriority priority2 = AnimationPriority(animation2);
if (priority1 != priority2) if (priority1 != priority2)
return priority1 < priority2; return priority1 < priority2;
// If the the animation class is CssAnimation or CssTransition, then first
// compare the owning element of animation1 and animation2, sort two of them
// by tree order of their conrresponding owning element
// The specs:
// https://drafts.csswg.org/css-animations-2/#animation-composite-order
// https://drafts.csswg.org/css-transitions-2/#animation-composite-order
if (priority1 != kDefaultPriority && animation1->effect() &&
animation2->effect()) {
// TODO(crbug.com/1043778): Implement and use OwningElement on CSSAnimation
// and CSSTransition.
auto* effect1 = DynamicTo<KeyframeEffect>(animation1->effect());
auto* effect2 = DynamicTo<KeyframeEffect>(animation2->effect());
Element* target1 = effect1->target();
Element* target2 = effect2->target();
if (*target1 != *target2) {
return target1->compareDocumentPosition(target2) &
Node::kDocumentPositionFollowing;
}
// A pseudo-element has a higher composite ordering than its originating
// element, but lower than the originating element's children. Two
// pseudo-elements sharing the same originating element are sorted as
// follows:
// ::marker
// ::before
// ::other
// ::after
// The "::other" category is a catch-all for any pseudo-element that does
// not match another label. This category is not currently covered in the
// spec.
// TODO: revisit when the spec is clarified
// (https://github.com/w3c/csswg-drafts/issues/4502).
const String& pseudo1 =
const_cast<KeyframeEffect*>(effect1)->pseudoElement();
const String& pseudo2 =
const_cast<KeyframeEffect*>(effect2)->pseudoElement();
if (!pseudo1.IsEmpty() && !pseudo2.IsEmpty()) {
PseudoPriority priority1 = ConvertStringtoPriority(pseudo1);
PseudoPriority priority2 = ConvertStringtoPriority(pseudo2);
// In this case, we are comparing the SequenceNumber for now.
// TODO(crbug.com/1045835): compare them via Animation Name Property
if (priority1 == priority2)
return animation1->SequenceNumber() < animation2->SequenceNumber();
return priority1 < priority2;
}
// If one is pseudo element and the other one is not, then we compare the
// hosting element and the owning element.
if (!pseudo1.IsEmpty())
return false;
if (!pseudo2.IsEmpty())
return true;
// TODO: Sort animation1 and animation2 based on their position in the
// computed value of "animation-name" property
}
// If the anmiations are not-CSS web animation or both animations have the
// same owning element then just compare them via generation time/ sequence
// number
return animation1->SequenceNumber() < animation2->SequenceNumber(); return animation1->SequenceNumber() < animation2->SequenceNumber();
} }
......
This is a testharness.js-based test. This is a testharness.js-based test.
PASS getAnimations for non-animated content PASS getAnimations for non-animated content
PASS getAnimations for CSS Animations PASS getAnimations for CSS Animations
FAIL Order of CSS Animations - within an element unaffected by start time assert_equals: Order of first animation returned expected "animBottom" but got "animLeft"
PASS Order of CSS Animations - within an element PASS Order of CSS Animations - within an element
FAIL Order of CSS Animations - across elements assert_equals: Order of second animation returned after tree surgery expected Element node <div style="animation: animLeft 100s"></div> but got Element node <div style="animation: animLeft 100s"></div> PASS Order of CSS Animations - across elements
PASS Order of CSS Animations - across and within elements PASS Order of CSS Animations - across and within elements
FAIL Order of CSS Animations - markup-bound vs free animations assert_equals: getAnimations returns markup-bound and free animations expected 2 but got 1 FAIL Order of CSS Animations - markup-bound vs free animations assert_equals: getAnimations returns markup-bound and free animations expected 2 but got 1
FAIL Order of CSS Animations - free animations assert_equals: getAnimations returns free animations expected 2 but got 0 FAIL Order of CSS Animations - free animations assert_equals: getAnimations returns free animations expected 2 but got 0
...@@ -12,6 +13,6 @@ PASS Finished but not filling CSS Animations are not returned ...@@ -12,6 +13,6 @@ PASS Finished but not filling CSS Animations are not returned
PASS Yet-to-start CSS Animations are returned PASS Yet-to-start CSS Animations are returned
PASS CSS Animations canceled via the API are not returned PASS CSS Animations canceled via the API are not returned
PASS CSS Animations canceled and restarted via the API are returned PASS CSS Animations canceled and restarted via the API are returned
FAIL CSS Animations targetting (pseudo-)elements should have correct order after sorting assert_equals: Animation #3 has expected target expected Element node <div id="parent" style="animation: animBottom 100s"><div ... but got Element node <div style="animation: animBottom 100s"></div> PASS CSS Animations targetting (pseudo-)elements should have correct order after sorting
Harness: the test ran to completion. Harness: the test ran to completion.
...@@ -48,6 +48,24 @@ test(t => { ...@@ -48,6 +48,24 @@ test(t => {
'getAnimations returns no running CSS Animations'); 'getAnimations returns no running CSS Animations');
}, 'getAnimations for CSS Animations'); }, 'getAnimations for CSS Animations');
test(t => {
const div = addDiv(t);
const animation1 = 'animLeft 100s'
const animation2 = 'animBottom 100s'
div.style.animation = animation1;
const animations1 = document.getAnimations();
assert_equals(animations1.length, 1,
'getAnimations returns all running CSS Animations');
div.style.animation = animation2 + ', ' + animation1;
const animations = document.getAnimations();
assert_equals(animations.length, 2,
'getAnimations returns all running CSS Animations');
assert_equals(animations[0].animationName, 'animBottom',
'Order of first animation returned');
assert_equals(animations[1].animationName, 'animLeft',
'Order of second animation returned');
}, 'Order of CSS Animations - within an element unaffected by start time');
test(t => { test(t => {
const div = addDiv(t); const div = addDiv(t);
div.style.animation = 'animLeft 100s, animTop 100s, animRight 100s, ' + div.style.animation = 'animLeft 100s, animTop 100s, animRight 100s, ' +
......
...@@ -19,7 +19,7 @@ FAIL getAnimations for CSS Animations follows animation-name order assert_equals ...@@ -19,7 +19,7 @@ FAIL getAnimations for CSS Animations follows animation-name order assert_equals
PASS { subtree: false } on a leaf element returns the element's animations and ignore pseudo-elements PASS { subtree: false } on a leaf element returns the element's animations and ignore pseudo-elements
PASS { subtree: true } on a leaf element returns the element's animations and its pseudo-elements' animations PASS { subtree: true } on a leaf element returns the element's animations and its pseudo-elements' animations
PASS { subtree: false } on an element with a child returns only the element's animations PASS { subtree: false } on an element with a child returns only the element's animations
FAIL { subtree: true } on an element with a child returns animations from the element, its pseudo-elements, its child and its child pseudo-elements assert_equals: The animation targeting the ::after pesudo-element should be returned third expected (string) "::after" but got (object) null PASS { subtree: true } on an element with a child returns animations from the element, its pseudo-elements, its child and its child pseudo-elements
PASS { subtree: true } on an element with many descendants returns animations from all the descendants PASS { subtree: true } on an element with many descendants returns animations from all the descendants
Harness: the test ran to completion. Harness: the test ran to completion.
...@@ -303,8 +303,8 @@ test(t => { ...@@ -303,8 +303,8 @@ test(t => {
'#target::before': 'animation: anim1 10s;' }); '#target::before': 'animation: anim1 10s;' });
const target = addDiv(t, { 'id': 'target' }); const target = addDiv(t, { 'id': 'target' });
target.style.animation = 'anim1 100s'; target.style.animation = 'anim1 100s';
const animations = target.getAnimations({ subtree: false }); const animations = target.getAnimations({ subtree: false });
assert_equals(animations.length, 1, assert_equals(animations.length, 1,
'Should find only the element'); 'Should find only the element');
assert_equals(animations[0].effect.target, target, assert_equals(animations[0].effect.target, target,
...@@ -317,8 +317,8 @@ test(t => { ...@@ -317,8 +317,8 @@ test(t => {
'#target::before': 'animation: anim1 10s;' }); '#target::before': 'animation: anim1 10s;' });
const target = addDiv(t, { 'id': 'target' }); const target = addDiv(t, { 'id': 'target' });
target.style.animation = 'anim1 100s'; target.style.animation = 'anim1 100s';
const animations = target.getAnimations({ subtree: true }); const animations = target.getAnimations({ subtree: true });
assert_equals(animations.length, 3, assert_equals(animations.length, 3,
'getAnimations({ subtree: true }) ' + 'getAnimations({ subtree: true }) ' +
'should return animations on pseudo-elements'); 'should return animations on pseudo-elements');
......
This is a testharness.js-based test. This is a testharness.js-based test.
PASS Same events are ordered by elements PASS Same events are ordered by elements
FAIL Same events on pseudo-elements follow the prescribed order assert_equals: Event #3 targets should match expected Element node <div style="animation: anim 100s" id="parent-div"><div st... but got Element node <div style="animation: anim 100s"></div> PASS Same events on pseudo-elements follow the prescribed order
FAIL Start and iteration events are ordered by time assert_equals: Event #1 types should match (expected: animationiteration, animationstart, actual: animationstart, animationiteration) expected "animationiteration" but got "animationstart" FAIL Start and iteration events are ordered by time assert_equals: Event #1 types should match (expected: animationiteration, animationstart, actual: animationstart, animationiteration) expected "animationiteration" but got "animationstart"
FAIL Iteration and end events are ordered by time assert_equals: Event #1 types should match (expected: animationiteration, animationend, actual: animationend, animationiteration) expected "animationiteration" but got "animationend" FAIL Iteration and end events are ordered by time assert_equals: Event #1 types should match (expected: animationiteration, animationend, actual: animationend, animationiteration) expected "animationiteration" but got "animationend"
FAIL Start and end events are sorted correctly when fired simultaneously assert_equals: Event #1 targets should match expected Element node <div style="animation: anim 100s 2"></div> but got Element node <div style="animation: anim 100s 100s"></div> FAIL Start and end events are sorted correctly when fired simultaneously assert_equals: Event #1 targets should match expected Element node <div style="animation: anim 100s 2"></div> but got Element node <div style="animation: anim 100s 100s"></div>
......
This is a testharness.js-based test.
PASS getAnimations for non-animated content
PASS getAnimations for CSS Transitions
FAIL CSS Transitions targetting (pseudo-)elements should have correct order after sorting assert_equals: Transition #3 has expected target expected Element node <div style="display: list-item; left: 100px; transition: ... but got Element node <div style="left: 100px; transition: left 100s ease 0s;">...
PASS Transitions are not returned after they have finished
Harness: the test ran to completion.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment