[css3-text] Implement text-decoration property shorthand

Source/core/editing/EditingStyle.cpp

 /*
  * Copyright (C) 2007, 2008, 2009 Apple Computer, Inc.
  * Copyright (C) 2010, 2011 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 40 matching lines @@
 #include "core/html/HTMLFontElement.h"
 #include "core/page/Frame.h"
 #include "core/page/RuntimeCSSEnabled.h"
 #include "core/rendering/style/RenderStyle.h"
 
 namespace WebCore {
 
 // Editing style properties must be preserved during editing operation.
 // e.g. when a user inserts a new paragraph, all properties listed here must be copied to the new paragraph.
 // NOTE: Use editingProperties() to respect runtime enabling of properties.
-static const unsigned nonInheritedStaticPropertiesCount = 2;
+static const unsigned nonInheritedStaticPropertiesCount = 3;
 
 static const CSSPropertyID staticEditingProperties[] = {
-    // NOTE: inheritableEditingProperties depends on these two properties being first.
+    // NOTE: inheritableEditingProperties depends on these properties being first.
     // If you change this list, make sure to update nonInheritedPropertyCount.
     CSSPropertyBackgroundColor,
+    // FIXME: CSSPropertyTextDecoration needs to be removed when CSS3 Text
+    // Decoration feature is no longer experimental (behind a flag).
     CSSPropertyTextDecoration,
+    CSSPropertyTextDecorationLine,
 
     // CSS inheritable properties
     CSSPropertyColor,
     CSSPropertyFontFamily,
     CSSPropertyFontSize,
     CSSPropertyFontStyle,
     CSSPropertyFontVariant,
     CSSPropertyFontWeight,
     CSSPropertyLetterSpacing,
     CSSPropertyLineHeight,
     CSSPropertyOrphans,
     CSSPropertyTextAlign,
     CSSPropertyTextIndent,
     CSSPropertyTextTransform,
     CSSPropertyWhiteSpace,
     CSSPropertyWidows,
     CSSPropertyWordSpacing,
     CSSPropertyWebkitTextDecorationsInEffect,
     CSSPropertyWebkitTextFillColor,
     CSSPropertyWebkitTextStrokeColor,
     CSSPropertyWebkitTextStrokeWidth,
 };
 
 enum EditingPropertiesType { OnlyInheritableEditingProperties, AllEditingProperties };
 
 static const Vector<CSSPropertyID>& allEditingProperties()
 {
     DEFINE_STATIC_LOCAL(Vector<CSSPropertyID>, properties, ());
-    if (properties.isEmpty())
+    if (properties.isEmpty()) {
         RuntimeCSSEnabled::filterEnabledCSSPropertiesIntoVector(staticEditingProperties, WTF_ARRAY_LENGTH(staticEditingProperties), properties);
+        if (RuntimeEnabledFeatures::css3TextDecorationsEnabled())
+            properties.remove(properties.find(CSSPropertyTextDecoration));
+    }
+    ASSERT(RuntimeEnabledFeatures::css3TextDecorationsEnabled() && !properties.contains(CSSPropertyTextDecoration));

[Julien - ping for review, 2013/07/30 22:37:22]

Won't this ASSERT will trigger on Chrome if you disable the new Web Platform
features?

[abinader, 2013/07/31 21:24:03]

Oh, dumb me. Indeed... I also need to make the other way (remove
CSSPropertyTextDecorationLine from properties vector if runtime feature is
disabled) on the code above it.

     return properties;
 }
 
 static const Vector<CSSPropertyID>& inheritableEditingProperties()
 {
     DEFINE_STATIC_LOCAL(Vector<CSSPropertyID>, properties, ());
     if (properties.isEmpty())
         RuntimeCSSEnabled::filterEnabledCSSPropertiesIntoVector(staticEditingProperties + nonInheritedStaticPropertiesCount, WTF_ARRAY_LENGTH(staticEditingProperties) - nonInheritedStaticPropertiesCount, properties);
     return properties;
 }
@@ skipping 88 matching lines @@
         return adoptPtr(new HTMLTextDecorationEquivalent(primitiveValue, tagName));
     }
     virtual bool propertyExistsInStyle(const StylePropertySet*) const;
     virtual bool valueIsPresentInStyle(Element*, StylePropertySet*) const;
 
 private:
     HTMLTextDecorationEquivalent(CSSValueID primitiveValue, const QualifiedName& tagName);
 };
 
 HTMLTextDecorationEquivalent::HTMLTextDecorationEquivalent(CSSValueID primitiveValue, const QualifiedName& tagName)
-    : HTMLElementEquivalent(CSSPropertyTextDecoration, primitiveValue, tagName)
+    : HTMLElementEquivalent(RuntimeEnabledFeatures::css3TextDecorationsEnabled() ? CSSPropertyTextDecorationLine : CSSPropertyTextDecoration, primitiveValue, tagName)
     // m_propertyID is used in HTMLElementEquivalent::addToStyle
 {
 }
 
 bool HTMLTextDecorationEquivalent::propertyExistsInStyle(const StylePropertySet* style) const
 {
+    if (RuntimeEnabledFeatures::css3TextDecorationsEnabled())
+        return style->getPropertyCSSValue(CSSPropertyWebkitTextDecorationsInEffect) || style->getPropertyCSSValue(CSSPropertyTextDecorationLine);
     return style->getPropertyCSSValue(CSSPropertyWebkitTextDecorationsInEffect) || style->getPropertyCSSValue(CSSPropertyTextDecoration);
 }
 
 bool HTMLTextDecorationEquivalent::valueIsPresentInStyle(Element* element, StylePropertySet* style) const
 {
     RefPtr<CSSValue> styleValue = style->getPropertyCSSValue(CSSPropertyWebkitTextDecorationsInEffect);
-    if (!styleValue)
-        styleValue = style->getPropertyCSSValue(CSSPropertyTextDecoration);
+    if (!styleValue) {
+        if (RuntimeEnabledFeatures::css3TextDecorationsEnabled())
+            styleValue = style->getPropertyCSSValue(CSSPropertyTextDecorationLine);
+        else
+            styleValue = style->getPropertyCSSValue(CSSPropertyTextDecoration);
+    }
     return matches(element) && styleValue && styleValue->isValueList() && static_cast<CSSValueList*>(styleValue.get())->hasValue(m_primitiveValue.get());
 }
 
 class HTMLAttributeEquivalent : public HTMLElementEquivalent {
 public:
     static PassOwnPtr<HTMLAttributeEquivalent> create(CSSPropertyID propertyID, const QualifiedName& tagName, const QualifiedName& attrName)
     {
         return adoptPtr(new HTMLAttributeEquivalent(propertyID, tagName, attrName));
     }
     static PassOwnPtr<HTMLAttributeEquivalent> create(CSSPropertyID propertyID, const QualifiedName& attrName)
@@ skipping 412 matching lines @@
 
 void EditingStyle::collapseTextDecorationProperties()
 {
     if (!m_mutableStyle)
         return;
 
     RefPtr<CSSValue> textDecorationsInEffect = m_mutableStyle->getPropertyCSSValue(CSSPropertyWebkitTextDecorationsInEffect);
     if (!textDecorationsInEffect)
         return;
 
-    if (textDecorationsInEffect->isValueList())
-        m_mutableStyle->setProperty(CSSPropertyTextDecoration, textDecorationsInEffect->cssText(), m_mutableStyle->propertyIsImportant(CSSPropertyTextDecoration));
-    else
-        m_mutableStyle->removeProperty(CSSPropertyTextDecoration);
+    if (textDecorationsInEffect->isValueList()) {
+        if (RuntimeEnabledFeatures::css3TextDecorationsEnabled())
+            m_mutableStyle->setProperty(CSSPropertyTextDecorationLine, textDecorationsInEffect->cssText(), m_mutableStyle->propertyIsImportant(CSSPropertyTextDecorationLine));
+        else
+            m_mutableStyle->setProperty(CSSPropertyTextDecoration, textDecorationsInEffect->cssText(), m_mutableStyle->propertyIsImportant(CSSPropertyTextDecoration));
+    } else {
+        if (RuntimeEnabledFeatures::css3TextDecorationsEnabled())
+            m_mutableStyle->removeProperty(CSSPropertyTextDecorationLine);
+        else
+            m_mutableStyle->removeProperty(CSSPropertyTextDecoration);
+    }
     m_mutableStyle->removeProperty(CSSPropertyWebkitTextDecorationsInEffect);
 }
 
 // CSS properties that create a visual difference only when applied to text.
 static const CSSPropertyID textOnlyProperties[] = {
+    // FIXME: CSSPropertyTextDecoration needs to be removed when CSS3 Text
+    // Decoration feature is no longer experimental (behind a runtime flag).
     CSSPropertyTextDecoration,
+    CSSPropertyTextDecorationLine,
     CSSPropertyWebkitTextDecorationsInEffect,
     CSSPropertyFontStyle,
     CSSPropertyFontWeight,
     CSSPropertyColor,
 };
 
 TriState EditingStyle::triStateOfStyle(EditingStyle* style) const
 {
     if (!style || !style->m_mutableStyle)
         return FalseTriState;
@@ skipping 52 matching lines @@
         return false;
 
     unsigned propertyCount = m_mutableStyle->propertyCount();
     for (unsigned i = 0; i < propertyCount; ++i) {
         CSSPropertyID propertyID = m_mutableStyle->propertyAt(i).id();
 
         // We don't override whitespace property of a tab span because that would collapse the tab into a space.
         if (propertyID == CSSPropertyWhiteSpace && isTabSpanNode(element))
             continue;
 
-        if (propertyID == CSSPropertyWebkitTextDecorationsInEffect && inlineStyle->getPropertyCSSValue(CSSPropertyTextDecoration)) {
-            if (!conflictingProperties)
-                return true;
-            conflictingProperties->append(CSSPropertyTextDecoration);
-            if (extractedStyle)
-                extractedStyle->setProperty(CSSPropertyTextDecoration, inlineStyle->getPropertyValue(CSSPropertyTextDecoration), inlineStyle->propertyIsImportant(CSSPropertyTextDecoration));
-            continue;
+        if (propertyID == CSSPropertyWebkitTextDecorationsInEffect) {
+            if (RuntimeEnabledFeatures::css3TextDecorationsEnabled()) {
+                if (inlineStyle->getPropertyCSSValue(CSSPropertyTextDecorationLine)) {
+                    if (!conflictingProperties)
+                        return true;
+                    conflictingProperties->append(CSSPropertyTextDecoration);
+                    conflictingProperties->append(CSSPropertyTextDecorationLine);
+                    if (extractedStyle)
+                        extractedStyle->setProperty(CSSPropertyTextDecorationLine, inlineStyle->getPropertyValue(CSSPropertyTextDecorationLine), inlineStyle->propertyIsImportant(CSSPropertyTextDecorationLine));
+                    continue;
+                }
+            } else {
+                if (inlineStyle->getPropertyCSSValue(CSSPropertyTextDecoration)) {
+                    if (!conflictingProperties)
+                        return true;
+                    conflictingProperties->append(CSSPropertyTextDecoration);
+                    if (extractedStyle)
+                        extractedStyle->setProperty(CSSPropertyTextDecoration, inlineStyle->getPropertyValue(CSSPropertyTextDecoration), inlineStyle->propertyIsImportant(CSSPropertyTextDecoration));
+                    continue;
+                }

[Julien - ping for review, 2013/07/30 22:37:22]

These 2 branches are really the same if you had a variable for the
CSSPropertyID:

CSSPropertyID textDecorationPropertyID =
RuntimeEnabledFeatures::css3TextDecorationsEnabled() ?
CSSPropertyTextDecorationLine : CSSPropertyTextDecoration;
...

Most of the code change in this file would actually be removed / simplified
greatly if you just stored this CSSPropertyID in EditingStyle and used this
variable instead of the hardcoded CSSPropertyTextDecoration.

[abinader, 2013/07/31 21:24:03]

Sounds good, I thought about it but wanted to highlight every place where we are
modifying the code for clarity. I'm working on this idea atm.

+            }
         }
 
         if (!inlineStyle->getPropertyCSSValue(propertyID))
             continue;
 
         if (propertyID == CSSPropertyUnicodeBidi && inlineStyle->getPropertyCSSValue(CSSPropertyDirection)) {
             if (!conflictingProperties)
                 return true;
             conflictingProperties->append(CSSPropertyDirection);
             if (extractedStyle)
@@ skipping 319 matching lines @@
         m_mutableStyle = style->mutableCopy();
         return;
     }
 
     unsigned propertyCount = style->propertyCount();
     for (unsigned i = 0; i < propertyCount; ++i) {
         StylePropertySet::PropertyReference property = style->propertyAt(i);
         RefPtr<CSSValue> value = m_mutableStyle->getPropertyCSSValue(property.id());
 
         // text decorations never override values
-        if ((property.id() == CSSPropertyTextDecoration || property.id() == CSSPropertyWebkitTextDecorationsInEffect) && property.value()->isValueList() && value) {
+        bool isPropertyTextDecorationEquivalent = property.id() == CSSPropertyWebkitTextDecorationsInEffect;
+        if (RuntimeEnabledFeatures::css3TextDecorationsEnabled())
+            isPropertyTextDecorationEquivalent |= property.id() == CSSPropertyTextDecorationLine;
+        else
+            isPropertyTextDecorationEquivalent |= property.id() == CSSPropertyTextDecoration;
+        if (isPropertyTextDecorationEquivalent && property.value()->isValueList() && value) {
             if (value->isValueList()) {
                 mergeTextDecorationValues(static_cast<CSSValueList*>(value.get()), static_cast<CSSValueList*>(property.value()));
                 continue;
             }
             value = 0; // text-decoration: none is equivalent to not having the property
         }
 
         if (mode == OverrideValues || (mode == DoNotOverrideValues && !value))
             m_mutableStyle->setProperty(property.id(), property.value()->cssText(), property.isImportant());
     }
@@ skipping 235 matching lines @@
         foundDirection = directionValue == CSSValueLtr ? LeftToRightWritingDirection : RightToLeftWritingDirection;
     }
     hasNestedOrMultipleEmbeddings = false;
     return foundDirection;
 }
 
 static void reconcileTextDecorationProperties(MutableStylePropertySet* style)
 {
     RefPtr<CSSValue> textDecorationsInEffect = style->getPropertyCSSValue(CSSPropertyWebkitTextDecorationsInEffect);
     RefPtr<CSSValue> textDecoration = style->getPropertyCSSValue(CSSPropertyTextDecoration);
+    RefPtr<CSSValue> textDecorationLine = style->getPropertyCSSValue(CSSPropertyTextDecorationLine);
     // We shouldn't have both text-decoration and -webkit-text-decorations-in-effect because that wouldn't make sense.
-    ASSERT(!textDecorationsInEffect || !textDecoration);
+    ASSERT(!textDecorationsInEffect || (!textDecoration || !textDecorationLine));
     if (textDecorationsInEffect) {
-        style->setProperty(CSSPropertyTextDecoration, textDecorationsInEffect->cssText());
+        if (RuntimeEnabledFeatures::css3TextDecorationsEnabled()) {
+            style->setProperty(CSSPropertyTextDecorationLine, textDecorationsInEffect->cssText());
+            textDecorationLine = textDecorationsInEffect;
+        } else {
+            style->setProperty(CSSPropertyTextDecoration, textDecorationsInEffect->cssText());
+            textDecoration = textDecorationsInEffect;
+        }
         style->removeProperty(CSSPropertyWebkitTextDecorationsInEffect);
-        textDecoration = textDecorationsInEffect;
     }
 
     // If text-decoration is set to "none", remove the property because we don't want to add redundant "text-decoration: none".
-    if (textDecoration && !textDecoration->isValueList())
-        style->removeProperty(CSSPropertyTextDecoration);
+    if (RuntimeEnabledFeatures::css3TextDecorationsEnabled()) {
+        if (textDecorationLine && !textDecorationLine->isValueList())
+            style->removeProperty(CSSPropertyTextDecorationLine);
+    } else {
+        if (textDecoration && !textDecoration->isValueList())
+            style->removeProperty(CSSPropertyTextDecoration);
+    }
 }
 
 StyleChange::StyleChange(EditingStyle* style, const Position& position)
     : m_applyBold(false)
     , m_applyItalic(false)
     , m_applyUnderline(false)
     , m_applyLineThrough(false)
     , m_applySubscript(false)
     , m_applySuperscript(false)
 {
@@ skipping 43 matching lines @@
     }
 
     int fontStyle = getIdentifierValue(style, CSSPropertyFontStyle);
     if (fontStyle == CSSValueItalic || fontStyle == CSSValueOblique) {
         style->removeProperty(CSSPropertyFontStyle);
         m_applyItalic = true;
     }
 
     // Assuming reconcileTextDecorationProperties has been called, there should not be -webkit-text-decorations-in-effect
     // Furthermore, text-decoration: none has been trimmed so that text-decoration property is always a CSSValueList.
-    RefPtr<CSSValue> textDecoration = style->getPropertyCSSValue(CSSPropertyTextDecoration);
+    CSSPropertyID textDecorationProperty = RuntimeEnabledFeatures::css3TextDecorationsEnabled() ? CSSPropertyTextDecorationLine : CSSPropertyTextDecoration;
+    RefPtr<CSSValue> textDecoration = style->getPropertyCSSValue(textDecorationProperty);
     if (textDecoration && textDecoration->isValueList()) {
         DEFINE_STATIC_LOCAL(RefPtr<CSSPrimitiveValue>, underline, (CSSPrimitiveValue::createIdentifier(CSSValueUnderline)));
         DEFINE_STATIC_LOCAL(RefPtr<CSSPrimitiveValue>, lineThrough, (CSSPrimitiveValue::createIdentifier(CSSValueLineThrough)));
 
         RefPtr<CSSValueList> newTextDecoration = static_cast<CSSValueList*>(textDecoration.get())->copy();
         if (newTextDecoration->removeAll(underline.get()))
             m_applyUnderline = true;
         if (newTextDecoration->removeAll(lineThrough.get()))
             m_applyLineThrough = true;
 
         // If trimTextDecorations, delete underline and line-through
-        setTextDecorationProperty(style, newTextDecoration.get(), CSSPropertyTextDecoration);
+        setTextDecorationProperty(style, newTextDecoration.get(), textDecorationProperty);
     }
 
     int verticalAlign = getIdentifierValue(style, CSSPropertyVerticalAlign);
     switch (verticalAlign) {
     case CSSValueSub:
         style->removeProperty(CSSPropertyVerticalAlign);
         m_applySubscript = true;
         break;
     case CSSValueSuper:
         style->removeProperty(CSSPropertyVerticalAlign);
@@ skipping 84 matching lines @@
 
 PassRefPtr<MutableStylePropertySet> getPropertiesNotIn(StylePropertySet* styleWithRedundantProperties, CSSStyleDeclaration* baseStyle)
 {
     ASSERT(styleWithRedundantProperties);
     ASSERT(baseStyle);
     RefPtr<MutableStylePropertySet> result = styleWithRedundantProperties->mutableCopy();
 
     result->removeEquivalentProperties(baseStyle);
 
     RefPtr<CSSValue> baseTextDecorationsInEffect = baseStyle->getPropertyCSSValueInternal(CSSPropertyWebkitTextDecorationsInEffect);
-    diffTextDecorations(result.get(), CSSPropertyTextDecoration, baseTextDecorationsInEffect.get());
+    if (RuntimeEnabledFeatures::css3TextDecorationsEnabled())
+        diffTextDecorations(result.get(), CSSPropertyTextDecorationLine, baseTextDecorationsInEffect.get());
+    else
+        diffTextDecorations(result.get(), CSSPropertyTextDecoration, baseTextDecorationsInEffect.get());
     diffTextDecorations(result.get(), CSSPropertyWebkitTextDecorationsInEffect, baseTextDecorationsInEffect.get());
 
     if (baseStyle->getPropertyCSSValueInternal(CSSPropertyFontWeight) && fontWeightIsBold(result.get()) == fontWeightIsBold(baseStyle))
         result->removeProperty(CSSPropertyFontWeight);
 
     if (baseStyle->getPropertyCSSValueInternal(CSSPropertyColor) && getRGBAFontColor(result.get()) == getRGBAFontColor(baseStyle))
         result->removeProperty(CSSPropertyColor);
 
     if (baseStyle->getPropertyCSSValueInternal(CSSPropertyTextAlign)
         && textAlignResolvingStartAndEnd(result.get()) == textAlignResolvingStartAndEnd(baseStyle))
@@ skipping 77 matching lines @@
 {
     for (Node* ancestor = node; ancestor; ancestor = ancestor->parentNode()) {
         RefPtr<CSSComputedStyleDeclaration> ancestorStyle = CSSComputedStyleDeclaration::create(ancestor);
         if (!hasTransparentBackgroundColor(ancestorStyle.get()))
             return ancestorStyle->getPropertyCSSValue(CSSPropertyBackgroundColor);
     }
     return 0;
 }
 
 }