Bug 170089 - [testcase] CSS inheritance and border-color
Summary: [testcase] CSS inheritance and border-color
Status: VERIFIED FIXED
Alias: None
Product: konqueror
Classification: Applications
Component: khtml (show other bugs)
Version: 4.1.0
Platform: Microsoft Windows Microsoft Windows
: NOR normal (vote)
Target Milestone: ---
Assignee: Konqueror Developers
URL: http://www.hixie.ch/tests/adhoc/css/i...
Keywords:
Depends on:
Blocks:
 
Reported: 2008-08-30 18:22 UTC by Gérard Talbot (no longer involved)
Modified: 2008-11-05 22:09 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
port of patch by Anatoli Papirovski <apapirovski@mac.com> (3.29 KB, patch)
2008-08-30 20:00 UTC, Germain Garand
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Gérard Talbot (no longer involved) 2008-08-30 18:22:01 UTC
Testcase:
http://www.hixie.ch/tests/adhoc/css/inheritance/border-color/001.html

Actual result: border is red

Expected results: border should be green

Firefox 3, Opera 9.5, Safari 4.0, Hv3 TKHTML alpha 16 all pass this test.

Regards, Gérard
Comment 1 Dario Andres 2008-08-30 18:25:10 UTC
Using KDE 4.1.1 (KDE 4.1.0 (4.1 >= 20080722)) (KDEmod) in ArchLinux i686:
I can reproduce this in Linuxi686
Comment 2 Germain Garand 2008-08-30 20:00:22 UTC
Created attachment 27140 [details]
port of patch by Anatoli Papirovski  <apapirovski@mac.com>
Comment 3 Maksim Orlovich 2008-08-30 20:06:45 UTC
Heh, I was just about to produce a similar patch, and was going to ask whether outline or others are affected before touching HANDLE_INHERIT_COND stuff. The patch you attached is incomplete in different way than mine, though: it doesn't handle the CSS_PROP_BORDER_LEFT: and similar cases.

--- css/cssstyleselector.cpp    (revision 818084)
+++ css/cssstyleselector.cpp    (working copy)
@@ -2022,6 +2022,16 @@
     return smaller ? a[r] : a[l];
 }

+// If we're explicitly inheriting an initial border-color, its computed value is based
+// on the parents' computed value of color, not ours.
+static QColor inheritedBorderColor( RenderStyle* parentStyle, const QColor& value )
+{
+    if ( value.isValid() )
+       return value;
+    else
+       return parentStyle->color();
+}
+
 void CSSStyleSelector::applyRule( int id, DOM::CSSValueImpl *value )
 {
 //     kDebug( 6080 ) << "applying property " << getPropertyName(id);
@@ -3531,10 +3541,10 @@
         if(id == CSS_PROP_BORDER || id == CSS_PROP_BORDER_COLOR)
         {
             if (isInherit) {
-                style->setBorderTopColor(parentStyle->borderTopColor());
-                style->setBorderBottomColor(parentStyle->borderBottomColor());
-                style->setBorderLeftColor(parentStyle->borderLeftColor());
-                style->setBorderRightColor(parentStyle->borderRightColor());
+                style->setBorderTopColor(inheritedBorderColor(parentStyle, parentStyle->borderTopColor()));
+                style->setBorderBottomColor(inheritedBorderColor(parentStyle, parentStyle->borderBottomColor()));
+                style->setBorderLeftColor(inheritedBorderColor(parentStyle, parentStyle->borderLeftColor()));
+                style->setBorderRightColor(inheritedBorderColor(parentStyle, parentStyle->borderRightColor()));
             }
             else if (isInitial) {
                 style->setBorderTopColor(QColor()); // Reset to invalid color so currentColor is used instead.
@@ -3577,7 +3587,7 @@
     case CSS_PROP_BORDER_TOP:
         if ( isInherit ) {
             style->setInheritedNoninherited(true);
-            style->setBorderTopColor(parentStyle->borderTopColor());
+            style->setBorderTopColor(inheritedBorderColor(parentStyle, parentStyle->borderTopColor()));
             style->setBorderTopStyle(parentStyle->borderTopStyle());
             style->setBorderTopWidth(parentStyle->borderTopWidth());
         } else if (isInitial)
@@ -3586,7 +3596,7 @@
     case CSS_PROP_BORDER_RIGHT:
         if (isInherit) {
             style->setInheritedNoninherited(true);
-            style->setBorderRightColor(parentStyle->borderRightColor());
+            style->setBorderRightColor(inheritedBorderColor(parentStyle, parentStyle->borderRightColor()));
             style->setBorderRightStyle(parentStyle->borderRightStyle());
             style->setBorderRightWidth(parentStyle->borderRightWidth());
         }
@@ -3596,7 +3606,7 @@
     case CSS_PROP_BORDER_BOTTOM:
         if (isInherit) {
             style->setInheritedNoninherited(true);
-            style->setBorderBottomColor(parentStyle->borderBottomColor());
+            style->setBorderBottomColor(inheritedBorderColor(parentStyle, parentStyle->borderBottomColor()));
             style->setBorderBottomStyle(parentStyle->borderBottomStyle());
             style->setBorderBottomWidth(parentStyle->borderBottomWidth());
         }
@@ -3606,7 +3616,7 @@
     case CSS_PROP_BORDER_LEFT:
         if (isInherit) {
             style->setInheritedNoninherited(true);
-            style->setBorderLeftColor(parentStyle->borderLeftColor());
+            style->setBorderLeftColor(inheritedBorderColor(parentStyle, parentStyle->borderLeftColor()));
             style->setBorderLeftStyle(parentStyle->borderLeftStyle());
             style->setBorderLeftWidth(parentStyle->borderLeftWidth());
         }
Comment 4 Germain Garand 2008-08-30 20:24:26 UTC
oops, sorry Maksim - bad timing... I knew I had that sitting in the merge pile :)

yes, I see no reason why outline wouldn't follow the same rule - and I see no other property that should.

And you are right, the patch by anatoli is missing some instances so we may merge both... do you want me to do it?
Comment 5 Germain Garand 2008-09-05 03:46:00 UTC
SVN commit 857212 by ggarand:

fix inheritance of border color

patch by Maksim Orlovich and
Anatoli Papirovski <apapirovski mac dot com>

BUG: 170089


 M  +32 -13    cssstyleselector.cpp  


--- trunk/KDE/kdelibs/khtml/css/cssstyleselector.cpp #857211:857212
@@ -186,6 +186,15 @@
     return;\
 }
 
+#define HANDLE_INHERIT_COND_WITH_BACKUP(propID, prop, propAlt, Prop) \
+if (id == propID) { \
+    if (parentStyle->prop().isValid()) \
+        style->set##Prop(parentStyle->prop()); \
+    else \
+        style->set##Prop(parentStyle->propAlt()); \
+    return; \
+}
+
 #define HANDLE_INITIAL_COND(propID, Prop) \
 if (id == propID) \
 {\
@@ -2091,6 +2100,16 @@
     return smaller ? a[r] : a[l];
 }
 
+// If we're explicitly inheriting an initial border-color, its computed value is based
+// on the parents' computed value of color, not ours.
+static QColor inheritedBorderColor( RenderStyle* parentStyle, const QColor& value )
+{
+    if ( value.isValid() )
+       return value;
+    else
+       return parentStyle->color();
+}
+
 void CSSStyleSelector::applyRule( int id, DOM::CSSValueImpl *value )
 {
 //     kDebug( 6080 ) << "applying property " << getPropertyName(id);
@@ -2719,12 +2738,12 @@
             if (id != CSS_PROP_COLOR)
                 style->setInheritedNoninherited(true);
             HANDLE_INHERIT_COND(CSS_PROP_BACKGROUND_COLOR, backgroundColor, BackgroundColor)
-            HANDLE_INHERIT_COND(CSS_PROP_BORDER_TOP_COLOR, borderTopColor, BorderTopColor)
-            HANDLE_INHERIT_COND(CSS_PROP_BORDER_BOTTOM_COLOR, borderBottomColor, BorderBottomColor)
-            HANDLE_INHERIT_COND(CSS_PROP_BORDER_RIGHT_COLOR, borderRightColor, BorderRightColor)
-            HANDLE_INHERIT_COND(CSS_PROP_BORDER_LEFT_COLOR, borderLeftColor, BorderLeftColor)
+            HANDLE_INHERIT_COND_WITH_BACKUP(CSS_PROP_BORDER_TOP_COLOR, borderTopColor, color, BorderTopColor)
+            HANDLE_INHERIT_COND_WITH_BACKUP(CSS_PROP_BORDER_BOTTOM_COLOR, borderBottomColor, color, BorderBottomColor)
+            HANDLE_INHERIT_COND_WITH_BACKUP(CSS_PROP_BORDER_RIGHT_COLOR, borderRightColor, color, BorderRightColor)
+            HANDLE_INHERIT_COND_WITH_BACKUP(CSS_PROP_BORDER_LEFT_COLOR, borderLeftColor, color, BorderLeftColor)
             HANDLE_INHERIT_COND(CSS_PROP_COLOR, color, Color)
-            HANDLE_INHERIT_COND(CSS_PROP_OUTLINE_COLOR, outlineColor, OutlineColor)
+            HANDLE_INHERIT_COND_WITH_BACKUP(CSS_PROP_OUTLINE_COLOR, outlineColor, color, OutlineColor)
             return;
         } else if (isInitial) {
             // The border/outline colors will just map to the invalid color |col| above.  This will have the
@@ -3608,10 +3627,10 @@
         if(id == CSS_PROP_BORDER || id == CSS_PROP_BORDER_COLOR)
         {
             if (isInherit) {
-                style->setBorderTopColor(parentStyle->borderTopColor());
-                style->setBorderBottomColor(parentStyle->borderBottomColor());
-                style->setBorderLeftColor(parentStyle->borderLeftColor());
-                style->setBorderRightColor(parentStyle->borderRightColor());
+                style->setBorderTopColor(inheritedBorderColor(parentStyle, parentStyle->borderTopColor()));
+                style->setBorderBottomColor(inheritedBorderColor(parentStyle, parentStyle->borderBottomColor()));
+                style->setBorderLeftColor(inheritedBorderColor(parentStyle, parentStyle->borderLeftColor()));
+                style->setBorderRightColor(inheritedBorderColor(parentStyle, parentStyle->borderRightColor()));
             }
             else if (isInitial) {
                 style->setBorderTopColor(QColor()); // Reset to invalid color so currentColor is used instead.
@@ -3654,7 +3673,7 @@
     case CSS_PROP_BORDER_TOP:
         if ( isInherit ) {
             style->setInheritedNoninherited(true);
-            style->setBorderTopColor(parentStyle->borderTopColor());
+            style->setBorderTopColor(inheritedBorderColor(parentStyle, parentStyle->borderTopColor()));
             style->setBorderTopStyle(parentStyle->borderTopStyle());
             style->setBorderTopWidth(parentStyle->borderTopWidth());
         } else if (isInitial)
@@ -3663,7 +3682,7 @@
     case CSS_PROP_BORDER_RIGHT:
         if (isInherit) {
             style->setInheritedNoninherited(true);
-            style->setBorderRightColor(parentStyle->borderRightColor());
+            style->setBorderRightColor(inheritedBorderColor(parentStyle,parentStyle->borderRightColor()));
             style->setBorderRightStyle(parentStyle->borderRightStyle());
             style->setBorderRightWidth(parentStyle->borderRightWidth());
         }
@@ -3673,7 +3692,7 @@
     case CSS_PROP_BORDER_BOTTOM:
         if (isInherit) {
             style->setInheritedNoninherited(true);
-            style->setBorderBottomColor(parentStyle->borderBottomColor());
+            style->setBorderBottomColor(inheritedBorderColor(parentStyle, parentStyle->borderBottomColor()));
             style->setBorderBottomStyle(parentStyle->borderBottomStyle());
             style->setBorderBottomWidth(parentStyle->borderBottomWidth());
         }
@@ -3683,7 +3702,7 @@
     case CSS_PROP_BORDER_LEFT:
         if (isInherit) {
             style->setInheritedNoninherited(true);
-            style->setBorderLeftColor(parentStyle->borderLeftColor());
+            style->setBorderLeftColor(inheritedBorderColor(parentStyle, parentStyle->borderLeftColor()));
             style->setBorderLeftStyle(parentStyle->borderLeftStyle());
             style->setBorderLeftWidth(parentStyle->borderLeftWidth());
         }
Comment 6 Gérard Talbot (no longer involved) 2008-11-05 22:09:27 UTC
With browsershots.org, I can see it fixed with Konqueror 4.1.2

Marking as VERIFIED