[webkit-reviews] review denied: [Bug 34301] Creating <animateMotion> elements via javascript do not execute : [Attachment 95375] Patch

[bugzilla-daemon at webkit.org]

Dirk Schulze <krit at webkit.org> has denied Rob Buis <rwlbuis at gmail.com>'s
request for review:
Bug 34301: Creating <animateMotion> elements via javascript do not execute
https://bugs.webkit.org/show_bug.cgi?id=34301

Attachment 95375: Patch
https://bugs.webkit.org/attachment.cgi?id=95375&action=review

------- Additional Comments from Dirk Schulze <krit at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=95375&action=review

Some snippets before I can give r+.

> Source/WebCore/ChangeLog:17
> +	   * CMakeLists.txt:
> +	   * CodeGenerators.pri:
> +	   * WebCore.gypi:

I wonder that you did not add it to Xcode!? Can you do that as well please?

> LayoutTests/svg/animations/script-tests/animate-mpath-insert.js:42
> +    passed = isCloseEnough(eval(name), value, error);
> +    if (passed) {
> +	   testPassed(name + " is almost " + value + " (within " + error +
")");
> +    } else {
> +	   testFailed(name + " is " + eval(name) + " but should be within " +
error + " of " + value);  
> +    }

We already have a function like passIfCloseEnough. Can you use this method
please? It is in the animation lib.

> LayoutTests/svg/animations/script-tests/animate-mpath-insert.js:52
> +    passIfCloseEnough("rootSVGElement.getBBox().x", 100, 20);
> +    passIfCloseEnough("rootSVGElement.getBBox().y", 250, 20);
> +}
> +
> +function endSample() {
> +    passIfCloseEnough("rootSVGElement.getBBox().x", 400, 20);
> +    passIfCloseEnough("rootSVGElement.getBBox().y", 250, 20);

20 is a high tolerance. Can you decrease it a bit?