[webkit-reviews] review granted: [Bug 178674] [Web Animations] Add basic timing and target properties : [Attachment 324567] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Oct 23 11:20:41 PDT 2017


Dean Jackson <dino at apple.com> has granted Antoine Quint <graouts at apple.com>'s
request for review:
Bug 178674: [Web Animations] Add basic timing and target properties
https://bugs.webkit.org/show_bug.cgi?id=178674

Attachment 324567: Patch

https://bugs.webkit.org/attachment.cgi?id=324567&action=review




--- Comment #3 from Dean Jackson <dino at apple.com> ---
Comment on attachment 324567
  --> https://bugs.webkit.org/attachment.cgi?id=324567
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=324567&action=review

> Source/WebCore/animation/AnimationEffect.cpp:41
> +AnimationEffect::~AnimationEffect()
> +{
> +}

Will this eventually have code? If not, put it in the header.

> Source/WebCore/animation/WebAnimation.h:55
> +    std::optional<double> m_startTime;

As smfr probably said, use Seconds or something.

> Source/WebCore/animation/WebAnimation.idl:35
>  ] interface WebAnimation {
> +		attribute AnimationEffect? effect;
>      readonly attribute AnimationTimeline? timeline;
> +		attribute double? startTime;
>  };

Is this weird spacing standard?

> Source/WebCore/bindings/js/JSAnimationEffectCustom.cpp:42
> +JSValue toJSNewlyCreated(ExecState*, JSDOMGlobalObject* globalObject,
Ref<AnimationEffect>&& value)
> +{
> +    if (value->isKeyframeEffect())
> +	   return createWrapper<KeyframeEffect>(globalObject, WTFMove(value));
> +    return createWrapper<AnimationEffect>(globalObject, WTFMove(value));
> +}

Wish this code was automatic. Paging Rabbi Weinig.

> LayoutTests/ChangeLog:23
> +	   * webanimations/animation-effect-expected.txt: Added.
> +	   * webanimations/animation-effect-timing-expected.txt: Added.
> +	   * webanimations/animation-effect-timing.html: Added.
> +	   * webanimations/animation-effect.html: Added.
> +	   * webanimations/animation-interface-effect-property-expected.txt:
Added.
> +	   * webanimations/animation-interface-effect-property.html: Added.
> +	   *
webanimations/animation-interface-start-time-property-expected.txt: Added.
> +	   * webanimations/animation-interface-start-time-property.html: Added.
> +	   * webanimations/keyframe-effect-expected.txt: Added.
> +	   *
webanimations/keyframe-effect-interface-timing-duration-expected.txt: Added.
> +	   * webanimations/keyframe-effect-interface-timing-duration.html:
Added.
> +	   * webanimations/keyframe-effect.html: Added.
> +

All these tests should be written for Web Platform Tests.

Put them in http/wpt/ for now, with the goal of submitting them to WPT. Use the
WPT API for testing.


More information about the webkit-reviews mailing list