Right now we throw SyntaxError, afaict. Per https://www.w3.org/Bugs/Public/show_bug.cgi?id=25672 this should throw InvalidStateError.

Walter, this is in CanvasRenderingContext2D::PutImageData_explicit method, but let's also see if some of the callers are explicitly expecting the "syntax" error...

I believe this is all that needed to be changed. I did a grep and a dxr search and it appears that nothing calls these putimage functions (that I can tell): http://dxr.mozilla.org/mozilla-central/search?q=PutImageData&case=true I also did a try build to see if anything non-obvious broke: https://tbpl.mozilla.org/?tree=Try&rev=bc761e7ee351 I don't think the two orange tests are caused by this (but I may be wrong).

Comment on attachment 8433458 [details] [diff] [review] canvas_2d_patch Per spec it actually looks to me like w==0/h==0 should be a no-op instead of throwing. At least at first glance.

I think it's still an error, although the wording is a little tough: http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#dom-imagedata-width says that "At least one pixel's worth of image data must be returned." Combined with: "[the] width attribute is set to the number of pixels per row in the image data, their height attribute is set to the number of rows in the image data". So I think it's still an error because PutImageData() shouldn't receive an ImageData object with w==0 or h==0 because then it would have less than 1 pixel of data.

That's for getImageData. But I guess createImageData also doesn't allow 0x0 size. So it doesn't matter too much what the w==0/h==0 case does, since that code is never hit anyway.

Comment on attachment 8433458 [details] [diff] [review] canvas_2d_patch Review of attachment 8433458 [details] [diff] [review]: ----------------------------------------------------------------- I like it! Let's get one of the graphics peers to give it the official review.

Comment on attachment 8433458 [details] [diff] [review] canvas_2d_patch Review of attachment 8433458 [details] [diff] [review]: ----------------------------------------------------------------- Ah, I see, neutering an ArrayBuffer does set its length to 0 here: http://dxr.mozilla.org/mozilla-central/source/js/src/vm/ArrayBufferObject.cpp#349 and the current canvas spec does agree that this should be an InvalidStateError http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#dom-context-2d-putimagedata

Only updated patch information. Was missing some author information.

Now it should be correct...