At least one problem here: when canvas.width=0 is called we don't actually set the width to 0 ... see http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/nsHTMLCanvasElement.cpp#121

If I change the clamp from <= to < we crash in Cairo while trying to get an inputstream from the context.

The check was added by bug 291216.

Here is a patch and test.

Comment on attachment 527714 [details] [diff] [review] patch to fix the issue Thanks for the patch Benjamin. Joe, can you review this? Not sure who reviews canvas stuff these days ...

Comment on attachment 527714 [details] [diff] [review] patch to fix the issue Review of attachment 527714 [details] [diff] [review]: ::: content/html/content/src/nsHTMLCanvasElement.cpp @@ +124,3 @@ size.width = DEFAULT_CANVAS_WIDTH; + if (size.height < 0) + size.height = DEFAULT_CANVAS_HEIGHT; I think I would rather GetWidthHeightInternal didn't do any frobbing of the returned data; if users want to get the raw data, they should be ready to handle silly results. (This would also imply changing GetWidthHeight to contain <= rather than ==.) @@ +360,5 @@ + nsIntSize size = GetWidthHeightInternal(); + if (size.height == 0 || size.width == 0) { + aDataURL = NS_LITERAL_STRING("data:,"); + return NS_OK; + } Similarly, here we'd change this to <= 0.

This addresses review comments. However, it does not change "width == 0" to "width <= 0" because I don't think a negative value can get past ParseNonNegativeIntValue in ParseAttribute(). Correct me if I'm wrong.

Comment on attachment 530456 [details] [diff] [review] fix the issue 's fine with me! :)

I don't think we should do this until we fix bug 421865.

This simple patch (requiring the the patch to 421865) fixes the issue.

Comment on attachment 531507 [details] [diff] [review] fix based on 421865 Review of attachment 531507 [details] [diff] [review]: ----------------------------------------------------------------- Love it.

Pushed: http://hg.mozilla.org/mozilla-central/rev/951de6cc1fdb

Thanks for fixing this Benjamin.

Verified fixed on: -> Linux: Mozilla/5.0 (X11; Linux i686; rv:6.0) Gecko/20100101 Firefox/6.0 -> Mac: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:6.0) Gecko/20100101 Firefox/6.0 -> Windows: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0) Gecko/20100101 Firefox/6.0 The test toDataURL on w3c-test.org (Comment 10) is PASS, as well as the test at the link: http://samples.msdn.microsoft.com/ietestcenter/html5/canvas_harness.htm?url=canvas-canvasElement-001 Also the depending bug 421865 has been fixed

Thanks for the verification, Andrei.