Closed Bug 812453 Opened 8 years ago Closed 8 years ago

Unexpected problem prompt when printing <canvas>

Categories

(Core :: Printing: Output, defect)

19 Branch
x86
Windows 7
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla20
Tracking Status
firefox18 --- unaffected
firefox19 + verified
firefox20 --- verified

People

(Reporter: scoobidiver, Assigned: nrc)

References

(Blocks 1 open bug, )

Details

(Keywords: regression, testcase)

Attachments

(2 files, 1 obsolete file)

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.
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)
Attached file Reduced testcase
It's an issue with printing <canvas>. See the reduced testcase.
Summary: Unexpected problem prompt when printing → Unexpected problem prompt when printing <canvas>
My guess goes to bug 800556.
Keywords: testcase
Duplicate of this bug: 811737
Assignee: nobody → ncameron
Attached patch fixes for printing (obsolete) — Splinter Review
Attachment #682852 - Flags: review?(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.
(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 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?
(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 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.
Attachment #682852 - Flags: review?(bzbarsky) → review+
Blocks: 748935
Depends on: 814149
carrying r=bz
Attachment #682852 - Attachment is obsolete: true
Attachment #684176 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/c4e31e87a072
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
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
Attachment #684176 - Flags: approval-mozilla-aurora?
Attachment #684176 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
In the future, please post direct hg changeset links rather than tbpl links. Also, don't forget to set the status when uplifting. Thanks!
Duplicate of this bug: 811737
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).
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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.