Bug 67898 – REGRESSION(r94900): fast/images/support-broken-image-delegate.html fails on Mac

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Ryosuke Niwa]

After http://trac.webkit.org/changeset/94900, fast/images/support-broken-image-delegate.html started failing on Leopard, Snow Leopard, & Lion:
http://build.webkit.org/results/SnowLeopard%20Intel%20Debug%20(Tests)/r94911%20(2096)/results.html
http://build.webkit.org/results/Leopard%20Intel%20Debug%20(Tests)/r94909%20(33500)/results.html
http://build.webkit.org/results/Lion%20Intel%20Debug%20(Tests)/r94910%20(535)/results.html

[Ryosuke Niwa]

Temporarily skipped the test in http://trac.webkit.org/changeset/94915.

[Beth Dakin]

Thanks so much Ryosuke. 

I looked at this a bit, and I think I understand the problem. I changed RenderImage::imageSizeForError() so that instead of accessing CachedImage::image() and then measuring the size, it goes directly to CachedImage::brokenImage(). I think this test shows that that is wrong because in this case m_shouldPaintBrokenImage would be false and rather than returning the brokenImage,  CachedImage::image() would have returned the null image. 

I think I have time to post a patch for this before I have to go.

[Beth Dakin]

Created attachment 106993 [details]
Patch

Here's a patch to fix the test failure. Maybe check check for willPaintBrokenImage() should be inside brokenImage() itself…not sure…for the time being, I made it an independent function.

[Ryosuke Niwa]

cc more people in the hope they can review this patch.

[WebKit Review Bot]

Comment on attachment 106993 [details]
Patch

Attachment 106993 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9641169

New failing tests:
svg/css/getComputedStyle-basic.xhtml
svg/custom/svg-fonts-word-spacing.html
media/video-zoom-controls.html

[Simon Fraser (smfr)]

Comment on attachment 106993 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=106993&action=review

> Source/WebCore/ChangeLog:6
> +        REGRESSION(r94900): fast/images/support-broken-image-delegate.html fails on Mac
> +
> +        No new tests. (OOPS!)

Some verbage here describing the change would be good.

[Beth Dakin]

(In reply to comment #6)
> (From update of attachment 106993 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=106993&action=review
> 
> > Source/WebCore/ChangeLog:6
> > +        REGRESSION(r94900): fast/images/support-broken-image-delegate.html fails on Mac
> > +
> > +        No new tests. (OOPS!)
> 
> Some verbage here describing the change would be good.

Thanks, Simon! I will add some verbiage.

I cannot reproduce those test failures that the Bot claims are failures. It would also be very surprising if this change caused failures in those tests anyway. I plan to commit the change and monitor the build bots.

[Beth Dakin]

Committed change with revision 94980.