| Summary: | Canvas: DOM errors should be raised on null/invalid images | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | George Staikos <staikos> | ||||||
| Component: | Canvas | Assignee: | 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: |
|
||||||||
Comment on attachment 52679 [details]
Patch to throw the DOM errors
r=me
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
George you should have commit privileges so try to avoid the commit-bot I have not. Only based on the latest draft of the spec plus a testsuite. (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 It gives me 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIDOMCanvasRenderingContext2D.createPattern] (In reply to comment #6) > It gives me 0x80040111 (NS_ERROR_NOT_AVAILABLE) > [nsIDOMCanvasRenderingContext2D.createPattern] And drawImage? Same. Running through the tests: there are many failures, some much more obvious than this. Maybe someone should test a head rev gecko. Attachment 52679 [details] was posted by a committer and has review+, assigning to George Staikos for commit.
Created attachment 52706 [details]
Self Contained Test
For convenience, a self-contained version of the layout test included in the patch.
(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]. Unsure of the status of this patch. It's been in pending-commit for over a month. Updates? We have not had time to resolve the question Oliver has. (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. 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/.
This was fixed in revisions 71268 and 71798. |
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.