Skip to content

Commit

Permalink
Web Animations: Fix bugs in procedure to process a keyframes argument
Browse files Browse the repository at this point in the history
There were three minor bugs left in the implementation:

  - We threw on lists-in-custom-iterators instead of just ignoring them.
  - We returned all properties on the keyframe rather than just those
    defined on the keyframe itself (e.g. we would include prototype
    properties, against spec).
  - We didn't access the properties in ascending unicode order.

Bug: 827573
Change-Id: I213ae5b24e1f35d7f28d16625025122950a6ba88
  • Loading branch information
stephenmcgruer authored and chromium-wpt-export-bot committed Apr 11, 2018
1 parent 4b3342e commit 5ded871
Showing 1 changed file with 73 additions and 0 deletions.
Expand Up @@ -307,6 +307,20 @@
}, 'Keyframes are read from a custom iterator with where an offset is'
+ ' specified');

test(() => {
const test_error = { name: 'test' };
const bad_keyframe = { get left() { throw test_error; } };
assert_throws(test_error, () => {
new KeyframeEffect(null, createIterable([
{ done: false, value: { left: '100px' } },
{ done: false, value: bad_keyframe },
{ done: false, value: { left: '200px' } },
{ done: true },
]));
});
}, 'If a keyframe throws for an animatable property, that exception should be'
+ ' propagated');

test(() => {
assert_throws({ name: 'TypeError' }, () => {
new KeyframeEffect(null, createIterable([
Expand All @@ -319,6 +333,39 @@
}, 'Reading from a custom iterator that returns a non-object keyframe'
+ ' should throw');

// The following two tests are currently against spec, but instead match what
// we intend the spec to be; see https://github.com/w3c/csswg-drafts/issues/2533

test(() => {
const effect = new KeyframeEffect(null, createIterable([
{ done: false, value: { left: '100px' } },
{ done: false }, // No value member; keyframe is undefined.
{ done: false, value: { left: '200px' } },
{ done: true },
]));
assert_frame_lists_equal(effect.getKeyframes(), [
{ left: '100px', offset: null, computedOffset: 0, easing: 'linear', composite: null },
{ offset: null, computedOffset: 0.5, easing: 'linear', composite: null },
{ left: '200px', offset: null, computedOffset: 1, easing: 'linear', composite: null },
]);
}, 'An undefined keyframe returned from a custom iterator should be treated as a'
+ ' default keyframe');

test(() => {
const effect = new KeyframeEffect(null, createIterable([
{ done: false, value: { left: '100px' } },
{ done: false, value: null },
{ done: false, value: { left: '200px' } },
{ done: true },
]));
assert_frame_lists_equal(effect.getKeyframes(), [
{ left: '100px', offset: null, computedOffset: 0, easing: 'linear', composite: null },
{ offset: null, computedOffset: 0.5, easing: 'linear', composite: null },
{ left: '200px', offset: null, computedOffset: 1, easing: 'linear', composite: null },
]);
}, 'A null keyframe returned from a custom iterator should be treated as a'
+ ' default keyframe');

test(() => {
const effect = new KeyframeEffect(null, createIterable([
{ done: false, value: { left: ['100px', '200px'] } },
Expand All @@ -329,6 +376,32 @@
]);
}, 'A list of values returned from a custom iterator should be ignored');

// Test handling of invalid Symbol.iterator

test(() => {
const test_error = { name: 'test' };
const keyframe_obj = {
[Symbol.iterator]() {
throw test_error;
},
};
assert_throws(test_error, () => {
new KeyframeEffect(null, keyframe_obj);
});
}, 'Accessing a Symbol.iterator property that throws should rethrow');

test(() => {
const keyframe_obj = {
[Symbol.iterator]() {
return 42; // Not an object.
},
};
assert_throws({ name: 'TypeError' }, () => {
new KeyframeEffect(null, keyframe_obj);
});
}, 'A non-object returned from the Symbol.iterator property should cause a'
+ ' TypeError to be thrown');

test(() => {
const keyframe = {};
Object.defineProperty(keyframe, 'width', { value: '200px' });
Expand Down

0 comments on commit 5ded871

Please sign in to comment.