Bug 10917 – REGRESSION (r16027): iFrame transparency broken

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

[Dan Cryer]

In the latest nightlies, iframe transparency has stopped working. The background displays as white, whereas other browsers display a transparent background. I made a redux that demonstrates it:

http://www.veriea.com/webkit/

The above works correctly in Firefox and the stock Safari, but not in the latest nightlies.

When loading the page, the iframe is initially rendered transparent, with its border, but then when the document within it loads, the white background appears. This doesn't happen with any background other than transparent, blue ( as you can see in the top iframe ) works fine.

Looks like a regression from r16207 (Allow changing the background color WebCore draws under transparent page backgrounds).

[Timothy Hatcher]

Created attachment 10655 [details]
Patch fixes this

- Don't paint the background color if we are told to do slow repaints.
- Removed unneeded Color isValid() checks where we also check alpha() > 0.
- Removed a check for alpha() > 0 before calling fillRect() since it also checks.

* rendering/RenderBox.cpp:
(WebCore::RenderBox::paintBackgroundExtended):

[Dave Hyatt]

Comment on attachment 10655 [details]
Patch fixes this

Slow repaints have nothing to do with background color painting.  That's just whether or not to blit or not when scrolling...

[Dave Hyatt]

-        bool shouldPaintBackgroundColor = !bgLayer->next() && bgColor.isValid() && bgColor.alpha() > 0;
+        bool shouldPaintBackgroundColor = !bgLayer->next() && bgColor.alpha() > 0;

I don't see the reason for this change.  This only applies when printing, and it seems wrong to me to remove the isValid check from it.

[Dave Hyatt]

I think to fix this you just need to adjust this line:

if (isRoot() && (!bgColor.isValid() || bgColor.alpha() < 0xFF) && !view()->frameView()->isTransparent()) {
        
To ask !isTransparent rather than !view()->frameView()->isTransparent().

To do that just pull the isTransparent boolean out of the previous if and init it to false.

I don't believe anything else should change in the code, since dropping the isValid checks is incorrect.

[Timothy Hatcher]

Created attachment 10668 [details]
Patch taking into account Dave's comments

Thanks Dave. I was under the impression that a color that has alpha > 0 was always valid. But that isn't the case it seems.

[Timothy Hatcher]

Created attachment 10669 [details]
Layout test attempt

I was going to make a pixel layout test for this, and here is the expected result image I get...

This file loads and looks fine in Safari (no white backgrounds). But DRT always paints the background of the iframes white. Any ideas? This happened with both versions of my patch.

[Dave Hyatt]

Comment on attachment 10668 [details]
Patch taking into account Dave's comments

r=me

[Timothy Hatcher]

Landed in r16491.