Bug 22457 – Webpages without bgcolor definition are rendered transparent by default when transparency is activated

• Component: Layout and Rendering
• Status: NEW

[Bernd Weber]

I run a GTK webkit here with transparency activated. Now we found when we load an HTML page that has no bgcolor information, neither as attribute in the body tag, nor as stylesheet property, then the page background is set to transparent and not to the default background color defined in WebCore/page/FrameView.cpp (->m_baseBackgroundColor). This seems to be because the check in WebCore/rendering/RenderBox.cpp is wrong (See patch). In case of an invalid background color, like in our case, it shouldn't matter whether transparency is active or not the background should then always be set to the baseBackgroundColor. I think the parenthesis' are just set incorrectly here.

Test page:
No background color information. Can be as short as just one line of HTML, like:

<a href="http://www.google.com">google</a>



Proposed patch:
--- RenderBox.cpp	2008-11-24 09:19:12.418949000 -0800
+++ WebKit-r38707/WebCore/rendering/RenderBox.cpp	2008-11-24 09:19:25.707078000 -0800
@@ -834,7 +834,7 @@
     if (!bgLayer->next()) {
         IntRect rect(tx, clipY, w, clipH);
         // If we have an alpha and we are painting the root element, go ahead and blend with the base background color.
-        if (isRoot() && (!bgColor.isValid() || bgColor.alpha() < 0xFF) && !isTransparent) {
+        if (isRoot() && (!bgColor.isValid() || (bgColor.alpha() < 0xFF && !isTransparent))) {
             Color baseColor = view()->frameView()->baseBackgroundColor();
             if (baseColor.alpha() > 0) {
                 context->save();

[Bernd Weber]

Created attachment 25431 [details]
Fix default background color for Transparency

Same patch as in original description just as diff file. Patch based on r38707

[Simon Fraser (smfr)]

A real tests here would require some native code to set the FrameView's baseBackgroundColor, if I am understanding the issue. We need to make sure that this change doesn't break Mac.

[David Kilzer (:ddkilzer)]

Comment on attachment 25431 [details]
Fix default background color for Transparency

Set review? flag.

[David Kilzer (:ddkilzer)]

Comment on attachment 25431 [details]
Fix default background color for Transparency

Bernd, you are not a reviewer for the WebKit project, so please don't set the "review+" flag.  I se the flag to "review?" so that others would take a look at it.

Please see this page for more details:

http://webkit.org/coding/commit-review-policy.html

[Dave Hyatt]

Comment on attachment 25431 [details]
Fix default background color for Transparency

There is nothing wrong with this check.  This is exactly what a view being transparent means.... that there *is* no base background color to blend with.

Changing this would break Dashboard widgets on Mac for example.

I'm not sure what kind of definition you're using for a transparent WebView if you still want the background color to be used... the WebView isn't truly transparent if it's always painting the background color...

[Darin Adler]

Comment on attachment 25431 [details]
Fix default background color for Transparency

This patch has no change log and no test case, so we can't do a review of it as-is. It may be necessary to add features to DumpRenderTree to test it.

In addition, I'm not sure the patch is right for all cases; this is why a test would be so important and valuable. I don't fully understand how "no background color" is different from "transparent background color", since this code treats them differently.

review- because we require tests for bug fixes like this one.

Perhaps this is a special case and can be landed without a test, but if so we'll need a lot more information to make it clear why, starting with a ChangeLog.

[Bernd Weber]

More detailed description of the problem:
During object initialization the background color value is reset to 0, to have an initialized value there. Now, if the webpage does not have a background color defined this value is never changed it stays at all 0 (0x00000000). In case of 32bit ARGB that means that all alpha bits are set to 0 and therefore the color is fully transparent. The system, however, recognizes that the color is not valid, since it has never really been set, except during initialization.
If the system does no transparency it will fall back to the default background color defined in FrameView.cpp in this check. For the transparency case, however, it will not fall back to default since it will always find the alpha value to be smaller than 0xFF, see above, initialization with 0.
I think this is wrong since the background color should either be defined as a specific color or always fall back to a default color. That default color may of course be transparent but this is not necessarily right in any case. The programmer of an application should be able to decide what the behavior should be, by defining the default background color. Right now he does have no choice since it will always be set to transparent if no background color is defined in the webpage.
As for the tests. I'm sorry but I don't have the time right now to look into this. I'll attach an HTML file that should trigger the problem. 
Just wanted to make you guys aware of the issue here since we ran into it.

[Bernd Weber]

Created attachment 25841 [details]
Basic HTML file without background color information

As described earlier, a basic HTML file without any background color information. For the transparency activitated case this should trigger the issue.