Closed
Bug 366616
Opened 18 years ago
Closed 18 years ago
Canvas in XUL is not rendered
Categories
(Core :: Graphics: Canvas2D, defect)
Core
Graphics: Canvas2D
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha1
People
(Reporter: andrew, Unassigned)
References
Details
(Keywords: regression, testcase)
Attachments
(4 files, 1 obsolete file)
(deleted),
application/vnd.mozilla.xul+xml
|
Details | |
(deleted),
patch
|
vlad
:
review+
dbaron
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
(deleted),
application/vnd.mozilla.xul+xml
|
Details |
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.
Reporter | ||
Comment 1•18 years ago
|
||
Reporter | ||
Comment 2•18 years ago
|
||
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.
Reporter | ||
Comment 3•18 years ago
|
||
Reporter | ||
Comment 4•18 years ago
|
||
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).
Assignee: nobody → ak.miller
Attachment #251131 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #251142 -
Flags: review?
Reporter | ||
Updated•18 years ago
|
Attachment #251142 -
Flags: review? → review?(vladimir)
Comment 5•18 years ago
|
||
This is a regression from the reflow branch.
Updated•18 years ago
|
Flags: blocking1.9?
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.
Attachment #251142 -
Flags: review?(vladimir) → review+
Reporter | ||
Updated•18 years ago
|
Attachment #251142 -
Flags: superreview?(dbaron)
Comment 7•18 years ago
|
||
Comment on attachment 251142 [details] [diff] [review]
Get rid of mCanvasSize altogether, and recompute every time
sr=dbaron
Attachment #251142 -
Flags: superreview?(dbaron) → superreview+
Reporter | ||
Updated•18 years ago
|
Whiteboard: [checkin needed]
Comment 8•18 years ago
|
||
Patch checked in on trunk, leaving open until we have a testcase committed for this...
Flags: in-testsuite?
Whiteboard: [checkin needed]
Target Milestone: --- → mozilla1.9alpha
Flags: blocking1.9? → blocking1.9+
Reporter | ||
Comment 10•18 years ago
|
||
Re-assigning to nobody, so someone who knows about Mozilla's test system can write a test-case.
Assignee: ak.miller → nobody
Status: ASSIGNED → NEW
Reporter | ||
Comment 11•18 years ago
|
||
Attachment #255747 -
Flags: superreview?(shaver)
Attachment #255747 -
Flags: review?(jwalden+bmo)
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. :)
Attachment #255747 -
Flags: superreview?(shaver) → superreview?(sayrer)
Comment 13•18 years ago
|
||
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. :-)
Attachment #255747 -
Flags: superreview?(shaver)
Attachment #255747 -
Flags: superreview?(sayrer)
Attachment #255747 -
Flags: review?(jwalden+bmo)
Attachment #255747 -
Flags: review+
Comment on attachment 255747 [details] [diff] [review]
Reftest
Again, I say, to sayrer!
Attachment #255747 -
Flags: superreview?(shaver) → superreview?(sayrer)
Comment 15•18 years ago
|
||
Comment on attachment 255747 [details] [diff] [review]
Reftest
sr? nonsense. down with the bureaucracy!
Attachment #255747 -
Flags: superreview?(sayrer) → review?(dbaron)
Comment 16•18 years ago
|
||
(sr is not required for code that isn't in-process)
Comment 17•18 years ago
|
||
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
Status: NEW → RESOLVED
Closed: 18 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
Updated•18 years ago
|
Attachment #255747 -
Flags: review?(dbaron)
Comment 18•18 years ago
|
||
This example doesn't work with XulRunner 1.9 (nightly build of 2007/03/14).
Comment 19•18 years ago
|
||
I've added a new testcase to show that bug is not fixed with 1.9a3, I'm using Windows XP.
Comment 20•18 years ago
|
||
Michael, could you file a new bug on that? Thanks.
Btw, I get an xml parser error with your testcase.
Comment 21•18 years ago
|
||
I've enterer a new bug :
http://bugzilla.mozilla.org/show_bug.cgi?id=374038
Sorry for the bad xml.
You need to log in
before you can comment on or make changes to this bug.
Description
•