Re: Processing model for the first argument of Animatable.animate is not well defined

Thanks Boris!

(2014/04/17 15:34), Boris Zbarsky wrote:
> Thanks, that helps.  Some more comments:
>
> 1) In section 5.9.1 step 4, it's not clear to me what it means for
> "effect" to be "an EffectCallback object".  Since EffectCallback isn't
> an interface, there is no such concept defined a priori....
>
> What you probably want to test there is IsCallable.
>
> Similarly, it's not clear to me what "effect is a Keyframe object or a
> sequence of Keyframe objects" means.  For example, Keyframe is a
> dictionary, which means it can be initialized with any object, so it
> doesn't make sense to talk about "a Keyframe object".

Yes, this is all very redundant. The main difference is this section 
also constructs a KeyframeEffect from a sequence of keyframes. I've 
added that to 5.9.4. Please let me know if this makes sense.

> What you might want to do is test
> http://people.mozilla.org/~jorendorff/es6-draft.html#sec-array.isarray
> and if so treat as a sequence, else as a Keyframe.  And if WebIDL
> switches to sequence meaning iterable, we can loosen the restriction
> here to allow any iterable, not just an Array.

Ok, done.

> Or should this entire step simply be referencing section 5.9.4, perhaps?
>   It looks like most of these issues are not present there.

Hopefully fixed now.

> Also, a nit.  "Assign animation.effect to effect." probably means to say
> "Set animation.effect to effect."

Fixed.

> 2) It's not clear to me whether the text in the table in section 5.9.1
> is normative or informative.  If the former, it seems like it should
> just reference 5.9.4 instead of repeating it?  If the latter, that would
> be good to flag somewhere.

I've removed the effect handling from here. I think the rest is still 
needed.

> 3) In section 5.14.4, the definition is OK for now, but once sequence in
> WebIDL means iterable we can just treat iterable as sequence here and
> otherwise treat the object as Keyframe.

For now I've just made it use Array.isArray for now as you suggested 
above. I suppose that doesn't work for platform objects that support 
indexed properties. Is that ok?

> 4) In 5.9.4, "effect" can't be undefined, since it's already been passed
> through an "object?" argument, so it's an object or null.

Fixed.

> 5) In 5.9.4, I don't think we should throw if a platform object that
> doesn't implement AnimationEffect is passed in.  I would replace that
> step of the if/then cascade with just:  "If effect is a platform object
> that implements the AnimationEffect interface, return the IDL value that
> represents a reference to that platform object".

Ok. I think I don't quite understand how that works but I've updated as 
you suggest.

> 6) In 5.9.4, when converting to a callback function type, we should say
> which type we convert to (EffectCallback in this case).

Fixed.

> 7) We shouldn't special-case Date and RegExp objects here in the next to
> last step of 5.9.4; those are valid things to pass to a dictionary
> argument (not that anyone really ever will, but...).  This also means
> the last step can go away, since we'll always convert to a
> sequence-or-Keyframe.

Fixed (but now we pass it on to the constructor for KeyframeEffect).

I suppose it's a bit weird that we describe the processing for effect 
along with the Animation ctor and reference from elsewhere, and we 
describe the processing for keyframes along with the 
KeyframeEffect.setFrames and reference from elsewhere (including ctor). 
I'll try to line those up.

Thanks Boris!

Brian


PS- There seems to be a lag for edits to show up on dev.w3.org in which 
case 
https://dvcs.w3.org/hg/FXTF/raw-file/default/web-animations/index.html 
should work.

Received on Thursday, 17 April 2014 07:46:23 UTC