Bug 37177

Summary: Canvas: DOM errors should be raised on null/invalid images
Product: WebKit Reporter: George Staikos <staikos>
Component: CanvasAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: dbates, eric, heldercorreia, jhanssen, kling, mdelaney7, oliver
Priority: P2 Keywords: HTML5
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch to throw the DOM errors
none
Self Contained Test none

George Staikos
Reported 2010-04-06 16:11:48 PDT
Created attachment 52679 [details] Patch to throw the DOM errors We should throw the correct DOM exception according to the canvas spec if createPattern or drawImage are called with an invalid image argument.
Attachments
Patch to throw the DOM errors (6.22 KB, patch)
2010-04-06 16:11 PDT, George Staikos 		no flags
DetailsFormatted DiffDiff
Self Contained Test (1.73 KB, text/html)
2010-04-06 23:49 PDT, Daniel Bates 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Darin Adler
Comment 1 2010-04-06 17:55:56 PDT
Comment on attachment 52679 [details] Patch to throw the DOM errors r=me
Oliver Hunt
Comment 2 2010-04-06 17:58:23 PDT
Comment on attachment 52679 [details] Patch to throw the DOM errors Have you tested compatibility with firefox on this? we've had problems in the past were we were throwing exceptions and ffx wasn't
Oliver Hunt
Comment 3 2010-04-06 17:58:55 PDT
George you should have commit privileges so try to avoid the commit-bot
George Staikos
Comment 4 2010-04-06 18:04:50 PDT
I have not. Only based on the latest draft of the spec plus a testsuite.
Oliver Hunt
Comment 5 2010-04-06 18:06:11 PDT
(In reply to comment #4) > I have not. Only based on the latest draft of the spec plus a testsuite. Please compare behaviour to firefox -- assuming firefox behaves the same this is fine, otherwise it may be time to email the whatwg
George Staikos
Comment 6 2010-04-06 18:11:13 PDT
It gives me 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIDOMCanvasRenderingContext2D.createPattern]
Oliver Hunt
Comment 7 2010-04-06 18:11:53 PDT
(In reply to comment #6) > It gives me 0x80040111 (NS_ERROR_NOT_AVAILABLE) > [nsIDOMCanvasRenderingContext2D.createPattern] And drawImage?
George Staikos
Comment 8 2010-04-06 18:17:27 PDT
Same. Running through the tests: there are many failures, some much more obvious than this. Maybe someone should test a head rev gecko.
Eric Seidel (no email)
Comment 9 2010-04-06 23:46:10 PDT
Attachment 52679 [details] was posted by a committer and has review+, assigning to George Staikos for commit.
Daniel Bates
Comment 10 2010-04-06 23:49:07 PDT
Created attachment 52706 [details] Self Contained Test For convenience, a self-contained version of the layout test included in the patch.
Daniel Bates
Comment 11 2010-04-06 23:50:54 PDT
(In reply to comment #7) > (In reply to comment #6) > > It gives me 0x80040111 (NS_ERROR_NOT_AVAILABLE) > > [nsIDOMCanvasRenderingContext2D.createPattern] > > And drawImage? On Mac Firefox 3.6.3, I get: Threw exception [Exception... "Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIDOMCanvasRenderingContext2D.drawImage]" nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE)" location: "JS frame :: http://trac.webkit.org/export/57195/trunk/LayoutTests/fast/js/resources/js-test-pre.js :: shouldThrow :: line 226" data: no]. For completeness, here are the results I see when I run the test <https://bugs.webkit.org/attachment.cgi?id=52706> in Firefox: FAIL context.createPattern(null, 'no-repeat') should throw Error: TYPE_MISMATCH_ERR: DOM Exception 17. Threw exception [Exception... "Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIDOMCanvasRenderingContext2D.createPattern]" nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE)" location: "JS frame :: http://trac.webkit.org/export/57195/trunk/LayoutTests/fast/js/resources/js-test-pre.js :: shouldThrow :: line 226" data: no]. FAIL context.putImageData(null, 0, 0) should throw Error: TYPE_MISMATCH_ERR: DOM Exception 17. Threw exception [Exception... "An invalid or illegal string was specified" code: "12" nsresult: "0x8053000c (NS_ERROR_DOM_SYNTAX_ERR)" location: "http://trac.webkit.org/export/57195/trunk/LayoutTests/fast/js/resources/js-test-pre.js Line: 226"]. FAIL context.drawImage(null, 0, 0) should throw Error: TYPE_MISMATCH_ERR: DOM Exception 17. Threw exception [Exception... "Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIDOMCanvasRenderingContext2D.drawImage]" nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE)" location: "JS frame :: http://trac.webkit.org/export/57195/trunk/LayoutTests/fast/js/resources/js-test-pre.js :: shouldThrow :: line 226" data: no].
Eric Seidel (no email)
Comment 12 2010-05-17 00:40:20 PDT
Unsure of the status of this patch. It's been in pending-commit for over a month. Updates?
George Staikos
Comment 13 2010-05-22 23:52:51 PDT
We have not had time to resolve the question Oliver has.
Andreas Kling
Comment 14 2010-08-20 00:51:42 PDT
(In reply to comment #2) > (From update of attachment 52679 [details]) > Have you tested compatibility with firefox on this? we've had problems in the > past were we were throwing exceptions and ffx wasn't We're already throwing TypeError. Changing it to TYPE_MISMATCH_ERR seems harmless to me.
Andreas Kling
Comment 15 2010-09-28 06:17:46 PDT
Comment on attachment 52679 [details] Patch to throw the DOM errors This patch needs some love. The layout test can be removed in favor of unskipping the relevant test(s) in LayoutTests/canvas/philip/.
Andreas Kling
Comment 16 2010-11-20 22:13:23 PST
This was fixed in revisions 71268 and 71798.
Note You need to log in before you can comment on or make changes to this bug.