366616 - Canvas in XUL is not rendered

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Andrew Miller]

Canvas embedded in XUL is no longer visible. Operations on the context do not raise exceptions, but the output is simply not visible.

This can be reproduced as follows:
=> Load the attached testcase into a recent trunk build of browser.
=> With older canvas-enabled builds (e.g. 1.8 or 1.8.0 branch, or the trunk, as checked out 21-Nov-2006), you will correctly see a red area 100px by 100px.
=> With the current trunk, this area is not visible.

Comment 1

[Andrew Miller]

Attachment: A test-case showing the problem.

Comment 2

[Andrew Miller]

This bug is happening because nsHTMLCanvasFrame::PaintCanvas assumes that nsHTMLCanvasFrame::ComputeSize is called, so that mCanvasSize is not initialised, and hence the X and Y scaling factors are ending up being inf.

Comment 3

[Andrew Miller]

Attachment: A patch to compute the canvas size from paint if needed.

Comment 4

[Andrew Miller]

Attachment: Get rid of mCanvasSize altogether, and recompute every time

The previous patch was insufficient, because it didn't take into account changes in the size of the canvas. I have now got rid of mCanvasSize (the only other option would be to invalidate it every time anything that might change the size happens, but it looks like there is nothing that expensive involved to calculate canvasSize anyway).

Comment 5

[:Gavin Sharp [email: [e-mail]]]

This is a regression from the reflow branch.

Comment 6

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 251142 [details] [diff] [review]
Get rid of mCanvasSize altogether, and recompute every time

This looks OK to me, but I'd be more comfortable if dbaron could glance at it to make sure there is no underlying bug.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 251142 [details] [diff] [review]
Get rid of mCanvasSize altogether, and recompute every time

sr=dbaron

Comment 8

[Jeff Walden [:Waldo]]

Patch checked in on trunk, leaving open until we have a testcase committed for this...

Comment 10

[Andrew Miller]

Re-assigning to nobody, so someone who knows about Mozilla's test system can write a test-case.

Comment 11

[Andrew Miller]

Attachment: Reftest

Comment 12

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 255747 [details] [diff] [review]
Reftest

I'm flattered, but I defer my powers of sr for this bug's test to sayrer, who actually knows something about the reftest stuff. :)

Comment 13

[Jeff Walden [:Waldo]]

Comment on attachment 255747 [details] [diff] [review]
Reftest

Looks right to me, but because of my current unique computing situation I can't actually test it to see whether it works or not.  If someone else could give this a whirl and see how it works, just to verify, that'd be great.  :-)

Comment 14

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 255747 [details] [diff] [review]
Reftest

Again, I say, to sayrer!

Comment 15

[Robert Sayre]

Comment on attachment 255747 [details] [diff] [review]
Reftest

sr? nonsense. down with the bureaucracy!

Comment 16

[Robert Sayre]

(sr is not required for code that isn't in-process)

Comment 17

[Martijn Wargers (dead)]

I went ahead and checked the reftest in (after approval from Jeff and checking that the test worked).
Please shoot me when I shouldn't have done that.
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=reftest.list&branch=&root=/cvsroot&subdir=mozilla/layout/reftests/bugs&command=DIFF_FRAMESET&rev1=1.24&rev2=1.25

Comment 18

[Michael Bonfils]

Attachment: Another testcase

This example doesn't work with XulRunner 1.9 (nightly build of 2007/03/14).

Comment 19

[Michael Bonfils]

I've added a new testcase to show that bug is not fixed with 1.9a3, I'm using Windows XP.

Comment 20

[Martijn Wargers (dead)]

Michael, could you file a new bug on that? Thanks.
Btw, I get an xml parser error with your testcase.

Comment 21

[Michael Bonfils]

I've enterer a new bug :
http://bugzilla.mozilla.org/show_bug.cgi?id=374038

Sorry for the bad xml.