there's a divide by zero crash in RenderBox::calculateBackgroundSize when you load a 0,0 bmp as the background image and specify -khtml-background-size It crashes on: w = bg->imageSize(this, style()->effectiveZoom()).width() * h / bg->imageSize(this, style()->effectiveZoom()).height(); since bg->imageSize returns 0,0
Created attachment 22187 [details] sample 0,0 bitmap
Created attachment 22188 [details] page that links to 0,0 bitmap bugfile that causes crash
I suspect change set 31981 introduced this bug when it removed the following statements from RenderBox::imageChanged: if (!image || !image->canRender(style()->effectiveZoom()) || !parent() || !view()) return; canRender will return false if the image is zero width or zero height. If we take the early out in RenderBox::imageChanged we never call calculateBackgroundSize, which assumes that any background images have non-zero height.
Created attachment 22479 [details] Proposed fix Proposed fix, including LayoutTest.
My fix is busted. Breaks rendering of broken images.
Created attachment 22681 [details] New patch for bug 19964
Comment on attachment 22681 [details] New patch for bug 19964 + * ChangeLog: The ChangeLog itself shouldn't be listed in the ChangeLog. We need a ChangeLog for the LayoutTests directory too, not just WebCore. Otherwise this patch looks good. I'm going to say review+ but having no patch for LayoutTests does add work for whoever commits the patch.
Hello, Can you please explain me what did you mean by having no patch for LayoutTests? The patch i submitted included a test for the problem itself: "LayoutTests/css3/khtml-background-size-0x0-bmp.html" and the needed bitmap for the test. If more tests are needed for the patch to land in the trunk, please tell me and i will be glad to do it. Regards, Mihnea Ovidenie > (From update of attachment 22681 [details] [edit]) > + * ChangeLog: > > The ChangeLog itself shouldn't be listed in the ChangeLog. > > We need a ChangeLog for the LayoutTests directory too, not just WebCore. > > Otherwise this patch looks good. I'm going to say review+ but having no patch > for LayoutTests does add work for whoever commits the patch. >
Landed in r36047. Mihnea, what Darin meant is that you did not include a ChangeLog entry for your changes inside LayoutTests. I added one when I landed the change, but in the future you should include one with your patches.