Bug 28329

Summary: HTML5 canvas's createImageData should raise the NOT_SUPPORTED_ERR exception when either of the arguments are not finite.
Product: WebKit Reporter: Benjamin Meyer <ben>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, krit
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
URL: http://philip.html5.org/tests/canvas/suite/tests/2d.imageData.create.nonfinite.html
Attachments:
Description Flags
Patch to solve the issue
darin: review-
tests + tests
none
patch + tests
none
patch + tests none

Description Benjamin Meyer 2009-08-14 16:11:35 PDT
According to http://www.whatwg.org/specs/web-apps/current-work/#pixel-manipulation:

            "If any of the arguments to createImageData() or getImageData() are
             infinite or NaN, or if the createImageData()  method is invoked with
             only one argument but that argument is null, the method must instead
             raise a NOT_SUPPORTED_ERR  exception."

Test page: http://philip.html5.org/tests/canvas/suite/tests/2d.imageData.create.nonfinite.html
Comment 1 Benjamin Meyer 2009-08-14 16:13:29 PDT
Created attachment 34878 [details]
Patch to solve the issue
Comment 2 Benjamin Meyer 2009-08-14 16:15:35 PDT
I have contacted the author of the canvas tests to determine the licensing of the generated tests.  If possible it would be nice to integrate them into our pixel tests and I started work in that direction, but before they can go into our repo the licensing needs to be determined.
Comment 3 Darin Adler 2009-08-14 17:08:30 PDT
Comment on attachment 34878 [details]
Patch to solve the issue

Fix looks great.

But we also require a regression test for fixes like this one. Please resubmit a patch that includes a regression test.

It should be straightforward to come up for a test for this. We don't need to use the canvas tests you referred to in the bug earlier. If you look in the LayoutTests/fast/canvas directory you'll see many other similar tests which could give you some ideas how to make a test that covers this fix.
Comment 4 Eric Seidel (no email) 2009-08-14 17:17:38 PDT
philip (the author of the tests) hangs out in #webkit.  You should be able to ask him there too. :)
Comment 5 Benjamin Meyer 2009-08-17 10:02:28 PDT
Created attachment 34972 [details]
tests + tests

Same patch as before, but now with a layout test
Comment 6 Benjamin Meyer 2009-08-17 10:08:33 PDT
Comment on attachment 34972 [details]
tests + tests

dupe
Comment 7 Benjamin Meyer 2009-08-17 10:09:40 PDT
Created attachment 34973 [details]
patch + tests
Comment 8 Benjamin Meyer 2009-08-17 10:11:53 PDT
Created attachment 34974 [details]
patch + tests
Comment 9 Darin Adler 2009-08-17 13:30:22 PDT
Comment on attachment 34974 [details]
patch + tests

It'd be nice to have some other test cases too, such as 0, positive numbers, negative numbers, and non-numeric values.

r=me
Comment 10 Eric Seidel (no email) 2009-08-17 17:06:02 PDT
Comment on attachment 34974 [details]
patch + tests

Clearing flags on attachment: 34974

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/canvas/canvas-2d-imageData-create-nonfinite-expected.txt
	A	LayoutTests/fast/canvas/canvas-2d-imageData-create-nonfinite.html
	A	LayoutTests/fast/canvas/resources/canvas-2d-imageData-create-nonfinite.js
	M	WebCore/ChangeLog
	M	WebCore/html/canvas/CanvasRenderingContext2D.cpp
	M	WebCore/html/canvas/CanvasRenderingContext2D.h
	M	WebCore/html/canvas/CanvasRenderingContext2D.idl
Committed r47398
	M	WebCore/ChangeLog
	M	WebCore/html/canvas/CanvasRenderingContext2D.cpp
	M	WebCore/html/canvas/CanvasRenderingContext2D.h
	M	WebCore/html/canvas/CanvasRenderingContext2D.idl
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/canvas/resources/canvas-2d-imageData-create-nonfinite.js
	A	LayoutTests/fast/canvas/canvas-2d-imageData-create-nonfinite-expected.txt
	A	LayoutTests/fast/canvas/canvas-2d-imageData-create-nonfinite.html
r47398 = bfd04504c6b001c7b8907e1d3a93ce7d42cd1e6b (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/47398
Comment 11 Eric Seidel (no email) 2009-08-17 17:06:08 PDT
All reviewed patches have been landed.  Closing bug.