[webkit-reviews] review denied: [Bug 38929] Canvas: Ignore calls to drawImage() with non-finite parameters : [Attachment 59852] Proposed patch (style fixed)

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Andreas Kling
<andreas.kling at nokia.com>'s request for review:
Bug 38929: Canvas: Ignore calls to drawImage() with non-finite parameters
https://bugs.webkit.org/show_bug.cgi?id=38929

Attachment 59852: Proposed patch (style fixed)
https://bugs.webkit.org/attachment.cgi?id=59852&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
I read the section on drawImage in the WHATWG specification and I see nothing
there that says a non-finite coordinate in an argument results in silently
doing nothing. In some cases that may be correct, but in other cases it seems
clear it is wrong.

For example it seems to me that if sx is infinite and all other values are
finite then we should raise an INDEX_SIZE_ERR exception.

Am I missing something?

> +    if (!isfinite(dstRect.x()) || !isfinite(dstRect.y()) ||
!isfinite(dstRect.width()) || !isfinite(dstRect.height())
> +	   || !isfinite(srcRect.x()) || !isfinite(srcRect.y()) ||
!isfinite(srcRect.width()) || !isfinite(srcRect.height()))
> +	   return;

If you don't like all this on one line there are two obvious things you can do:


    1) Break it up into two separate if statements. There's no reason it has to
be all in one line.

    2) Add a function named something like isFinite or isAllFinite or some
better name that returns true only if all coordinates of a FloatRect finite.
This could be a free function or a member function of FloatRect. If that was
added, the code would be much easier to read.