Bug 37177 – Canvas: DOM errors should be raised on null/invalid images

• Component: Canvas
• Status: RESOLVED FIXED

[George Staikos]

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.

[Darin Adler]

Comment on attachment 52679 [details]
Patch to throw the DOM errors

r=me

[Oliver Hunt]

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]

George you should have commit privileges so try to avoid the commit-bot

[George Staikos]

I have not.  Only based on the latest draft of the spec plus a testsuite.

[Oliver Hunt]

(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]

It gives me 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIDOMCanvasRenderingContext2D.createPattern]

[Oliver Hunt]

(In reply to comment #6)
> It gives me 0x80040111 (NS_ERROR_NOT_AVAILABLE)
> [nsIDOMCanvasRenderingContext2D.createPattern]

And drawImage?

[George Staikos]

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)]

Attachment 52679 [details] was posted by a committer and has review+, assigning to George Staikos for commit.

[Daniel Bates]

Created attachment 52706 [details]
Self Contained Test

For convenience, a self-contained version of the layout test included in the patch.

[Daniel Bates]

(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)]

Unsure of the status of this patch.  It's been in pending-commit for over a month.  Updates?

[George Staikos]

We have not had time to resolve the question Oliver has.

[Andreas Kling]

(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 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]

This was fixed in revisions 71268 and 71798.