[webkit-reviews] review denied: [Bug 20435] Canvas missing exceptioncode for gradients : [Attachment 22897] drawImage() update

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 20 13:06:27 PDT 2008


Oliver Hunt <oliver at apple.com> has denied Dirk Schulze <vbs85 at gmx.de>'s request
for review:
Bug 20435: Canvas missing exceptioncode for gradients
https://bugs.webkit.org/show_bug.cgi?id=20435

Attachment 22897: drawImage() update
https://bugs.webkit.org/attachment.cgi?id=22897&action=edit

------- Additional Comments from Oliver Hunt <oliver at apple.com>
fixing draw image should go in a separate bug, however issues i spotted, that
make me r- this in its current form:
You changed FloatRect::contains -- i'm not entirely happy with this as
FloatRect as the contains semantics would be subtly different from
IntRect::contains.  A better approach would to normalise the rect before doing
the contains check.

The code also now ends up checking for !srcRect.width(), etc multiple times
throwing an exception in one case and silently returning in another.

Your testcase also isn't particularly great as you  don't actually test that
you can specify a negative sized source rect (0-width/height for some of the
components result in an exception), and you don't test the rendering behaviour
of a negative source.  This last part is especially critical as you are adding
new functionality, so ou need to confirm that the new functionality actually
works.


More information about the webkit-reviews mailing list