Closed
Bug 649618
Opened 13 years ago
Closed 13 years ago
If the canvas has a horizontal dimension of zero, then toDataURL() must return the string "data:,".
Categories
(Core :: Graphics: Canvas2D, defect)
Core
Graphics: Canvas2D
Tracking
()
VERIFIED
FIXED
mozilla6
People
(Reporter: khuey, Assigned: Benjamin)
References
()
Details
(Whiteboard: ietestcenter)
Attachments
(1 file, 2 obsolete files)
1.11 KB,
patch
|
joe
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Comment 1•13 years ago
|
||
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
Updated•13 years ago
|
Version: unspecified → Trunk
Reporter | ||
Comment 2•13 years ago
|
||
If I change the clamp from <= to < we crash in Cairo while trying to get an inputstream from the context.
Comment 3•13 years ago
|
||
The check was added by bug 291216.
Reporter | ||
Comment 5•13 years ago
|
||
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 ...
Attachment #527714 -
Flags: review? → review?(joe)
Comment 6•13 years ago
|
||
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.
Attachment #527714 -
Flags: review?(joe) → review-
Assignee | ||
Comment 7•13 years ago
|
||
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.
Attachment #527714 -
Attachment is obsolete: true
Attachment #530456 -
Flags: review?
Assignee | ||
Updated•13 years ago
|
Attachment #530456 -
Flags: review? → review?(joe)
Comment 8•13 years ago
|
||
Comment on attachment 530456 [details] [diff] [review] fix the issue 's fine with me! :)
Attachment #530456 -
Flags: review?(joe) → review+
Comment 9•13 years ago
|
||
I don't think we should do this until we fix bug 421865.
Reporter | ||
Updated•13 years ago
|
Assignee: nobody → benjamin
Status: NEW → ASSIGNED
Comment 10•13 years ago
|
||
http://www.w3c-test.org/html/tests/approved/canvas/toDataURL.zerosize.html
Assignee: benjamin → nobody
Blocks: 622842
Status: ASSIGNED → NEW
OS: Windows 7 → All
Hardware: x86 → All
Updated•13 years ago
|
Assignee: nobody → benjamin
Status: NEW → ASSIGNED
Assignee | ||
Comment 11•13 years ago
|
||
This simple patch (requiring the the patch to 421865) fixes the issue.
Attachment #530456 -
Attachment is obsolete: true
Attachment #531507 -
Flags: review?(joe)
Comment 12•13 years ago
|
||
Comment on attachment 531507 [details] [diff] [review] fix based on 421865 Review of attachment 531507 [details] [diff] [review]: ----------------------------------------------------------------- Love it.
Attachment #531507 -
Flags: review?(joe) → review+
Assignee | ||
Updated•13 years ago
|
Keywords: checkin-needed
Updated•13 years ago
|
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: ietestcenter → ietestcenter [fixed in cedar]
Comment 13•13 years ago
|
||
Pushed: http://hg.mozilla.org/mozilla-central/rev/951de6cc1fdb
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: ietestcenter [fixed in cedar] → ietestcenter
Target Milestone: --- → mozilla6
Reporter | ||
Comment 14•13 years ago
|
||
Thanks for fixing this Benjamin.
Comment 15•13 years ago
|
||
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
Status: RESOLVED → VERIFIED
Assignee | ||
Updated•13 years ago
|
Status: VERIFIED → RESOLVED
Closed: 13 years ago → 13 years ago
You need to log in
before you can comment on or make changes to this bug.
Description
•