[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,
     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));
+    }
     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 203 matching lines @@
         node = tabSpanNode(node)->parentNode();
     else if (isTabSpanNode(node))
         node = node->parentNode();
 
     RefPtr<CSSComputedStyleDeclaration> computedStyleAtPosition = CSSComputedStyleDeclaration::create(node);
     m_mutableStyle = propertiesToInclude == AllProperties && computedStyleAtPosition ? computedStyleAtPosition->copyProperties() : editingStyleFromComputedStyle(computedStyleAtPosition);
 
     if (propertiesToInclude == EditingPropertiesInEffect) {
         if (RefPtr<CSSValue> value = backgroundColorInEffect(node))
             m_mutableStyle->setProperty(CSSPropertyBackgroundColor, value->cssText());
-        if (RefPtr<CSSValue> value = computedStyleAtPosition->getPropertyCSSValue(CSSPropertyWebkitTextDecorationsInEffect))
+        if (RefPtr<CSSValue> value = computedStyleAtPosition->getPropertyCSSValue(CSSPropertyWebkitTextDecorationsInEffect)) {
             m_mutableStyle->setProperty(CSSPropertyTextDecoration, value->cssText());
+            if (RuntimeEnabledFeatures::css3TextDecorationsEnabled())
+                m_mutableStyle->setProperty(CSSPropertyTextDecorationLine, value->cssText());
+        }
     }
 
     if (node && node->computedStyle()) {
         RenderStyle* renderStyle = node->computedStyle();
         removeTextFillAndStrokeColorsIfNeeded(renderStyle);
         replaceFontSizeByKeywordIfPossible(renderStyle, computedStyleAtPosition.get());
     }
 
     m_shouldUseFixedDefaultFontSize = computedStyleAtPosition->useFixedFontDefaultSize();
     extractFontSizeDelta();
@@ skipping 187 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
+    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);
         m_mutableStyle->removeProperty(CSSPropertyTextDecoration);

[Julien - ping for review, 2013/07/29 18:53:12]

This should be in an 'else': shorthands are expanded at parsing time so we
shouldn't see them here.

[abinader, 2013/07/29 21:15:32]

You are right, I probably overlooked at it.

+    }
     m_mutableStyle->removeProperty(CSSPropertyWebkitTextDecorationsInEffect);
 }
 
 // CSS properties that create a visual difference only when applied to text.
 static const CSSPropertyID textOnlyProperties[] = {
     CSSPropertyTextDecoration,

[Julien - ping for review, 2013/07/29 18:53:12]

Note that once we remove RuntimeEnabledFeatures::css3TextDecorationsEnabled()
this should be removed from the list as we should never see a shorthand.

[abinader, 2013/07/29 21:15:32]

Ack, I'll add a FIXME: Needs to be removed when CSS3 Text Decoration feature is
no longer experimental.

+    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) {

[Julien - ping for review, 2013/07/29 18:53:12]

Probably overstepping my knowledge of editing here but
WebKitTextDecorationInEffect seems very much unused in our metrics. The extra
complexity from it seems nasty so maybe we could remove it and simplify this CL?
(it shouldn't block this CL though and could be removed as a follow-up CL but I
would prefer to do this change before)

[abinader, 2013/07/29 21:15:32]

Yes, totally agreed upon removing this somewhat hackish property - however, I'm
afraid due to its complexity it would be better to postpone this task to be done
after this patch gets landed (as we dicussed on IRC). 

+            if (RuntimeEnabledFeatures::css3TextDecorationsEnabled()) {
+                RefPtr<CSSValue> textDecorationLine = inlineStyle->getPropertyCSSValue(CSSPropertyTextDecorationLine);

[Julien - ping for review, 2013/07/29 18:53:12]

This is usually inlined into the if () statement as you only need
|textDecorationLine| inside the branch.

[abinader, 2013/07/29 21:15:32]

Indeed, makes code looks sharper as well :)

+                if (textDecorationLine) {
+                    if (!conflictingProperties)
+                        return true;
+                    conflictingProperties->append(CSSPropertyTextDecoration);
+                    conflictingProperties->append(CSSPropertyTextDecorationLine);
+                    if (extractedStyle)
+                        extractedStyle->setProperty(CSSPropertyTextDecorationLine, inlineStyle->getPropertyValue(CSSPropertyTextDecorationLine), inlineStyle->propertyIsImportant(CSSPropertyTextDecorationLine));
+                    continue;
+                }
+            } else {
+                RefPtr<CSSValue> textDecoration = inlineStyle->getPropertyCSSValue(CSSPropertyTextDecoration);
+                if (textDecoration) {
+                    if (!conflictingProperties)
+                        return true;
+                    conflictingProperties->append(CSSPropertyTextDecoration);
+                    if (extractedStyle)
+                        extractedStyle->setProperty(CSSPropertyTextDecoration, inlineStyle->getPropertyValue(CSSPropertyTextDecoration), inlineStyle->propertyIsImportant(CSSPropertyTextDecoration));
+                    continue;
+                }
+            }
         }
 
         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;
 }
 
 }