Bug 130225

Created attachment 16882 [details]
Test case

Outputs:
DIV: #cccccc
TD 1: #dddddd
TD 2: #000000
TR: #ff0000

Expected:
DIV: #cccccc
TD 1: #dddddd
TD 2: #ff0000
TR: #ff0000

Sorry, the "Expected" above is wrong.

It should return "transparent" for the "TD 1" probably, as Firefox does or "" (empty).

I don't know what the specs say, but #000000 (black) is obviously wrong.

It not only happens with TR, it happens with every element having a transparent background.

Most browsers return "transparent" as value. Opera 8.5 returns "rgba(0, 0, 0, 0)" -> opacity=0 which also means "transparent".

I was searching for a workaround via getCSSPropertyValue, but this seems to be pretty broked in Konq 3.5.2, also... although I guess that's a different bug.

I suppose TD 2: rgba(0,0,0,0.286275) isn't the right answer, either? ;-) 
[working on this]

SVN commit 559918 by orlovich:

Properly output 'transparent' for cssText. While at it, also output 
rgba values, as we parse them (though we can't render them yet).

This centralization will also makes this easier to fully port to Qt4.
BUG:130225


 M  +15 -8     css_renderstyledeclarationimpl.cpp  
 M  +11 -1     css_valueimpl.cpp  


--- branches/KDE/3.5/kdelibs/khtml/css/css_renderstyledeclarationimpl.cpp #559917:559918
@@ -265,6 +265,14 @@
     return "";
 }
 
+static CSSPrimitiveValueImpl* valueForColor(QColor color)
+{
+    if (color.isValid())
+        return new CSSPrimitiveValueImpl(color.rgb());//### KDE4: use rgba!
+    else
+        return new CSSPrimitiveValueImpl(khtml::transparentColor);
+}
+
 static CSSValueImpl* valueForShadow(const ShadowData *shadow)
 {
     if (!shadow)
@@ -274,7 +282,7 @@
         CSSPrimitiveValueImpl *x = new CSSPrimitiveValueImpl(s->x, CSSPrimitiveValue::CSS_PX);
         CSSPrimitiveValueImpl *y = new CSSPrimitiveValueImpl(s->y, CSSPrimitiveValue::CSS_PX);
         CSSPrimitiveValueImpl *blur = new CSSPrimitiveValueImpl(s->blur, CSSPrimitiveValue::CSS_PX);
-        CSSPrimitiveValueImpl *color = new CSSPrimitiveValueImpl(s->color.rgb());
+        CSSPrimitiveValueImpl *color = valueForColor(s->color);
         list->append(new ShadowValueImpl(x, y, blur, color));
     }
     return list;
@@ -319,7 +327,6 @@
     return new CSSPrimitiveValueImpl(CSS_VAL_AUTO);
  }
 
-
 RenderStyleDeclarationImpl::RenderStyleDeclarationImpl( DOM::NodeImpl *node )
     : CSSStyleDeclarationImpl(0), m_node(node)
 {
@@ -374,7 +381,7 @@
     switch(propertyID)
     {
     case CSS_PROP_BACKGROUND_COLOR:
-        return new CSSPrimitiveValueImpl(style->backgroundColor().rgb());
+        return valueForColor(style->backgroundColor());
     case CSS_PROP_BACKGROUND_IMAGE:
         if (style->backgroundImage())
             return new CSSPrimitiveValueImpl(style->backgroundImage()->url(),
@@ -438,13 +445,13 @@
         return new CSSPrimitiveValueImpl(style->borderVerticalSpacing(),
                                          CSSPrimitiveValue::CSS_PX);
     case CSS_PROP_BORDER_TOP_COLOR:
-        return new CSSPrimitiveValueImpl(style->borderTopColor().rgb());
+        return valueForColor(style->borderTopColor());
     case CSS_PROP_BORDER_RIGHT_COLOR:
-        return new CSSPrimitiveValueImpl(style->borderRightColor().rgb());
+        return valueForColor(style->borderRightColor());
     case CSS_PROP_BORDER_BOTTOM_COLOR:
-        return new CSSPrimitiveValueImpl(style->borderBottomColor().rgb());
+        return valueForColor(style->borderBottomColor());
     case CSS_PROP_BORDER_LEFT_COLOR:
-        return new CSSPrimitiveValueImpl(style->borderLeftColor().rgb());
+        return valueForColor(style->borderLeftColor());
     case CSS_PROP_BORDER_TOP_STYLE:
         return valueForBorderStyle(style->borderTopStyle());
     case CSS_PROP_BORDER_RIGHT_STYLE:
@@ -492,7 +499,7 @@
     case CSS_PROP_CLIP:
         break;
     case CSS_PROP_COLOR:
-        return new CSSPrimitiveValueImpl(style->color().rgb());
+        return valueForColor(style->color());
     case CSS_PROP_CONTENT:
         break;
     case CSS_PROP_COUNTER_INCREMENT:
--- branches/KDE/3.5/kdelibs/khtml/css/css_valueimpl.cpp #559917:559918
@@ -755,7 +755,17 @@
             break;
         }
 	case CSSPrimitiveValue::CSS_RGBCOLOR:
-	    text = QColor(m_value.rgbcolor).name();
+	    if (qAlpha(m_value.rgbcolor) != 0xFF) {
+		if (m_value.rgbcolor == khtml::transparentColor)
+		    text = "transparent";
+		else
+		    text = "rgba(" + QString::number(qRed  (m_value.rgbcolor)) + "," 
+				   + QString::number(qBlue (m_value.rgbcolor)) + "," 
+				   + QString::number(qGreen(m_value.rgbcolor)) + "," 
+				   + QString::number(qAlpha(m_value.rgbcolor)/255.0) + ")";
+	    } else {
+		text = QColor(m_value.rgbcolor).name();
+	    }
 	    break;
         case CSSPrimitiveValue::CSS_PAIR:
             text = m_value.pair->first()->cssText();

By the way, please file reports of whatever other bugs you notice...

Created attachment 17086 [details]
Another backgroundColor bug patch

I found similar bug. Take a look to http://www.bright-shadows.net/. On the left
is menu which "on hover" fade colors to other one (check FF or IE). The problem
exist in JS, where to site programmer expect for that style.backgroundColor
returns value in this same format that was entered (in this case rgb(rr,gg,bb)
), but konqueror always returns it in #RRGGBB. The patch above from Maksism is
not enough. My patch fix my case but not this one, and for sure colliding with
Maksim's one. Someone should reopen this bug, and merge our patches.

Patch description: In CSSPrimitiveValueImpl I add one member which holds type
of entered color description (#,rgb,rgba). The CSSPrimitiveValueImpl::cssText
create return value on base of this value.

Looks like a good idea, (I think there is a fixme about something like this), 
though it needs some massaging not to increase memory usage (easy fix may be to bitfield m_type, and have this outside the union as m_subType or such), and seems likely to screw up on transparent. Will take a look sometime.

This patch don't increase memory usage, there is double in this union so it has 8 bit with or without this patch. 

Also, I think that adding transparent (RGBTYPE_TRANSPARENT) to enum can help to merge those patches.

Good point, missed that. 

Created attachment 17087 [details]
merged patches

OK I've merged both patches, but you should check it. I did it by hand. Those
SVN diffs are impossible to apply :/, or at least I don't know how to do that.

Created attachment 17214 [details]
patch against 3.5.4

Patch against 3.5.4 + minor (but important [when color is
khtml::transparentColor set RGBTYPE_TRANSPARENT] ) change I forget to add last
time.

I changed the code to always produce rgb() and rgba() respectively. Firefox does so. The #RRGGBB format is never returned. Fixes the menu for me.

Thanks for fix!