Implement !important rule handling for inline-style. Work from Jabier.

Added "!important" rule tests to style-test.cpp.

3 files changed, 93 insertions, 38 deletions

diff --git a/src/style-internal.cpp b/src/style-internal.cpp
index 5c2f5697f..666bd75c6 100644
--- a/src/style-internal.cpp
+++ b/src/style-internal.cpp
@@ -101,7 +101,7 @@ SPIFloat::write( guint const flags, SPStyleSrc const &style_src_req, SPIBase con
             return (name + ":inherit;");
         } else {
             Inkscape::CSSOStringStream os;
-            os << name << ":" << this->value << ";";
+            os << name << ":" << this->value << important_str() << ";";
             return os.str();
         }
     }
@@ -175,7 +175,7 @@ SPIScale24::write( guint const flags, SPStyleSrc const &style_src_req, SPIBase c
             return (name + ":inherit;");
         } else {
             Inkscape::CSSOStringStream os;
-            os << name << ":" << SP_SCALE24_TO_FLOAT(this->value) << ";";
+            os << name << ":" << SP_SCALE24_TO_FLOAT(this->value) << important_str() << ";";
             return os.str();
         }
     }
@@ -327,39 +327,41 @@ SPILength::write( guint const flags, SPStyleSrc const &style_src_req, SPIBase co
             Inkscape::CSSOStringStream os;
             switch (this->unit) {
                 case SP_CSS_UNIT_NONE:
-                    os << name << ":" << this->computed << ";";
+                    os << name << ":" << this->computed;
                     break;
                 case SP_CSS_UNIT_PX:
-                    os << name << ":" << this->computed << "px;";
+                    os << name << ":" << this->computed << "px";
                     break;
                 case SP_CSS_UNIT_PT:
-                    os << name << ":" << Inkscape::Util::Quantity::convert(this->computed, "px", "pt") << "pt;";
+                    os << name << ":" << Inkscape::Util::Quantity::convert(this->computed, "px", "pt") << "pt";
                     break;
                 case SP_CSS_UNIT_PC:
-                    os << name << ":" << Inkscape::Util::Quantity::convert(this->computed, "px", "pc") << "pc;";
+                    os << name << ":" << Inkscape::Util::Quantity::convert(this->computed, "px", "pc") << "pc";
                     break;
                 case SP_CSS_UNIT_MM:
-                    os << name << ":" << Inkscape::Util::Quantity::convert(this->computed, "px", "mm") << "mm;";
+                    os << name << ":" << Inkscape::Util::Quantity::convert(this->computed, "px", "mm") << "mm";
                     break;
                 case SP_CSS_UNIT_CM:
-                    os << name << ":" << Inkscape::Util::Quantity::convert(this->computed, "px", "cm") << "cm;";
+                    os << name << ":" << Inkscape::Util::Quantity::convert(this->computed, "px", "cm") << "cm";
                     break;
                 case SP_CSS_UNIT_IN:
-                    os << name << ":" << Inkscape::Util::Quantity::convert(this->computed, "px", "in") << "in;";
+                    os << name << ":" << Inkscape::Util::Quantity::convert(this->computed, "px", "in") << "in";
                     break;
                 case SP_CSS_UNIT_EM:
-                    os << name << ":" << this->value << "em;";
+                    os << name << ":" << this->value << "em";
                     break;
                 case SP_CSS_UNIT_EX:
-                    os << name << ":" << this->value << "ex;";
+                    os << name << ":" << this->value << "ex";
                     break;
                 case SP_CSS_UNIT_PERCENT:
-                    os << name << ":" << (this->value * 100.0) << "%;";
+                    os << name << ":" << (this->value * 100.0) << "%";
                     break;
                 default:
                     /* Invalid */
                     break;
             }
+            os << important_str();
+            os << ";";
             return os.str();
         }
     }
@@ -477,7 +479,7 @@ SPILengthOrNormal::write( guint const flags, SPStyleSrc const &style_src_req, SP
     bool src = (style_src_req == style_src || !(flags & SP_STYLE_FLAG_IFSRC));
     if (should_write(flags, set, dfp, src)) {
         if (this->normal) {
-            return (name + ":normal;");
+            return (name + ":normal" + important_str() + ";");
         } else {
             return SPILength::write(flags, style_src_req, base);
         }
@@ -571,13 +573,15 @@ SPIFontVariationSettings::write( guint const flags, SPStyleSrc const &style_src_
     bool src = (style_src_req == style_src || !(flags & SP_STYLE_FLAG_IFSRC));
     if (should_write(flags, set, dfp, src)) {
         if (this->normal) {
-            return (name + ":normal;");
+            return (name + ":normal" + important_str() + ";");
         } else {
             Inkscape::CSSOStringStream os;
             for (std::map<char*,float>::const_iterator it=axes.begin(); it!=axes.end(); ++it){
                 os << "\"" << it->first << "\" " << it->second << " ";
                 // FIXME: can we avoid the last space char ?
             }
+            os << important_str();
+            os << ";";
             return os.str();
         }
     }
@@ -645,11 +649,11 @@ SPIEnum::write( guint const flags, SPStyleSrc const &style_src_req, SPIBase cons
     bool src = (style_src_req == style_src || !(flags & SP_STYLE_FLAG_IFSRC));
     if (should_write(flags, set, dfp, src)) {
         if (this->inherit) {
-            return (name + ":inherit;");
+            return (name + ":inherit" + important_str() + ";" );
         }
         for (unsigned i = 0; enums[i].key; ++i) {
             if (enums[i].value == static_cast< gint > (this->value) ) {
-                return (name + ":" + enums[i].key + ";");
+                return (name + ":" + enums[i].key + important_str() + ";");
             }
         }
     }
@@ -782,10 +786,10 @@ SPIEnumBits::write( guint const flags, SPStyleSrc const &style_src_req, SPIBase
     bool src = (style_src_req == style_src || !(flags & SP_STYLE_FLAG_IFSRC));
     if (should_write(flags, set, dfp, src)) {
         if (this->inherit) {
-            return (name + ":inherit;");
+            return (name + ":inherit" + important_str() + ";" );
         }
         if (this->value == 0 ) {
-            return (name + ":normal");
+            return (name + ":normal" + important_str() + ";" );
         }
         Glib::ustring return_string = name + ":";
         unsigned j = 1;
@@ -796,6 +800,8 @@ SPIEnumBits::write( guint const flags, SPStyleSrc const &style_src_req, SPIBase
             }
             j *= 2;
         }
+        return_string += important_str();
+        return_string += ";";
         return return_string;
     }
     return Glib::ustring("");
@@ -851,13 +857,13 @@ SPILigatures::write( guint const flags, SPStyleSrc const &style_src_req, SPIBase
     bool src = (style_src_req == style_src || !(flags & SP_STYLE_FLAG_IFSRC));
     if (should_write(flags, set, dfp, src)) {
         if (this->inherit) {
-            return (name + ":inherit;");
+            return (name + ":inherit" + important_str() + ";");
         }
         if (value == SP_CSS_FONT_VARIANT_LIGATURES_NONE ) {
-                return (name + ":none;");
+            return (name + ":none" + important_str() + ";" );
         }
         if (value == SP_CSS_FONT_VARIANT_LIGATURES_NORMAL ) {
-                return (name + ":normal;");
+            return (name + ":normal" + important_str() + ";");
         }
 
         Glib::ustring return_string = name + ":";
@@ -870,6 +876,7 @@ SPILigatures::write( guint const flags, SPStyleSrc const &style_src_req, SPIBase
         if ( !(value & SP_CSS_FONT_VARIANT_LIGATURES_CONTEXTUAL) )
             return_string += "no-contextual ";
         return_string.erase( return_string.size() - 1 );
+        return_string += important_str();
         return_string += ";";
         return return_string;
     }
@@ -950,10 +957,10 @@ SPINumeric::write( guint const flags, SPStyleSrc const &style_src_req, SPIBase c
     bool src = (style_src_req == style_src || !(flags & SP_STYLE_FLAG_IFSRC));
     if (should_write(flags, set, dfp, src)) {
         if (this->inherit) {
-            return (name + ":inherit;");
+            return (name + ":inherit" + important_str() + ";");
         }
         if (value == SP_CSS_FONT_VARIANT_NUMERIC_NORMAL ) {
-                return (name + ":normal;");
+            return (name + ":normal" + important_str() + ";");
         }
 
         Glib::ustring return_string = name + ":";
@@ -974,6 +981,7 @@ SPINumeric::write( guint const flags, SPStyleSrc const &style_src_req, SPIBase c
         if ( value & SP_CSS_FONT_VARIANT_NUMERIC_SLASHED_ZERO )
             return_string += "slashed-zero ";
         return_string.erase( return_string.size() - 1 );
+        return_string += important_str();
         return_string += ";";
         return return_string;
     }
@@ -1019,19 +1027,19 @@ SPIString::write( guint const flags, SPStyleSrc const &style_src_req, SPIBase co
     bool src = (style_src_req == style_src || !(flags & SP_STYLE_FLAG_IFSRC));
     if (should_write(flags, set, dfp, src)) {
         if (this->inherit) {
-            return (name + ":inherit;");
+            return (name + ":inherit" + important_str() + ";");
         } else {
             if( this->value ) {
                 if( name.compare( "font-family" ) == 0 ) {
                     Glib::ustring font_family( this->value );
                     css_font_family_quote( font_family );
-                    return (name + ":" + font_family + ";");
+                    return (name + ":" + font_family + important_str() + ";");
                 } else if( name.compare( "-inkscape-font-specification" ) == 0 ) {
                     Glib::ustring font_spec( this->value );
                     css_quote( font_spec );
-                    return (name + ":" + font_spec + ";");
+                    return (name + ":" + font_spec + important_str() + ";");
                 } else {
-                    return (name + ":" + this->value + ";");
+                    return (name + ":" + this->value + important_str() + ";");
                 }
             }
         }
@@ -1154,7 +1162,7 @@ SPIColor::write( guint const flags, SPStyleSrc const &style_src_req, SPIBase con
         }
 
         if ( !css.str().empty() ) {
-            return (name + ":" + css.str() + ";");
+            return (name + ":" + css.str() + important_str() + ";");
         }
     }
 
@@ -1419,7 +1427,7 @@ SPIPaint::write( guint const flags, SPStyleSrc const &style_src_req, SPIBase con
         }
 
         if ( !css.str().empty() ) {
-            return (name + ":" + css.str() + ";");
+            return (name + ":" + css.str() + important_str() + ";");
         }
     }
 
@@ -1654,7 +1662,7 @@ SPIPaintOrder::write( guint const flags, SPStyleSrc const &style_src_req, SPIBas
                 }
             }
         }
-        return (name + ":" + css.str() + ";");
+        return (name + ":" + css.str() + important_str() + ";");
     }
     return Glib::ustring("");
 }
@@ -1773,10 +1781,10 @@ const Glib::ustring SPIFilter::write( guint const flags, SPStyleSrc const &style
     bool src = (style_src_req == style_src || !(flags & SP_STYLE_FLAG_IFSRC));
     if (should_write(flags, set, dfp, src)) {
         if (this->inherit) {
-            return (name + ":inherit;");
+            return (name + ":inherit" + important_str() + ";");
         } else if(this->href && this->href->getURI()) {
             gchar *uri = this->href->getURI()->toString();
-            Glib::ustring retval = name + ":url(" + uri + ");";
+            Glib::ustring retval = name + ":url(" + uri + ")" + important_str() + ";";
             g_free(uri);
             return retval;
         }
@@ -1922,6 +1930,7 @@ SPIDashArray::write( guint const flags, SPStyleSrc const &style_src_req, SPIBase
                 }
                 os << this->values[i];
             }
+            os << important_str();
             os << ";";
             return os.str();
         }
@@ -2044,7 +2053,7 @@ SPIFontSize::write( guint const flags, SPStyleSrc const &style_src_req, SPIBase
         } else if (this->type == SP_FONT_SIZE_PERCENTAGE) {
             css << (this->value * 100.0) << "%";
         }
-        return (name + ":" + css.str() + ";");
+        return (name + ":" + css.str() + important_str() + ";");
     }
     return Glib::ustring("");
 }
@@ -2412,7 +2421,7 @@ SPIBaselineShift::write( guint const flags, SPStyleSrc const &style_src_req, SPI
         } else if (this->type == SP_BASELINE_SHIFT_PERCENTAGE) {
             css << (this->value * 100.0) << "%";
         }
-        return (name + ":" + css.str() + ";");
+        return (name + ":" + css.str() + important_str() + ";");
     }
     return Glib::ustring("");
 }
@@ -2593,6 +2602,7 @@ SPITextDecorationLine::write( guint const flags, SPStyleSrc const &style_src_req
         } else {
             os << "none";
         }
+        os << important_str();
         os << ";";
         return ( os.str() );
     }
@@ -2729,6 +2739,7 @@ SPITextDecorationStyle::write( guint const flags, SPStyleSrc const &style_src_re
             std::cerr  << "SPITextDecorationStyle::write(): No valid value for property" << std::endl;
             return Glib::ustring("");
         }
+        os << important_str();
         os << ";";
         return ( os.str() );
     }
@@ -2883,6 +2894,7 @@ SPITextDecoration::write( guint const flags, SPStyleSrc const &style_src_req, SP
         } else {
             os << "none";
         }
+        os << important_str();
         os << ";";
         return ( os.str() );
     }
diff --git a/src/style-internal.h b/src/style-internal.h
index c0b2f3e72..d8d2a37bc 100644
--- a/src/style-internal.h
+++ b/src/style-internal.h
@@ -59,6 +59,10 @@ enum SPStyleSrc {
  *   reading in the properties backwards. If a property is already set, it
  *   prevents an earlier property from being read.
  *
+ *   A declaration with an "!important" rule overrides any other declarations (except those that
+ *   also have an "!important" rule). Attributes can not use the "!important" rule and the rule
+ *   is not inherited.
+ *
  *   In order for cascading to work, each element in the tree must be read in from top to bottom
  *   (parent before child). At each step, if a style property is not explicitly set, the property
  *   value is taken from the parent. Some properties have "computed" values that depend on:
@@ -122,6 +126,7 @@ public:
           inherits(inherits),
           set(false),
           inherit(false),
+          important(false),
           style_src(SP_STYLE_SRC_STYLE_PROP), // Default to property, see bug 1662285.
           style(NULL)
     {}
@@ -131,14 +136,43 @@ public:
 
     virtual void read( gchar const *str ) = 0;
     virtual void readIfUnset( gchar const *str, SPStyleSrc const &source = SP_STYLE_SRC_STYLE_PROP ) {
-        if ( !set ) {
-            read( str );
+        if (!str) return;
+
+        bool has_important = false;
+        Glib::ustring stripped = strip_important(str, has_important); // Sets 'has_important'
+
+        // '!important' is invalid on attributes, don't read.
+        if (source == SP_STYLE_SRC_ATTRIBUTE && has_important){
+            return;
+        }
+
+        if ( !set || (has_important && !important) ) {
+            read( stripped.c_str() );
             if ( set ) {
                 style_src = source;
+                if (has_important) {
+                    important = true;
+                }
             }
         }
     }
 
+    Glib::ustring important_str() const {
+        return Glib::ustring(important ? " !important" : "");
+    }
+
+    Glib::ustring strip_important( gchar const *str, bool &important ) {
+        assert (str != NULL);
+        Glib::ustring string = Glib::ustring(str);
+        auto pos = string.rfind( " !important" );
+        important = false;
+        if (pos != std::string::npos) {
+            important = true;
+            string.erase(pos);
+        }
+        return string;
+    }
+
     virtual void readAttribute( Inkscape::XML::Node *repr ) {
         readIfUnset( repr->attribute( name.c_str() ), SP_STYLE_SRC_ATTRIBUTE );
     }
@@ -147,7 +181,7 @@ public:
                                        SPStyleSrc const &style_src_req = SP_STYLE_SRC_STYLE_PROP,
                                        SPIBase const *const base = NULL ) const = 0;
     virtual void clear() {
-        set = false, inherit = false;
+        set = false, inherit = false, important = false;
     }
 
     virtual void cascade( const SPIBase* const parent ) = 0;
@@ -162,6 +196,7 @@ public:
         name        = rhs.name;
         inherits    = rhs.inherits;
         set         = rhs.set;
+        important   = rhs.important;
         inherit     = rhs.inherit;
         style_src   = rhs.style_src;
         style       = rhs.style;
@@ -183,6 +218,7 @@ public:
     unsigned inherits : 1;    // Property inherits by default from parent.
     unsigned set : 1;         // Property has been explicitly set (vs. inherited).
     unsigned inherit : 1;     // Property value set to 'inherit'.
+    unsigned important : 1;   // Property rule 'important' has been explicitly set.
     SPStyleSrc style_src : 2; // Source (attribute, style attribute, style-sheet).
 
   // To do: make private after g_asserts removed
diff --git a/src/style.cpp b/src/style.cpp
index 36d669301..d5bbcd9ce 100644
--- a/src/style.cpp
+++ b/src/style.cpp
@@ -1153,9 +1153,16 @@ SPStyle::_mergeDecl(  CRDeclaration const *const decl, SPStyleSrc const &source
          * convert to string. Alternatively, set from CRTerm directly rather
          * than converting to string.
          */
+
         guchar *const str_value_unsigned = cr_term_to_string(decl->value);
         gchar *const str_value = reinterpret_cast<gchar *>(str_value_unsigned);
-        readIfUnset( prop_idx, str_value, source );
+
+        // Add "!important" rule if necessary as this is not handled by cr_term_to_string().
+        gchar const * important = decl->important ? " !important" : "";
+        Inkscape::CSSOStringStream os;
+        os << str_value << important;
+
+        readIfUnset( prop_idx, os.str().c_str(), source );
         g_free(str_value);
     }
 }