Last Comment Bug 670442 - CanvasRenderingContext is broken after being sized to 0 and back
: CanvasRenderingContext is broken after being sized to 0 and back
Status: VERIFIED FIXED
: regression, testcase
Product: Core
Classification: Components
Component: Canvas: 2D (show other bugs)
: 6 Branch
: All Other
: -- normal (vote)
: mozilla6
Assigned To: :Benjamin Peterson
:
Mentors:
Depends on:
Blocks: 421865
  Show dependency treegraph
 
Reported: 2011-07-09 09:13 PDT by Pierre Bertet
Modified: 2013-12-27 14:33 PST (History)
6 users (show)
khuey: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
+
fixed
fixed
fixed


Attachments
firefox6-putimagedata.html (1.66 KB, text/plain)
2011-07-09 09:13 PDT, Pierre Bertet
no flags Details
Reftest (3.16 KB, patch)
2011-07-09 21:33 PDT, Kyle Huey [:khuey] (khuey@mozilla.com)
no flags Details | Diff | Splinter Review
reset mZero when new dimensions are set (3.81 KB, patch)
2011-07-10 10:06 PDT, :Benjamin Peterson
joe: review+
Details | Diff | Splinter Review
fixes style nits; ready for landing (4.05 KB, patch)
2011-07-12 08:46 PDT, :Benjamin Peterson
asa: approval‑mozilla‑aurora+
asa: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Pierre Bertet 2011-07-09 09:13:39 PDT
Created attachment 544998 [details]
firefox6-putimagedata.html

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:6.0) Gecko/20100101 Firefox/6.0
Build ID: 20110705195857

Steps to reproduce:

Here is a test (attached too): http://dl.dropbox.com/u/360991/tmp/firefox6-putimagedata.html


Actual results:

The first canvas is not drawn on the second one.


Expected results:

The first canvas should be drawn on the second one.
Comment 1 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-07-09 18:26:51 PDT
If I click the button once, the line from the first canvas is drawn on the second.

If I click the button again, both canvases become blank.

In Webkit (Safari 5, specifically) clicking the button more than once has no visual effect.
Comment 2 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-07-09 21:33:01 PDT
Created attachment 545045 [details] [diff] [review]
Reftest
Comment 3 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-07-09 23:36:49 PDT
This is a regression from Bug 421865.
Comment 4 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-07-10 04:17:24 PDT
I think we should back out bug 421865 on trunk and branches. Joe, can you do that?
Comment 5 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-07-10 08:07:30 PDT
I think the fix is trivial.  Testing now.
Comment 6 :Benjamin Peterson 2011-07-10 10:06:15 PDT
Created attachment 545084 [details] [diff] [review]
reset mZero when new dimensions are set
Comment 7 Sheila Mooney 2011-07-11 14:51:31 PDT
We are going to track this since it seems like something needs to be done if we are proposing to back this out on nightly. Cc'ing Asa on the bug.
Comment 8 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-07-11 14:59:18 PDT
I think we should probably just take the fix (once reviewed and checked in on central, of course) but something will have to be done.
Comment 9 Joe Drew (not getting mail) 2011-07-12 06:55:42 PDT
Comment on attachment 545084 [details] [diff] [review]
reset mZero when new dimensions are set

Review of attachment 545084 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for not catching this in review!

::: content/canvas/src/nsCanvasRenderingContext2D.cpp
@@ +1067,2 @@
>          // Zero sized surfaces have problems, so just use a 1 by 1.
>          if (height == 0 || width == 0) {

I think I'd rather have the mZero = PR_FALSE in an else block, but I don't feel strongly about it, so do what feels more natural.

::: content/canvas/src/nsCanvasRenderingContext2DAzure.cpp
@@ +1225,2 @@
>    // Zero sized surfaces cause issues, so just go with 1x1.
>    if (height == 0 || width == 0) {

Same as the other mZero.
Comment 10 :Benjamin Peterson 2011-07-12 08:46:34 PDT
Created attachment 545388 [details] [diff] [review]
fixes style nits; ready for landing
Comment 12 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-07-12 09:07:48 PDT
Comment on attachment 545388 [details] [diff] [review]
fixes style nits; ready for landing

Drivers, this low-risk (adds two lines of code) patch fixes a web-facing regression in <canvas> from Bug 421865, which landed for Firefox 6.
Comment 14 George Carstoiu 2011-07-14 04:04:56 PDT
Mozilla/5.0 (X11; Linux i686; rv:6.0) Gecko/20100101 Firefox/6.0 - beta 2

I've just tried to verify this issue but it's unclear to me whether it is fixed or not.

On Fx beta 6 (on Ubuntu 11.04 and WinXP), using the testcase from Comment 0, if I press once the canvas is copied and if I press a second time nothing happens. On Nightly and Aurora pushing the button twice clears the two image areas. 

On Mac OS X 10.7 Nightly, Aurora an Beta builds display the same behavior: on the second click on the button, nothing changes.

Can you please clarify this?

Thanks!
Comment 15 Pierre Bertet 2011-07-14 06:59:42 PDT
George: On the second click, nothing should changes.

When the bug was reported, with Firefox 6 beta, 7 and 8, the two canvas were erased.

Tested with the repo tip on OS X 10.6, it works for me now (nothing happens on the second click).
Comment 16 George Carstoiu 2011-07-14 09:03:18 PDT
Yes, on Mac OS builds everything works as expected - the second click does nothing. The problem still persists on Windows and Linux builds as I mentioned in Comment 14.
Comment 17 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-07-14 09:05:19 PDT
I see the expected behavior on Windows on http://hg.mozilla.org/releases/mozilla-aurora/rev/ce993f19fc4ba and http://hg.mozilla.org/mozilla-central/rev/931f06b80727 ....
Comment 18 George Carstoiu 2011-07-18 00:52:23 PDT
Mozilla/5.0 (X11; Linux i686; rv:8.0a1) Gecko/20110717 Firefox/8.0a1

Rechecked on Nightly, Aurora and 6 Beta 2 and everything seems in order across platforms. It looks like the patch hadn't landed yet when I tried to verify.

The issue is no longer reproducible on all three builds on WinXP, Ubuntu 11.04, Win 7 and Mac OS X 10.7.

Setting status to Verified Fixed.

Thanks guys and sorry for the small confusion.

Note You need to log in before you can comment on or make changes to this bug.