Bug 13559 – REGRESSION: Canvas aspect ratio is incorrect

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Alexey Proskuryakov]

The animation in this test is rendered correctly by Firefox and shipping Safari, but is horizontally stretched in TOT.

Seems to work correctly when cached, but reproducible again after a reload.

[Darin Adler]

For some reason the width and height attributes of the canvas element are not changing the width and height -- the canvas dimensions instead match the default canvas width and height.

[Darin Adler]

There's a race condition here. When I set a breakpoint at HTMLCanvasElement::reset, the page worked fine.

I think the issue is that the reset function needs to do something to trigger layout.

[Darin Adler]

I think that HTMLCanvasElement::reset needs code that's more like what's in RenderImage::imageChanged. It needs to call setNeedsLayout(true) and setPrefWidthsDirty(true) at least in some cases. And maybe calcWidth and calcHeight too.

[Darin Adler]

I've got a fix, but I could use a reduced test case to check in as a layout test.

Created attachment 14296 [details]
Layout test

(In reply to comment #4)
> I've got a fix, but I could use a reduced test case to check in as a layout
> test.
> 

This should work.

[Darin Adler]

Created attachment 14297 [details]
patch -- passes all layout tests

(In reply to comment #3)
> I think that HTMLCanvasElement::reset needs code that's more like what's in
> RenderImage::imageChanged. It needs to call setNeedsLayout(true) and
> setPrefWidthsDirty(true) at least in some cases. And maybe calcWidth and
> calcHeight too.
> 

The RenderImage code is a bad example. See <http://bugs.webkit.org/show_bug.cgi?id=9276#c3>. Looks like with your patch, RenderHTMLCanvas would still have the bad behavior RenderImage currently has.

[Darin Adler]

Comment on attachment 14297 [details]
patch -- passes all layout tests

Mitz says review-. I'll make a new version addressing his comment.

[Darin Adler]

Created attachment 14299 [details]
patch -- passes all layout tests

[Darin Adler]

Created attachment 14300 [details]
patch incorporating Mitz's feedback, passes layout tests

[Dave Hyatt]

Comment on attachment 14300 [details]
patch incorporating Mitz's feedback, passes layout tests

r=me

[Darin Adler]

checked in r21211