812453 - Unexpected problem prompt when printing <canvas>

Status: VERIFIED FIXED

(Core :: Printing: Output, defect)

Description

[Scoobidiver (away)]

When I try to print the above URL in Nightly, I get the "There was an unexpected problem when printing." prompt. There's no problem in Aurora.

Comment 1

[Loic]

It's a fresh regression:

m-c
good=2012-11-14
bad=2012-11-15
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=dd68409d7810&tochange=a761bfc192b5

(NB: during finding the regression range, you may meet bug 810597 which is fixed in nightly)

Comment 2

[Loic]

Attachment: Reduced testcase

It's an issue with printing <canvas>. See the reduced testcase.

Comment 3

[Loic]

My guess goes to bug 800556.

Comment 5

[Nick Cameron [:nrc]]

Attachment: fixes for printing

Comment 6

[Nick Cameron [:nrc]]

try push: https://tbpl.mozilla.org/?tree=Try&rev=829d4964ffb2

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 682852 [details] [diff] [review]
fixes for printing

Why is the change to what GetCanvas() returns needed?  That seems like a pretty odd change to me at first glance....  At the very least, it deserves a comment in the code.

Comment 8

[Nick Cameron [:nrc]]

(In reply to Boris Zbarsky (:bz) from comment #7)
> Comment on attachment 682852 [details] [diff] [review]
> fixes for printing
> 
> Why is the change to what GetCanvas() returns needed?  That seems like a
> pretty odd change to me at first glance....  At the very least, it deserves
> a comment in the code.

I don't really know, this was a change that had been made to the XPIDL version of the method, but not to the WebIDL version, I'm not sure if it directly affects printing. But bdahl pointed out that not doing it was a mistake.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 682852 [details] [diff] [review]
fixes for printing

> this was a change that had been made to the XPIDL version of the method, but not to the
> WebIDL version

Ah, I see.  The fix for bug 745025 was kinda buggy, alright.  Do we need to backport this part of the fix to 18?

Also, please add a comment with a pointer to that bug, since otherwise the blame would be off.

Past that, I assume the substantive part here is that drawImage with zero size throws and we used to ignore the error but now we do not?  Should we perhaps go back to ignoring the error?

Comment 10

[Nick Cameron [:nrc]]

(In reply to Boris Zbarsky (:bz) from comment #9)
> Comment on attachment 682852 [details] [diff] [review]
> fixes for printing
> 
> > this was a change that had been made to the XPIDL version of the method, but not to the
> > WebIDL version
> 
> Ah, I see.  The fix for bug 745025 was kinda buggy, alright.  Do we need to
> backport this part of the fix to 18?
> 

I'm not sure - I assume the new binding of GetCanvas will get used and we won't always use the old binding for some reason, in which case, yes we should probably backport that fix.

> Also, please add a comment with a pointer to that bug, since otherwise the
> blame would be off.
> 

Sure.

> Past that, I assume the substantive part here is that drawImage with zero
> size throws and we used to ignore the error but now we do not?  Should we
> perhaps go back to ignoring the error?

No, I don't think so. I think the issue was that the drawImage API changed from old to new bindings from one method which took a bunch of parameters (and a parameter to indicate which to use) to several overloaded methods taking some subset of those parameters and a protected implementation method (which was similar to the old API). That confused me and I used the wrong variant of the method in bug 800556. So this is just putting that right.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 682852 [details] [diff] [review]
fixes for printing

> I'm not sure - I assume the new binding of GetCanvas will get used and we won't always
> use the old binding for some reason

I believe we never use the old binding in 18, from script.

> I think the issue was that the drawImage API changed from old to new bindings 

Ah, of course.

r=me.  Let's get the GetCanvas thing backported too, probably in a separate bug.

Comment 12

[Nick Cameron [:nrc]]

Attachment: patch - just the CopyInnerTo parts

carrying r=bz

Comment 13

[Nick Cameron [:nrc]]

https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=c4e31e87a072

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c4e31e87a072

Comment 15

[Nick Cameron [:nrc]]

Comment on attachment 684176 [details] [diff] [review]
patch - just the CopyInnerTo parts

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 800556
User impact if declined: printing is impossible
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): very low, just correcting an error introduced recently.
String or UUID changes made by this patch: none

Comment 16

[Nick Cameron [:nrc]]

https://tbpl.mozilla.org/?tree=Mozilla-Aurora&rev=82774195d578

Comment 17

[Ryan VanderMeulen [:RyanVM]]

In the future, please post direct hg changeset links rather than tbpl links. Also, don't forget to set the status when uplifting. Thanks!

Comment 19

[Bogdan Maris, Desktop QA]

Mozilla/5.0 (Windows NT 6.1; rv:19.0) Gecko/20100101 Firefox/19.0

Verified as fixed in Firefox 19.0 RC (buildID: 20130215130331).

Comment 20

[Bogdan Maris, Desktop QA]

Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20100101 Firefox/20.0

Verified as fixed in Firefox 20 beta 6 (buildID: 20130320062118), the URLs provided in comment 0 and comment 2 can be printed with no problem.