Changeset 183770 in webkit

Timestamp:

May 4, 2015 1:42:41 PM (9 years ago)

Author:

rniwa@webkit.org

Message:

Toggling underline or strike through affects each other
​https://bugs.webkit.org/show_bug.cgi?id=27818

Reviewed by Darin Adler.

Source/WebCore:

This patch introduces a new mechanism to apply and remove text decorations. This is necessary because text
decorations are always additive and we can't differentiate whether we're adding or removing a text decoration.
Conceptually, we need four values for text decorations: adding underline, removing underline, adding
line-through, and removing line-through but we have only three: underline, line-through, none.

After this patch, there are three mechanism by which text decorations states are kept tracked. While applying
or removing text decorations, we use newly added m_underlineChange and m_strikeThroughChange in EditingStyle.
For the typing style, we use -webkit-text-decorations-in-effect to store the state since we need to preserve
every type of text decorations such as overline in addition to underline and line-through. Once applied, all
text decorations should be expressed in terms of the standard text-decoration property.

Test: editing/execCommand/toggle-mixed-text-decorations.html

• editing/ApplyStyleCommand.cpp:

(WebCore::ApplyStyleCommand::applyBlockStyle):
(WebCore::ApplyStyleCommand::removeCSSStyle): conflictsWithInlineStyleOfElement now creates a new inline style
instead of a list of properties to remove.
(WebCore::ApplyStyleCommand::addBlockStyle):
(WebCore::ApplyStyleCommand::applyInlineStyleChange): Merge inline styles instead of adding as string.
Otherwise it would generate style content attribute with multiple text-decoration properties.

• editing/EditingStyle.cpp:

(WebCore::HTMLElementEquivalent::matches):
(WebCore::HTMLElementEquivalent::propertyExistsInStyle): Takes an EditingStyle instead of StyleProperties.
(WebCore::HTMLElementEquivalent::valueIsPresentInStyle):
(WebCore::HTMLTextDecorationEquivalent::HTMLTextDecorationEquivalent):
(WebCore::HTMLTextDecorationEquivalent::propertyExistsInStyle): Respect newly added m_strikeThroughChange and
m_underlineChange in EditingStyle.
(WebCore::HTMLTextDecorationEquivalent::valueIsPresentInStyle): Ditto.
(WebCore::HTMLTextDecorationEquivalent::changeInStyle): Added. Retrieves the change enum for the associated
type of text-decoration (underline or strike through).
(WebCore::HTMLAttributeEquivalent::matches):
(WebCore::HTMLAttributeEquivalent::valueIsPresentInStyle):
(WebCore::EditingStyle::EditingStyle): Initialize m_underlineChange and m_strikeThroughChange. Also use the
delegating constructor elsewhere. Also added the missing call to extractFontSizeDelta() in the variant that
takes CSSPropertyID and String, and added a variant that takes CSSPropertyID and CSSValueID.
(WebCore::EditingStyle::isEmpty): Return false when m_underlineChange and m_strikeThroughChange are not "none".
(WebCore::applyTextDecorationChangeToValueList): Added.
(WebCore::EditingStyle::overrideTypingStyleAt): Added. Used by Editor::computeAndSetTypingStyle to set a new
typing style. Resolve m_underlineChange and m_strikeThroughChange into -webkit-text-decorations-in-effect.
(WebCore::EditingStyle::clear): Clear m_underlineChange and m_strikeThroughChange.
(WebCore::EditingStyle::copy): Copy m_underlineChange and m_strikeThroughChange.
(WebCore::textDecorationValueList): Added.
(WebCore::EditingStyle::conflictsWithInlineStyleOfElement): Now takes a pointer to MutableStyleProperties
instead of a vector. This was necessary we can't simply remove text-decoration property in ApplyStyleCommand's
removeCSSStyle as that would result in unrelated text decorations also getting removed. Also added the code
for m_underlineChange and m_strikeThroughChange. Only removing text decoration changes can cause a conflict
since text decorations are always additive.
(WebCore::EditingStyle::conflictsWithImplicitStyleOfElement): Check isEmpty() instead of the nullity of
m_mutableStyle to respect m_underlineChange and m_strikeThroughChange.
(WebCore::EditingStyle::conflictsWithImplicitStyleOfAttributes):
(WebCore::EditingStyle::extractConflictingImplicitStyleOfAttributes):
(WebCore::EditingStyle::styleIsPresentInComputedStyleOfNode): Respect the values of m_underlineChange and
m_strikeThroughChange. Here, the style is considered present if it has text decorations that are being added.
(WebCore::EditingStyle::elementIsStyledSpanOrHTMLEquivalent):
(WebCore::elementMatchesAndPropertyIsNotInInlineStyleDecl): Takes EditingStyle instead of StyleProperties to
respect m_underlineChange and m_strikeThroughChange.
(WebCore::EditingStyle::mergeInlineAndImplicitStyleOfElement):
(WebCore::mergeTextDecorationValues):
(WebCore::EditingStyle::mergeStyle): Make a copy of CSSValueList before modifying it since CSSValueList's are
shared with other immutable StyleProperties.
(WebCore::StyleChange::StyleChange): Set m_applyUnderline, m_applyLineThrough, and m_cssStyle if either
m_underlineChange or m_strikeThroughChange are TextDecorationChange::Add in EditingStyle if the current position
doesn't already have the matching style.
(WebCore::StyleChange::operator==): Moved from the header file. Also added the logic to compare m_cssStyle now
that it's a StyleProperties instead of String.

• editing/EditingStyle.h: Added TextDecorationChange.

(WebCore::EditingStyle::create): Added a variant that takes CSSPropertyID and CSSValueID.
(WebCore::EditingStyle::conflictsWithInlineStyleOfElement):
(WebCore::EditingStyle::setUnderlineChange): Added.
(WebCore::EditingStyle::underlineChange): Added.
(WebCore::EditingStyle::setStrikeThroughChange): Added.
(WebCore::EditingStyle::strikeThroughChange): Added.
(WebCore::StyleChange::cssStyle): Now returns StyleProperties* instead of String so that ApplyStyleCommand's
applyInlineStyleChange could merge inline styles instead of just appending it to the end.
(WebCore::StyleChange::operator==): Moved into the cpp file.

• editing/Editor.cpp:

(WebCore::Editor::applyStyle): Added. This variant takes EditingStyle instead of StyleProperties.
(WebCore::Editor::applyStyleToSelection): Ditto.
(WebCore::Editor::computeAndSetTypingStyle): Added a variant for EditingStyle. Also use overrideTypingStyleAt
to set -webkit-text-decorations-in-effect based on m_underlineChange and m_strikeThroughChange

• editing/Editor.h:
• editing/EditorCommand.cpp:

(WebCore::applyCommandToFrame):
(WebCore::isStylePresent): Extracted from executeToggleStyle.
(WebCore::executeApplyStyle):
(WebCore::executeToggleStyle):
(WebCore::executeToggleStyleInList): Deleted.
(WebCore::textDecorationChangeForToggling): Added. Used in executeStrikethrough and executeUnderline.
(WebCore::executeStrikethrough):
(WebCore::executeUnderline):

Source/WebKit/mac:

• WebView/WebFrame.mm:

(-[WebFrame _setTypingStyle:withUndoAction:]):

LayoutTests:

Added a regression test and rebaselined various tests as explained below.

• editing/execCommand/script-tests/toggle-style-2.js: The order in which u and strike elements appear have switched.
• editing/execCommand/script-tests/toggle-text-decorations.js: Ditto for line-through and overline.
• editing/execCommand/toggle-mixed-text-decorations-expected.txt: Added.
• editing/execCommand/toggle-mixed-text-decorations.html: Added.
• editing/execCommand/toggle-style-2-expected.txt: Rebaselined.
• editing/execCommand/toggle-text-decorations-expected.txt: Rebaselined.
• editing/undo/remove-css-property-and-remove-style-expected.txt: The order in which color and font-weight properties

appear have switched.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/editing/execCommand/script-tests/toggle-style-2.js (modified) (1 diff)
• LayoutTests/editing/execCommand/script-tests/toggle-text-decorations.js (modified) (1 diff)
• LayoutTests/editing/execCommand/toggle-mixed-text-decorations-expected.txt (added)
• LayoutTests/editing/execCommand/toggle-mixed-text-decorations.html (added)
• LayoutTests/editing/execCommand/toggle-style-2-expected.txt (modified) (1 diff)
• LayoutTests/editing/execCommand/toggle-text-decorations-expected.txt (modified) (1 diff)
• LayoutTests/editing/undo/remove-css-property-and-remove-style-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/editing/ApplyStyleCommand.cpp (modified) (5 diffs)
• Source/WebCore/editing/EditingStyle.cpp (modified) (31 diffs)
• Source/WebCore/editing/EditingStyle.h (modified) (10 diffs)
• Source/WebCore/editing/Editor.cpp (modified) (5 diffs)
• Source/WebCore/editing/Editor.h (modified) (2 diffs)
• Source/WebCore/editing/EditorCommand.cpp (modified) (4 diffs)
• Source/WebKit/mac/ChangeLog (modified) (1 diff)
• Source/WebKit/mac/WebView/WebFrame.mm (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2015-05-04  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Toggling underline or strike through affects each other
+        https://bugs.webkit.org/show_bug.cgi?id=27818
+
+        Reviewed by Darin Adler.
+
+        Added a regression test and rebaselined various tests as explained below.
+
+        * editing/execCommand/script-tests/toggle-style-2.js: The order in which u and strike elements appear have switched.
+        * editing/execCommand/script-tests/toggle-text-decorations.js: Ditto for line-through and overline.
+        * editing/execCommand/toggle-mixed-text-decorations-expected.txt: Added.
+        * editing/execCommand/toggle-mixed-text-decorations.html: Added.
+        * editing/execCommand/toggle-style-2-expected.txt: Rebaselined.
+        * editing/execCommand/toggle-text-decorations-expected.txt: Rebaselined.
+        * editing/undo/remove-css-property-and-remove-style-expected.txt: The order in which color and font-weight properties
+        appear have switched.
+
 2015-05-04  Chris Dumez  <cdumez@apple.com>
 

trunk/LayoutTests/editing/execCommand/script-tests/toggle-style-2.js

@@ -38 +38 @@
 
 testSingleToggle("strikethrough", "<u>test</u>", "<u><strike>test</strike></u>");
-testSingleToggle("underline", "<strike>test</strike>", "<u><strike>test</strike></u>");
+testSingleToggle("underline", "<strike>test</strike>", "<strike><u>test</u></strike>");
 
 testSingleToggle("strikethrough", '<span style="text-decoration: overline;">test</span>', '<span style="text-decoration: overline;"><strike>test</strike></span>');

trunk/LayoutTests/editing/execCommand/script-tests/toggle-text-decorations.js

@@ -20 +20 @@
 testSingleToggle("underline", "test", "<span style=\"text-decoration: underline;\">test</span>");
 testSingleToggle("underline", "<span style=\"text-decoration: underline;\">test</span>", "test");
-testSingleToggle("underline", "<span style=\"text-decoration: underline line-through overline;\">test</span>", "<span style=\"text-decoration: overline line-through;\">test</span>");
+testSingleToggle("underline", "<span style=\"text-decoration: underline line-through overline;\">test</span>",
+    "<span style=\"text-decoration: line-through overline;\">test</span>");
 testSingleToggle("strikethrough", "test", "<span style=\"text-decoration: line-through;\">test</span>");
 testSingleToggle("strikethrough", "<span style=\"text-decoration: line-through;\">test</span>", "test");

trunk/LayoutTests/editing/execCommand/toggle-style-2-expected.txt

@@ -11 +11 @@
 PASS two strikethrough commands converted test to test
 PASS one strikethrough command converted <u>test</u> to <u><strike>test</strike></u>
-PASS one underline command converted <strike>test</strike> to <u><strike>test</strike></u>
+PASS one underline command converted <strike>test</strike> to <strike><u>test</u></strike>
 PASS one strikethrough command converted <span style="text-decoration: overline;">test</span> to <span style="text-decoration: overline;"><strike>test</strike></span>
 PASS one underline command converted <span style="text-decoration: overline;">test</span> to <span style="text-decoration: overline;"><u>test</u></span>

trunk/LayoutTests/editing/execCommand/toggle-text-decorations-expected.txt

@@ -6 +6 @@
 PASS one underline command converted test to <span style="text-decoration: underline;">test</span>
 PASS one underline command converted <span style="text-decoration: underline;">test</span> to test
-PASS one underline command converted <span style="text-decoration: underline line-through overline;">test</span> to <span style="text-decoration: overline line-through;">test</span>
+PASS one underline command converted <span style="text-decoration: underline line-through overline;">test</span> to <span style="text-decoration: line-through overline;">test</span>
 PASS one strikethrough command converted test to <span style="text-decoration: line-through;">test</span>
 PASS one strikethrough command converted <span style="text-decoration: line-through;">test</span> to test

trunk/LayoutTests/editing/undo/remove-css-property-and-remove-style-expected.txt

@@ -60 +60 @@
 Undo should reset the style attribute so that "test" is both bold and blue:
 | <span>
-|   style="color: blue; font-weight: 900;"
+|   style="font-weight: 900; color: blue;"
 |   "<#selection-anchor>test<#selection-focus>"
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-05-04  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Toggling underline or strike through affects each other
+        https://bugs.webkit.org/show_bug.cgi?id=27818
+
+        Reviewed by Darin Adler.
+
+        This patch introduces a new mechanism to apply and remove text decorations. This is necessary because text
+        decorations are always additive and we can't differentiate whether we're adding or removing a text decoration.
+        Conceptually, we need four values for text decorations: adding underline, removing underline, adding
+        line-through, and removing line-through but we have only three: underline, line-through, none.
+
+        After this patch, there are three mechanism by which text decorations states are kept tracked. While applying
+        or removing text decorations, we use newly added m_underlineChange and m_strikeThroughChange in EditingStyle.
+        For the typing style, we use -webkit-text-decorations-in-effect to store the state since we need to preserve
+        every type of text decorations such as overline in addition to underline and line-through. Once applied, all
+        text decorations should be expressed in terms of the standard text-decoration property.
+
+        Test: editing/execCommand/toggle-mixed-text-decorations.html
+
+        * editing/ApplyStyleCommand.cpp:
+        (WebCore::ApplyStyleCommand::applyBlockStyle):
+        (WebCore::ApplyStyleCommand::removeCSSStyle): conflictsWithInlineStyleOfElement now creates a new inline style
+        instead of a list of properties to remove.
+        (WebCore::ApplyStyleCommand::addBlockStyle):
+        (WebCore::ApplyStyleCommand::applyInlineStyleChange): Merge inline styles instead of adding as string.
+        Otherwise it would generate style content attribute with multiple text-decoration properties.
+
+        * editing/EditingStyle.cpp:
+        (WebCore::HTMLElementEquivalent::matches):
+        (WebCore::HTMLElementEquivalent::propertyExistsInStyle): Takes an EditingStyle instead of StyleProperties.
+        (WebCore::HTMLElementEquivalent::valueIsPresentInStyle):
+        (WebCore::HTMLTextDecorationEquivalent::HTMLTextDecorationEquivalent): 
+        (WebCore::HTMLTextDecorationEquivalent::propertyExistsInStyle): Respect newly added m_strikeThroughChange and
+        m_underlineChange in EditingStyle.
+        (WebCore::HTMLTextDecorationEquivalent::valueIsPresentInStyle): Ditto.
+        (WebCore::HTMLTextDecorationEquivalent::changeInStyle): Added. Retrieves the change enum for the associated
+        type of text-decoration (underline or strike through).
+        (WebCore::HTMLAttributeEquivalent::matches):
+        (WebCore::HTMLAttributeEquivalent::valueIsPresentInStyle):
+        (WebCore::EditingStyle::EditingStyle): Initialize m_underlineChange and m_strikeThroughChange. Also use the
+        delegating constructor elsewhere. Also added the missing call to extractFontSizeDelta() in the variant that
+        takes CSSPropertyID and String, and added a variant that takes CSSPropertyID and CSSValueID.
+        (WebCore::EditingStyle::isEmpty): Return false when m_underlineChange and m_strikeThroughChange are not "none".
+        (WebCore::applyTextDecorationChangeToValueList): Added.
+        (WebCore::EditingStyle::overrideTypingStyleAt): Added. Used by Editor::computeAndSetTypingStyle to set a new
+        typing style. Resolve m_underlineChange and m_strikeThroughChange into -webkit-text-decorations-in-effect.
+        (WebCore::EditingStyle::clear): Clear m_underlineChange and m_strikeThroughChange.
+        (WebCore::EditingStyle::copy): Copy m_underlineChange and m_strikeThroughChange.
+        (WebCore::textDecorationValueList): Added.
+        (WebCore::EditingStyle::conflictsWithInlineStyleOfElement): Now takes a pointer to MutableStyleProperties
+        instead of a vector. This was necessary we can't simply remove text-decoration property in ApplyStyleCommand's
+        removeCSSStyle as that would result in unrelated text decorations also getting removed. Also added the code
+        for m_underlineChange and m_strikeThroughChange. Only removing text decoration changes can cause a conflict
+        since text decorations are always additive.
+        (WebCore::EditingStyle::conflictsWithImplicitStyleOfElement): Check isEmpty() instead of the nullity of
+        m_mutableStyle to respect m_underlineChange and m_strikeThroughChange.
+        (WebCore::EditingStyle::conflictsWithImplicitStyleOfAttributes):
+        (WebCore::EditingStyle::extractConflictingImplicitStyleOfAttributes):
+        (WebCore::EditingStyle::styleIsPresentInComputedStyleOfNode): Respect the values of m_underlineChange and
+        m_strikeThroughChange. Here, the style is considered present if it has text decorations that are being added.
+        (WebCore::EditingStyle::elementIsStyledSpanOrHTMLEquivalent):
+        (WebCore::elementMatchesAndPropertyIsNotInInlineStyleDecl): Takes EditingStyle instead of StyleProperties to
+        respect m_underlineChange and m_strikeThroughChange.
+        (WebCore::EditingStyle::mergeInlineAndImplicitStyleOfElement):
+        (WebCore::mergeTextDecorationValues):
+        (WebCore::EditingStyle::mergeStyle): Make a copy of CSSValueList before modifying it since CSSValueList's are
+        shared with other immutable StyleProperties.
+        (WebCore::StyleChange::StyleChange): Set m_applyUnderline, m_applyLineThrough, and m_cssStyle if either
+        m_underlineChange or m_strikeThroughChange are TextDecorationChange::Add in EditingStyle if the current position
+        doesn't already have the matching style.
+        (WebCore::StyleChange::operator==): Moved from the header file. Also added the logic to compare m_cssStyle now
+        that it's a StyleProperties instead of String.
+
+        * editing/EditingStyle.h: Added TextDecorationChange.
+        (WebCore::EditingStyle::create): Added a variant that takes CSSPropertyID and CSSValueID.
+        (WebCore::EditingStyle::conflictsWithInlineStyleOfElement):
+        (WebCore::EditingStyle::setUnderlineChange): Added.
+        (WebCore::EditingStyle::underlineChange): Added.
+        (WebCore::EditingStyle::setStrikeThroughChange): Added.
+        (WebCore::EditingStyle::strikeThroughChange): Added.
+        (WebCore::StyleChange::cssStyle): Now returns StyleProperties* instead of String so that ApplyStyleCommand's
+        applyInlineStyleChange could merge inline styles instead of just appending it to the end.
+        (WebCore::StyleChange::operator==): Moved into the cpp file.
+
+        * editing/Editor.cpp:
+        (WebCore::Editor::applyStyle): Added. This variant takes EditingStyle instead of StyleProperties.
+        (WebCore::Editor::applyStyleToSelection): Ditto.
+        (WebCore::Editor::computeAndSetTypingStyle): Added a variant for EditingStyle. Also use overrideTypingStyleAt
+        to set -webkit-text-decorations-in-effect based on m_underlineChange and m_strikeThroughChange 
+
+        * editing/Editor.h:
+        * editing/EditorCommand.cpp:
+        (WebCore::applyCommandToFrame):
+        (WebCore::isStylePresent): Extracted from executeToggleStyle.
+        (WebCore::executeApplyStyle):
+        (WebCore::executeToggleStyle):
+        (WebCore::executeToggleStyleInList): Deleted.
+        (WebCore::textDecorationChangeForToggling): Added. Used in executeStrikethrough and executeUnderline.
+        (WebCore::executeStrikethrough):
+        (WebCore::executeUnderline):
+
 2015-05-04  Eric Carlson  <eric.carlson@apple.com>
 

trunk/Source/WebCore/editing/ApplyStyleCommand.cpp

@@ -264 +264 @@
     while (paragraphStart.isNotNull() && paragraphStart != beyondEnd) {
         StyleChange styleChange(style, paragraphStart.deepEquivalent());
-        if (styleChange.cssStyle().length() || m_removeOnly) {
+        if (styleChange.cssStyle() || m_removeOnly) {
             RefPtr<Node> block = enclosingBlock(paragraphStart.deepEquivalent().deprecatedNode());
             if (!m_removeOnly) {
@@ -946 +946 @@
         return style->conflictsWithInlineStyleOfElement(element);
 
-    Vector<CSSPropertyID> properties;
-    if (!style->conflictsWithInlineStyleOfElement(element, extractedStyle, properties))
+    RefPtr<MutableStyleProperties> newInlineStyle;
+    if (!style->conflictsWithInlineStyleOfElement(element, newInlineStyle, extractedStyle))
         return false;
 
-    // FIXME: We should use a mass-removal function here but we don't have an undoable one yet.
-    for (size_t i = 0; i < properties.size(); i++)
-        removeCSSProperty(element, properties[i]);
-
-    // No need to serialize <foo style=""> if we just removed the last css property
-    if (element->inlineStyle()->isEmpty())
+    if (newInlineStyle->isEmpty())
         removeNodeAttribute(element, styleAttr);
+    else
+        setNodeAttribute(element, styleAttr, newInlineStyle->asText());
 
     if (isSpanWithoutAttributesOrUnstyledStyleSpan(element))
@@ -1371 +1368 @@
 void ApplyStyleCommand::addBlockStyle(const StyleChange& styleChange, HTMLElement* block)
 {
+    ASSERT(styleChange.cssStyle());
     // Do not check for legacy styles here. Those styles, like <B> and <I>, only apply for
     // inline content.
@@ -1376 +1374 @@
         return;
         
-    String cssStyle = styleChange.cssStyle();
+    String cssStyle = styleChange.cssStyle()->asText();
     StringBuilder cssText;
     cssText.append(cssStyle);
@@ -1457 +1455 @@
     }
 
-    if (styleChange.cssStyle().length()) {
+    if (auto styleToMerge = styleChange.cssStyle()) {
         if (styleContainer) {
-            if (const StyleProperties* existingStyle = styleContainer->inlineStyle()) {
-                String existingText = existingStyle->asText();
-                StringBuilder cssText;
-                cssText.append(existingText);
-                if (!existingText.isEmpty())
-                    cssText.append(' ');
-                cssText.append(styleChange.cssStyle());
-                setNodeAttribute(styleContainer, styleAttr, cssText.toString());
+            if (auto existingStyle = styleContainer->inlineStyle()) {
+                auto inlineStyle = EditingStyle::create(existingStyle);
+                inlineStyle->overrideWithStyle(styleToMerge);
+                setNodeAttribute(styleContainer, styleAttr, inlineStyle->style()->asText());
             } else
-                setNodeAttribute(styleContainer, styleAttr, styleChange.cssStyle());
+                setNodeAttribute(styleContainer, styleAttr, styleToMerge->asText());
         } else {
             RefPtr<Element> styleElement = createStyleSpanElement(document());
-            styleElement->setAttribute(styleAttr, styleChange.cssStyle());
+            styleElement->setAttribute(styleAttr, styleToMerge->asText());
             surroundNodeRangeWithElement(startNode, endNode, styleElement.release());
         }

trunk/Source/WebCore/editing/EditingStyle.cpp

@@ -161 +161 @@
 
     virtual ~HTMLElementEquivalent() { }
-    virtual bool matches(const Element* element) const { return !m_tagName || element->hasTagName(*m_tagName); }
+    virtual bool matches(const Element& element) const { return !m_tagName || element.hasTagName(*m_tagName); }
     virtual bool hasAttribute() const { return false; }
-    virtual bool propertyExistsInStyle(const StyleProperties* style) const { return style->getPropertyCSSValue(m_propertyID); }
-    virtual bool valueIsPresentInStyle(Element*, StyleProperties*) const;
+    virtual bool propertyExistsInStyle(const EditingStyle& style) const { return style.m_mutableStyle && style.m_mutableStyle->getPropertyCSSValue(m_propertyID); }
+    virtual bool valueIsPresentInStyle(Element&, const EditingStyle&) const;
     virtual void addToStyle(Element*, EditingStyle*) const;
 
@@ -195 +195 @@
 }
 
-bool HTMLElementEquivalent::valueIsPresentInStyle(Element* element, StyleProperties* style) const
-{
-    RefPtr<CSSValue> value = style->getPropertyCSSValue(m_propertyID);
+bool HTMLElementEquivalent::valueIsPresentInStyle(Element& element, const EditingStyle& style) const
+{
+    RefPtr<CSSValue> value = style.m_mutableStyle->getPropertyCSSValue(m_propertyID);
     return matches(element) && is<CSSPrimitiveValue>(value.get()) && downcast<CSSPrimitiveValue>(*value).getValueID() == m_primitiveValue->getValueID();
 }
@@ -208 +208 @@
 class HTMLTextDecorationEquivalent : public HTMLElementEquivalent {
 public:
-    HTMLTextDecorationEquivalent(CSSValueID primitiveValue, const QualifiedName& tagName);
-
-    virtual bool propertyExistsInStyle(const StyleProperties*) const;
-    virtual bool valueIsPresentInStyle(Element*, StyleProperties*) const;
+    HTMLTextDecorationEquivalent(CSSValueID primitiveValue, const QualifiedName& tagName)
+        : HTMLElementEquivalent(CSSPropertyTextDecoration, primitiveValue, tagName)
+        , m_isUnderline(primitiveValue == CSSValueUnderline)
+    {
+    }
+
+    bool propertyExistsInStyle(const EditingStyle& style) const override
+    {
+        if (changeInStyle(style) != TextDecorationChange::None)
+            return true;
+
+        if (!style.m_mutableStyle)
+            return false;
+
+        auto& mutableStyle = *style.m_mutableStyle;
+        return mutableStyle.getPropertyCSSValue(CSSPropertyWebkitTextDecorationsInEffect)
+            || mutableStyle.getPropertyCSSValue(CSSPropertyTextDecoration);
+    }
+
+    bool valueIsPresentInStyle(Element& element, const EditingStyle& style) const override
+    {
+        if (!matches(element))
+            return false;
+        auto change = changeInStyle(style);
+        if (change != TextDecorationChange::None)
+            return change == TextDecorationChange::Add;
+        RefPtr<CSSValue> styleValue = style.m_mutableStyle->getPropertyCSSValue(CSSPropertyWebkitTextDecorationsInEffect);
+        if (!styleValue)
+            styleValue = style.m_mutableStyle->getPropertyCSSValue(CSSPropertyTextDecoration);
+        return is<CSSValueList>(styleValue.get()) && downcast<CSSValueList>(*styleValue).hasValue(m_primitiveValue.get());
+    }
+
+private:
+    TextDecorationChange changeInStyle(const EditingStyle& style) const
+    {
+        return m_isUnderline ? style.underlineChange() : style.strikeThroughChange();
+    }
+
+    bool m_isUnderline;
 };
-
-HTMLTextDecorationEquivalent::HTMLTextDecorationEquivalent(CSSValueID primitiveValue, const QualifiedName& tagName)
-    : HTMLElementEquivalent(CSSPropertyTextDecoration, primitiveValue, tagName)
-    // m_propertyID is used in HTMLElementEquivalent::addToStyle
-{
-}
-
-bool HTMLTextDecorationEquivalent::propertyExistsInStyle(const StyleProperties* style) const
-{
-    return style->getPropertyCSSValue(CSSPropertyWebkitTextDecorationsInEffect) || style->getPropertyCSSValue(CSSPropertyTextDecoration);
-}
-
-bool HTMLTextDecorationEquivalent::valueIsPresentInStyle(Element* element, StyleProperties* style) const
-{
-    RefPtr<CSSValue> styleValue = style->getPropertyCSSValue(CSSPropertyWebkitTextDecorationsInEffect);
-    if (!styleValue)
-        styleValue = style->getPropertyCSSValue(CSSPropertyTextDecoration);
-    return matches(element) && is<CSSValueList>(styleValue.get()) && downcast<CSSValueList>(*styleValue).hasValue(m_primitiveValue.get());
-}
 
 class HTMLAttributeEquivalent : public HTMLElementEquivalent {
@@ -238 +254 @@
     HTMLAttributeEquivalent(CSSPropertyID, const QualifiedName& attrName);
 
-    bool matches(const Element* elem) const { return HTMLElementEquivalent::matches(elem) && elem->hasAttribute(m_attrName); }
+    bool matches(const Element& element) const { return HTMLElementEquivalent::matches(element) && element.hasAttribute(m_attrName); }
     virtual bool hasAttribute() const { return true; }
-    virtual bool valueIsPresentInStyle(Element*, StyleProperties*) const;
+    virtual bool valueIsPresentInStyle(Element&, const EditingStyle&) const;
     virtual void addToStyle(Element*, EditingStyle*) const;
     virtual PassRefPtr<CSSValue> attributeValueAsCSSValue(Element*) const;
@@ -261 +277 @@
 }
 
-bool HTMLAttributeEquivalent::valueIsPresentInStyle(Element* element, StyleProperties* style) const
-{
-    RefPtr<CSSValue> value = attributeValueAsCSSValue(element);
-    RefPtr<CSSValue> styleValue = style->getPropertyCSSValue(m_propertyID);
+bool HTMLAttributeEquivalent::valueIsPresentInStyle(Element& element, const EditingStyle& style) const
+{
+    RefPtr<CSSValue> value = attributeValueAsCSSValue(&element);
+    RefPtr<CSSValue> styleValue = style.m_mutableStyle->getPropertyCSSValue(m_propertyID);
     
     return compareCSSValuePtr(value, styleValue);
@@ -316 +332 @@
 EditingStyle::EditingStyle()
     : m_shouldUseFixedDefaultFontSize(false)
-    , m_fontSizeDelta(NoFontDelta)
+    , m_underlineChange(static_cast<unsigned>(TextDecorationChange::None))
+    , m_strikeThroughChange(static_cast<unsigned>(TextDecorationChange::None))
 {
 }
 
 EditingStyle::EditingStyle(Node* node, PropertiesToInclude propertiesToInclude)
-    : m_shouldUseFixedDefaultFontSize(false)
-    , m_fontSizeDelta(NoFontDelta)
+    : EditingStyle()
 {
     init(node, propertiesToInclude);
@@ -328 +344 @@
 
 EditingStyle::EditingStyle(const Position& position, PropertiesToInclude propertiesToInclude)
-    : m_shouldUseFixedDefaultFontSize(false)
-    , m_fontSizeDelta(NoFontDelta)
+    : EditingStyle()
 {
     init(position.deprecatedNode(), propertiesToInclude);
@@ -335 +350 @@
 
 EditingStyle::EditingStyle(const StyleProperties* style)
-    : m_shouldUseFixedDefaultFontSize(false)
-    , m_fontSizeDelta(NoFontDelta)
+    : EditingStyle()
 {
     if (style)
@@ -344 +358 @@
 
 EditingStyle::EditingStyle(CSSPropertyID propertyID, const String& value)
-    : m_mutableStyle(0)
-    , m_shouldUseFixedDefaultFontSize(false)
-    , m_fontSizeDelta(NoFontDelta)
+    : EditingStyle()
 {
     setProperty(propertyID, value);
+    extractFontSizeDelta();
+}
+
+EditingStyle::EditingStyle(CSSPropertyID propertyID, CSSValueID value)
+    : EditingStyle()
+{
+    m_mutableStyle = MutableStyleProperties::create();
+    m_mutableStyle->setProperty(propertyID, value);
+    extractFontSizeDelta();
 }
 
@@ -493 +514 @@
 bool EditingStyle::isEmpty() const
 {
-    return (!m_mutableStyle || m_mutableStyle->isEmpty()) && m_fontSizeDelta == NoFontDelta;
+    return (!m_mutableStyle || m_mutableStyle->isEmpty()) && m_fontSizeDelta == NoFontDelta
+        && underlineChange() == TextDecorationChange::None && strikeThroughChange() == TextDecorationChange::None;
 }
 
@@ -538 +560 @@
 }
 
+static void applyTextDecorationChangeToValueList(CSSValueList& valueList, TextDecorationChange change, Ref<CSSPrimitiveValue>&& value)
+{
+    switch (change) {
+    case TextDecorationChange::None:
+        break;
+    case TextDecorationChange::Add:
+        valueList.append(WTF::move(value));
+        break;
+    case TextDecorationChange::Remove:
+        valueList.removeAll(&value.get());
+        break;
+    }
+}
+
+void EditingStyle::overrideTypingStyleAt(const EditingStyle& style, const Position& position)
+{
+    mergeStyle(style.m_mutableStyle.get(), OverrideValues);
+
+    m_fontSizeDelta += style.m_fontSizeDelta;
+
+    prepareToApplyAt(position, EditingStyle::PreserveWritingDirection);
+
+    auto underlineChange = style.underlineChange();
+    auto strikeThroughChange = style.strikeThroughChange();
+    if (underlineChange == TextDecorationChange::None && strikeThroughChange == TextDecorationChange::None)
+        return;
+
+    if (!m_mutableStyle)
+        m_mutableStyle = MutableStyleProperties::create();
+
+    Ref<CSSPrimitiveValue> underline = cssValuePool().createIdentifierValue(CSSValueUnderline);
+    Ref<CSSPrimitiveValue> lineThrough = cssValuePool().createIdentifierValue(CSSValueLineThrough);
+    RefPtr<CSSValue> value = m_mutableStyle->getPropertyCSSValue(CSSPropertyWebkitTextDecorationsInEffect);
+    RefPtr<CSSValueList> valueList;
+    if (value && value->isValueList()) {
+        valueList = downcast<CSSValueList>(*value).copy();
+        applyTextDecorationChangeToValueList(*valueList, underlineChange, WTF::move(underline));
+        applyTextDecorationChangeToValueList(*valueList, strikeThroughChange, WTF::move(lineThrough));
+    } else {
+        valueList = CSSValueList::createSpaceSeparated();
+        if (underlineChange == TextDecorationChange::Add)
+            valueList->append(WTF::move(underline));
+        if (strikeThroughChange == TextDecorationChange::Add)
+            valueList->append(WTF::move(lineThrough));
+    }
+    m_mutableStyle->setProperty(CSSPropertyWebkitTextDecorationsInEffect, valueList.get());
+}
+
 void EditingStyle::clear()
 {
@@ -543 +613 @@
     m_shouldUseFixedDefaultFontSize = false;
     m_fontSizeDelta = NoFontDelta;
+    setUnderlineChange(TextDecorationChange::None);
+    setStrikeThroughChange(TextDecorationChange::None);
 }
 
@@ -551 +623 @@
         copy->m_mutableStyle = m_mutableStyle->mutableCopy();
     copy->m_shouldUseFixedDefaultFontSize = m_shouldUseFixedDefaultFontSize;
+    copy->m_underlineChange = m_underlineChange;
+    copy->m_strikeThroughChange = m_strikeThroughChange;
     copy->m_fontSizeDelta = m_fontSizeDelta;
     return copy;
@@ -695 +769 @@
 }
 
-bool EditingStyle::conflictsWithInlineStyleOfElement(StyledElement* element, EditingStyle* extractedStyle, Vector<CSSPropertyID>* conflictingProperties) const
+static RefPtr<CSSValueList> textDecorationValueList(const StyleProperties& properties)
+{
+    RefPtr<CSSValue> value = properties.getPropertyCSSValue(CSSPropertyTextDecoration);
+    if (!is<CSSValueList>(value.get()))
+        return nullptr;
+    return downcast<CSSValueList>(value.get());
+}
+
+bool EditingStyle::conflictsWithInlineStyleOfElement(StyledElement* element, RefPtr<MutableStyleProperties>* newInlineStylePtr, EditingStyle* extractedStyle) const
 {
     ASSERT(element);
-    ASSERT(!conflictingProperties || conflictingProperties->isEmpty());
 
     const StyleProperties* inlineStyle = element->inlineStyle();
-    if (!m_mutableStyle || !inlineStyle)
+    if (!inlineStyle)
         return false;
-
-    unsigned propertyCount = m_mutableStyle->propertyCount();
+    bool conflicts = false;
+    RefPtr<MutableStyleProperties> newInlineStyle;
+    if (newInlineStylePtr) {
+        newInlineStyle = inlineStyle->mutableCopy();
+        *newInlineStylePtr = newInlineStyle;
+    }
+
+    bool shouldRemoveUnderline = underlineChange() == TextDecorationChange::Remove;
+    bool shouldRemoveStrikeThrough = strikeThroughChange() == TextDecorationChange::Remove;
+    if (shouldRemoveUnderline || shouldRemoveStrikeThrough) {
+        if (RefPtr<CSSValueList> valueList = textDecorationValueList(*inlineStyle)) {
+            RefPtr<CSSValueList> newValueList = valueList->copy();
+            RefPtr<CSSValueList> extractedValueList = CSSValueList::createSpaceSeparated();
+
+            Ref<CSSPrimitiveValue> underline = cssValuePool().createIdentifierValue(CSSValueUnderline);
+            if (shouldRemoveUnderline && valueList->hasValue(underline.ptr())) {
+                if (!newInlineStyle)
+                    return true;
+                newValueList->removeAll(underline.ptr());
+                extractedValueList->append(WTF::move(underline));
+            }
+
+            Ref<CSSPrimitiveValue> lineThrough = cssValuePool().createIdentifierValue(CSSValueLineThrough);
+            if (shouldRemoveStrikeThrough && valueList->hasValue(lineThrough.ptr())) {
+                if (!newInlineStyle)
+                    return true;
+                newValueList->removeAll(lineThrough.ptr());
+                extractedValueList->append(WTF::move(lineThrough));
+            }
+
+            if (extractedValueList->length()) {
+                conflicts = true;
+                if (newValueList->length())
+                    newInlineStyle->setProperty(CSSPropertyTextDecoration, newValueList);
+                else
+                    newInlineStyle->removeProperty(CSSPropertyTextDecoration);
+
+                if (extractedStyle) {
+                    bool isImportant = inlineStyle->propertyIsImportant(CSSPropertyTextDecoration);
+                    extractedStyle->setProperty(CSSPropertyTextDecoration, extractedValueList->cssText(), isImportant);
+                }
+            }
+        }
+    }
+
+    unsigned propertyCount = m_mutableStyle ? m_mutableStyle->propertyCount() : 0;
     for (unsigned i = 0; i < propertyCount; ++i) {
         CSSPropertyID propertyID = m_mutableStyle->propertyAt(i).id();
@@ -713 +838 @@
 
         if (propertyID == CSSPropertyWebkitTextDecorationsInEffect && inlineStyle->getPropertyCSSValue(CSSPropertyTextDecoration)) {
-            if (!conflictingProperties)
+            if (!newInlineStyle)
                 return true;
-            conflictingProperties->append(CSSPropertyTextDecoration);
+            conflicts = true;
+            newInlineStyle->removeProperty(CSSPropertyTextDecoration);
             if (extractedStyle)
                 extractedStyle->setProperty(CSSPropertyTextDecoration, inlineStyle->getPropertyValue(CSSPropertyTextDecoration), inlineStyle->propertyIsImportant(CSSPropertyTextDecoration));
-            continue;
         }
 
@@ -725 +850 @@
 
         if (propertyID == CSSPropertyUnicodeBidi && inlineStyle->getPropertyCSSValue(CSSPropertyDirection)) {
-            if (!conflictingProperties)
+            if (!newInlineStyle)
                 return true;
-            conflictingProperties->append(CSSPropertyDirection);
+            conflicts = true;
+            newInlineStyle->removeProperty(CSSPropertyDirection);
             if (extractedStyle)
                 extractedStyle->setProperty(propertyID, inlineStyle->getPropertyValue(propertyID), inlineStyle->propertyIsImportant(propertyID));
         }
 
-        if (!conflictingProperties)
+        if (!newInlineStyle)
             return true;
 
-        conflictingProperties->append(propertyID);
-
+        conflicts = true;
+        newInlineStyle->removeProperty(propertyID);
         if (extractedStyle)
             extractedStyle->setProperty(propertyID, inlineStyle->getPropertyValue(propertyID), inlineStyle->propertyIsImportant(propertyID));
     }
 
-    return conflictingProperties && !conflictingProperties->isEmpty();
+    return conflicts;
 }
 
@@ -767 +893 @@
 bool EditingStyle::conflictsWithImplicitStyleOfElement(HTMLElement* element, EditingStyle* extractedStyle, ShouldExtractMatchingStyle shouldExtractMatchingStyle) const
 {
-    if (!m_mutableStyle)
+    if (isEmpty())
         return false;
 
     const Vector<std::unique_ptr<HTMLElementEquivalent>>& HTMLElementEquivalents = htmlElementEquivalents();
-    for (size_t i = 0; i < HTMLElementEquivalents.size(); ++i) {
-        const HTMLElementEquivalent* equivalent = HTMLElementEquivalents[i].get();
-        if (equivalent->matches(element) && equivalent->propertyExistsInStyle(m_mutableStyle.get())
-            && (shouldExtractMatchingStyle == ExtractMatchingStyle || !equivalent->valueIsPresentInStyle(element, m_mutableStyle.get()))) {
+    for (auto& equivalent : HTMLElementEquivalents) {
+        if (equivalent->matches(*element) && equivalent->propertyExistsInStyle(*this)
+            && (shouldExtractMatchingStyle == ExtractMatchingStyle || !equivalent->valueIsPresentInStyle(*element, *this))) {
             if (extractedStyle)
                 equivalent->addToStyle(element, extractedStyle);
@@ -804 +929 @@
 {
     ASSERT(element);
-    if (!m_mutableStyle)
+    if (isEmpty())
         return false;
 
     const Vector<std::unique_ptr<HTMLAttributeEquivalent>>& HTMLAttributeEquivalents = htmlAttributeEquivalents();
-    for (size_t i = 0; i < HTMLAttributeEquivalents.size(); ++i) {
-        if (HTMLAttributeEquivalents[i]->matches(element) && HTMLAttributeEquivalents[i]->propertyExistsInStyle(m_mutableStyle.get())
-            && !HTMLAttributeEquivalents[i]->valueIsPresentInStyle(element, m_mutableStyle.get()))
+    for (auto& equivalent : HTMLAttributeEquivalents) {
+        if (equivalent->matches(*element) && equivalent->propertyExistsInStyle(*this) && !equivalent->valueIsPresentInStyle(*element, *this))
             return true;
     }
@@ -835 +959 @@
             continue;
 
-        if (!equivalent->matches(element) || !equivalent->propertyExistsInStyle(m_mutableStyle.get())
-            || (shouldExtractMatchingStyle == DoNotExtractMatchingStyle && equivalent->valueIsPresentInStyle(element, m_mutableStyle.get())))
+        if (!equivalent->matches(*element) || !equivalent->propertyExistsInStyle(*this)
+            || (shouldExtractMatchingStyle == DoNotExtractMatchingStyle && equivalent->valueIsPresentInStyle(*element, *this)))
             continue;
 
@@ -850 +974 @@
 bool EditingStyle::styleIsPresentInComputedStyleOfNode(Node* node) const
 {
-    if (!m_mutableStyle)
+    if (isEmpty())
         return true;
     ComputedStyleExtractor computedStyle(node);
-    return getPropertiesNotIn(*m_mutableStyle, computedStyle)->isEmpty();
+
+    bool shouldAddUnderline = underlineChange() == TextDecorationChange::Add;
+    bool shouldAddLineThrough = strikeThroughChange() == TextDecorationChange::Add;
+    if (shouldAddUnderline || shouldAddLineThrough) {
+        bool hasUnderline = false;
+        bool hasLineThrough = false;
+        if (RefPtr<CSSValue> value = computedStyle.propertyValue(CSSPropertyTextDecoration)) {
+            if (value->isValueList()) {
+                const CSSValueList& valueList = downcast<CSSValueList>(*value);
+                hasUnderline = valueList.hasValue(&cssValuePool().createIdentifierValue(CSSValueUnderline).get());
+                hasLineThrough = valueList.hasValue(&cssValuePool().createIdentifierValue(CSSValueLineThrough).get());
+            }
+        }
+        if ((shouldAddUnderline && !hasUnderline) || (shouldAddLineThrough && !hasLineThrough))
+            return false;
+    }
+
+    return !m_mutableStyle || getPropertiesNotIn(*m_mutableStyle, computedStyle)->isEmpty();
 }
 
@@ -865 +1006 @@
         size_t i;
         for (i = 0; i < HTMLElementEquivalents.size(); ++i) {
-            if (HTMLElementEquivalents[i]->matches(element)) {
+            if (HTMLElementEquivalents[i]->matches(*element)) {
                 elementIsSpanOrElementEquivalent = true;
                 break;
@@ -878 +1019 @@
     const Vector<std::unique_ptr<HTMLAttributeEquivalent>>& HTMLAttributeEquivalents = htmlAttributeEquivalents();
     for (size_t i = 0; i < HTMLAttributeEquivalents.size(); ++i) {
-        if (HTMLAttributeEquivalents[i]->matches(element) && HTMLAttributeEquivalents[i]->attributeName() != HTMLNames::dirAttr)
+        if (HTMLAttributeEquivalents[i]->matches(*element) && HTMLAttributeEquivalents[i]->attributeName() != HTMLNames::dirAttr)
             matchedAttributes++;
     }
@@ -970 +1111 @@
 
 static inline bool elementMatchesAndPropertyIsNotInInlineStyleDecl(const HTMLElementEquivalent* equivalent, const StyledElement* element,
-    EditingStyle::CSSPropertyOverrideMode mode, StyleProperties* style)
-{
-    return equivalent->matches(element) && (!element->inlineStyle() || !equivalent->propertyExistsInStyle(element->inlineStyle()))
-        && (mode == EditingStyle::OverrideValues || !equivalent->propertyExistsInStyle(style));
+    EditingStyle::CSSPropertyOverrideMode mode, EditingStyle& style)
+{
+    if (!equivalent->matches(*element))
+        return false;
+    if (mode != EditingStyle::OverrideValues && equivalent->propertyExistsInStyle(style))
+        return false;
+
+    return !element->inlineStyle() || !equivalent->propertyExistsInStyle(EditingStyle::create(element->inlineStyle()).get());
 }
 
@@ -1006 +1151 @@
     const Vector<std::unique_ptr<HTMLElementEquivalent>>& elementEquivalents = htmlElementEquivalents();
     for (size_t i = 0; i < elementEquivalents.size(); ++i) {
-        if (elementMatchesAndPropertyIsNotInInlineStyleDecl(elementEquivalents[i].get(), element, mode, m_mutableStyle.get()))
+        if (elementMatchesAndPropertyIsNotInInlineStyleDecl(elementEquivalents[i].get(), element, mode, *this))
             elementEquivalents[i]->addToStyle(element, this);
     }
@@ -1014 +1159 @@
         if (attributeEquivalents[i]->attributeName() == HTMLNames::dirAttr)
             continue; // We don't want to include directionality
-        if (elementMatchesAndPropertyIsNotInInlineStyleDecl(attributeEquivalents[i].get(), element, mode, m_mutableStyle.get()))
+        if (elementMatchesAndPropertyIsNotInInlineStyleDecl(attributeEquivalents[i].get(), element, mode, *this))
             attributeEquivalents[i]->addToStyle(element, this);
     }
@@ -1050 +1195 @@
 
 
-static void mergeTextDecorationValues(CSSValueList* mergedValue, const CSSValueList* valueToMerge)
-{
-    RefPtr<CSSPrimitiveValue> underline = cssValuePool().createIdentifierValue(CSSValueUnderline);
-    RefPtr<CSSPrimitiveValue> lineThrough = cssValuePool().createIdentifierValue(CSSValueLineThrough);
-
-    if (valueToMerge->hasValue(underline.get()) && !mergedValue->hasValue(underline.get()))
-        mergedValue->append(underline.releaseNonNull());
-
-    if (valueToMerge->hasValue(lineThrough.get()) && !mergedValue->hasValue(lineThrough.get()))
-        mergedValue->append(lineThrough.releaseNonNull());
+static void mergeTextDecorationValues(CSSValueList& mergedValue, const CSSValueList& valueToMerge)
+{
+    Ref<CSSPrimitiveValue> underline = cssValuePool().createIdentifierValue(CSSValueUnderline);
+    Ref<CSSPrimitiveValue> lineThrough = cssValuePool().createIdentifierValue(CSSValueLineThrough);
+
+    if (valueToMerge.hasValue(underline.ptr()) && !mergedValue.hasValue(underline.ptr()))
+        mergedValue.append(WTF::move(underline));
+
+    if (valueToMerge.hasValue(lineThrough.ptr()) && !mergedValue.hasValue(lineThrough.ptr()))
+        mergedValue.append(WTF::move(lineThrough));
 }
 
@@ -1078 +1223 @@
 
         // text decorations never override values.
-        if ((property.id() == CSSPropertyTextDecoration || property.id() == CSSPropertyWebkitTextDecorationsInEffect) && property.value()->isValueList() && value) {
+        if ((property.id() == CSSPropertyTextDecoration || property.id() == CSSPropertyWebkitTextDecorationsInEffect)
+            && is<CSSValueList>(*property.value()) && value) {
             if (is<CSSValueList>(*value)) {
-                mergeTextDecorationValues(downcast<CSSValueList>(value.get()), downcast<CSSValueList>(property.value()));
+                RefPtr<CSSValueList> newValue = downcast<CSSValueList>(*value).copy();
+                mergeTextDecorationValues(*newValue, downcast<CSSValueList>(*property.value()));
+                m_mutableStyle->setProperty(property.id(), newValue.release(), property.isImportant());
                 continue;
             }
@@ -1087 +1235 @@
 
         if (mode == OverrideValues || (mode == DoNotOverrideValues && !value))
-            m_mutableStyle->setProperty(property.id(), property.value()->cssText(), property.isImportant());
+            m_mutableStyle->setProperty(property.id(), property.value(), property.isImportant());
     }
 
@@ -1398 +1546 @@
     , m_applySuperscript(false)
 {
-    Document* document = position.anchorNode() ? &position.anchorNode()->document() : 0;
-    if (!style || !style->style() || !document || !document->frame())
+    Document* document = position.deprecatedNode() ? &position.deprecatedNode()->document() : 0;
+    if (!style || style->isEmpty() || !document || !document->frame())
         return;
 
@@ -1409 +1557 @@
 
     // FIXME: take care of background-color in effect
-    RefPtr<MutableStyleProperties> mutableStyle = getPropertiesNotIn(*style->style(), computedStyle);
+    RefPtr<MutableStyleProperties> mutableStyle = style->style() ?
+        getPropertiesNotIn(*style->style(), computedStyle) : MutableStyleProperties::create();
 
     reconcileTextDecorationProperties(mutableStyle.get());
-    if (!document->frame()->editor().shouldStyleWithCSS())
+    bool shouldStyleWithCSS = document->frame()->editor().shouldStyleWithCSS();
+    if (!shouldStyleWithCSS)
         extractTextStyles(document, *mutableStyle, computedStyle.useFixedFontDefaultSize());
+
+    bool shouldAddUnderline = style->underlineChange() == TextDecorationChange::Add;
+    bool shouldAddStrikeThrough = style->strikeThroughChange() == TextDecorationChange::Add;
+    if (shouldAddUnderline || shouldAddStrikeThrough) {
+        RefPtr<CSSValue> value = computedStyle.propertyValue(CSSPropertyWebkitTextDecorationsInEffect);
+        if (!is<CSSValueList>(value.get()))
+            value = computedStyle.propertyValue(CSSPropertyTextDecoration);
+
+        RefPtr<CSSValueList> valueList;
+        if (is<CSSValueList>(value.get()))
+            valueList = downcast<CSSValueList>(value.get());
+
+        Ref<CSSValue> underline = cssValuePool().createIdentifierValue(CSSValueUnderline);
+        bool hasUnderline = valueList && valueList->hasValue(underline.ptr());
+
+        Ref<CSSValue> lineThrough = cssValuePool().createIdentifierValue(CSSValueLineThrough);
+        bool hasLineThrough = valueList && valueList->hasValue(lineThrough.ptr());
+
+        if (shouldStyleWithCSS) {
+            valueList = valueList ? valueList->copy() : CSSValueList::createSpaceSeparated();
+            if (shouldAddUnderline && !hasUnderline)
+                valueList->append(WTF::move(underline));
+            if (shouldAddStrikeThrough && !hasLineThrough)
+                valueList->append(WTF::move(lineThrough));
+            mutableStyle->setProperty(CSSPropertyTextDecoration, valueList.get());
+        } else {
+            m_applyUnderline = shouldAddUnderline && !hasUnderline;
+            m_applyLineThrough = shouldAddStrikeThrough && !hasLineThrough;
+        }
+    }
 
     // Changing the whitespace style in a tab span would collapse the tab into a space.
@@ -1424 +1604 @@
         mutableStyle->setProperty(CSSPropertyDirection, style->style()->getPropertyValue(CSSPropertyDirection));
 
-    // Save the result for later
-    m_cssStyle = mutableStyle->asText().stripWhiteSpace();
+    if (!mutableStyle->isEmpty())
+        m_cssStyle = mutableStyle;
+}
+
+bool StyleChange::operator==(const StyleChange& other)
+{
+    if (m_applyBold != other.m_applyBold
+        || m_applyItalic != other.m_applyItalic
+        || m_applyUnderline != other.m_applyUnderline
+        || m_applyLineThrough != other.m_applyLineThrough
+        || m_applySubscript != other.m_applySubscript
+        || m_applySuperscript != other.m_applySuperscript
+        || m_applyFontColor != other.m_applyFontColor
+        || m_applyFontFace != other.m_applyFontFace
+        || m_applyFontSize != other.m_applyFontSize)
+        return false;
+
+    return (!m_cssStyle && !other.m_cssStyle)
+        || (m_cssStyle && other.m_cssStyle && m_cssStyle->asText() == other.m_cssStyle->asText());
 }
 

trunk/Source/WebCore/editing/EditingStyle.h

@@ -35 +35 @@
 #include "CSSPropertyNames.h"
 #include "CSSValueKeywords.h"
+#include "StyleProperties.h"
 #include "WritingDirection.h"
 #include <wtf/Forward.h>
+#include <wtf/HashMap.h>
 #include <wtf/RefCounted.h>
 #include <wtf/RefPtr.h>
@@ -62 +64 @@
 class VisibleSelection;
 
+enum class TextDecorationChange { None, Add, Remove };
+
 class EditingStyle : public RefCounted<EditingStyle> {
 public:
@@ -92 +96 @@
 
     static Ref<EditingStyle> create(CSSPropertyID propertyID, const String& value)
+    {
+        return adoptRef(*new EditingStyle(propertyID, value));
+    }
+
+    static Ref<EditingStyle> create(CSSPropertyID propertyID, CSSValueID value)
     {
         return adoptRef(*new EditingStyle(propertyID, value));
@@ -103 +112 @@
     void setStyle(PassRefPtr<MutableStyleProperties>);
     void overrideWithStyle(const StyleProperties*);
+    void overrideTypingStyleAt(const EditingStyle&, const Position&);
     void clear();
     PassRefPtr<EditingStyle> copy() const;
@@ -116 +126 @@
     TriState triStateOfStyle(const VisibleSelection&) const;
     bool conflictsWithInlineStyleOfElement(StyledElement* element) const { return conflictsWithInlineStyleOfElement(element, 0, 0); }
-    bool conflictsWithInlineStyleOfElement(StyledElement* element, EditingStyle* extractedStyle, Vector<CSSPropertyID>& conflictingProperties) const
-    {
-        return conflictsWithInlineStyleOfElement(element, extractedStyle, &conflictingProperties);
+    bool conflictsWithInlineStyleOfElement(StyledElement* element, RefPtr<MutableStyleProperties>& newInlineStyle,
+        EditingStyle* extractedStyle) const
+    {
+        return conflictsWithInlineStyleOfElement(element, &newInlineStyle, extractedStyle);
     }
     bool conflictsWithImplicitStyleOfElement(HTMLElement*, EditingStyle* extractedStyle = 0, ShouldExtractMatchingStyle = DoNotExtractMatchingStyle) const;
@@ -145 +156 @@
     bool hasFontSizeDelta() const { return m_fontSizeDelta != NoFontDelta; }
     bool shouldUseFixedDefaultFontSize() const { return m_shouldUseFixedDefaultFontSize; }
+    
+    void setUnderlineChange(TextDecorationChange change) { m_underlineChange = static_cast<unsigned>(change); }
+    TextDecorationChange underlineChange() const { return static_cast<TextDecorationChange>(m_underlineChange); }
+    void setStrikeThroughChange(TextDecorationChange change) { m_strikeThroughChange = static_cast<unsigned>(change); }
+    TextDecorationChange strikeThroughChange() const { return static_cast<TextDecorationChange>(m_strikeThroughChange); }
 
     static PassRefPtr<EditingStyle> styleAtSelectionStart(const VisibleSelection&, bool shouldUseBackgroundColorInEffect = false);
@@ -154 +170 @@
     explicit EditingStyle(const StyleProperties*);
     EditingStyle(CSSPropertyID, const String& value);
+    EditingStyle(CSSPropertyID, CSSValueID);
     void init(Node*, PropertiesToInclude);
     void removeTextFillAndStrokeColorsIfNeeded(RenderStyle*);
@@ -159 +176 @@
     void extractFontSizeDelta();
     template<typename T> TriState triStateOfStyle(T& styleToCompare, ShouldIgnoreTextOnlyProperties) const;
-    bool conflictsWithInlineStyleOfElement(StyledElement*, EditingStyle* extractedStyle, Vector<CSSPropertyID>* conflictingProperties) const;
+    bool conflictsWithInlineStyleOfElement(StyledElement*, RefPtr<MutableStyleProperties>* newInlineStyle, EditingStyle* extractedStyle) const;
     void mergeInlineAndImplicitStyleOfElement(StyledElement*, CSSPropertyOverrideMode, PropertiesToInclude);
     void mergeStyle(const StyleProperties*, CSSPropertyOverrideMode);
 
     RefPtr<MutableStyleProperties> m_mutableStyle;
-    bool m_shouldUseFixedDefaultFontSize;
-    float m_fontSizeDelta;
+    unsigned m_shouldUseFixedDefaultFontSize : 1;
+    unsigned m_underlineChange : 2;
+    unsigned m_strikeThroughChange : 2;
+    float m_fontSizeDelta = NoFontDelta;
 
     friend class HTMLElementEquivalent;
     friend class HTMLAttributeEquivalent;
+    friend class HTMLTextDecorationEquivalent;
 };
 
 class StyleChange {
 public:
-    StyleChange()
-        : m_applyBold(false)
-        , m_applyItalic(false)
-        , m_applyUnderline(false)
-        , m_applyLineThrough(false)
-        , m_applySubscript(false)
-        , m_applySuperscript(false)
-    { }
-
+    StyleChange() { }
     StyleChange(EditingStyle*, const Position&);
 
-    String cssStyle() const { return m_cssStyle; }
+    const StyleProperties* cssStyle() const { return m_cssStyle.get(); }
     bool applyBold() const { return m_applyBold; }
     bool applyItalic() const { return m_applyItalic; }
@@ -199 +211 @@
     String fontSize() { return m_applyFontSize; }
 
-    bool operator==(const StyleChange& other)
-    {
-        return m_cssStyle == other.m_cssStyle
-            && m_applyBold == other.m_applyBold
-            && m_applyItalic == other.m_applyItalic
-            && m_applyUnderline == other.m_applyUnderline
-            && m_applyLineThrough == other.m_applyLineThrough
-            && m_applySubscript == other.m_applySubscript
-            && m_applySuperscript == other.m_applySuperscript
-            && m_applyFontColor == other.m_applyFontColor
-            && m_applyFontFace == other.m_applyFontFace
-            && m_applyFontSize == other.m_applyFontSize;
-    }
+    bool operator==(const StyleChange&);
     bool operator!=(const StyleChange& other)
     {
@@ -219 +219 @@
     void extractTextStyles(Document*, MutableStyleProperties&, bool shouldUseFixedFontDefaultSize);
 
-    String m_cssStyle;
-    bool m_applyBold;
-    bool m_applyItalic;
-    bool m_applyUnderline;
-    bool m_applyLineThrough;
-    bool m_applySubscript;
-    bool m_applySuperscript;
+    RefPtr<MutableStyleProperties> m_cssStyle;
+    bool m_applyBold = false;
+    bool m_applyItalic = false;
+    bool m_applyUnderline = false;
+    bool m_applyLineThrough = false;
+    bool m_applySubscript = false;
+    bool m_applySuperscript = false;
     String m_applyFontColor;
     String m_applyFontFace;

trunk/Source/WebCore/editing/Editor.cpp

@@ -888 +888 @@
         break;
     case VisibleSelection::CaretSelection:
-        computeAndSetTypingStyle(style, editingAction);
+        computeAndSetTypingStyle(EditingStyle::create(style), editingAction);
         break;
     case VisibleSelection::RangeSelection:
         if (style)
             applyCommand(ApplyStyleCommand::create(document(), EditingStyle::create(style).ptr(), editingAction));
+        break;
+    }
+}
+
+void Editor::applyStyle(RefPtr<EditingStyle>&& style, EditAction editingAction)
+{
+    switch (m_frame.selection().selection().selectionType()) {
+    case VisibleSelection::NoSelection:
+        // do nothing
+        break;
+    case VisibleSelection::CaretSelection:
+        computeAndSetTypingStyle(*style, editingAction);
+        break;
+    case VisibleSelection::RangeSelection:
+        if (style)
+            applyCommand(ApplyStyleCommand::create(document(), style.get(), editingAction));
         break;
     }
@@ -921 +937 @@
         return;
 
-    if (client() && client()->shouldApplyStyle(style, m_frame.selection().toNormalizedRange().get()))
-        applyStyle(style, editingAction);
+    if (!client() || !client()->shouldApplyStyle(style, m_frame.selection().toNormalizedRange().get()))
+        return;
+    applyStyle(style, editingAction);
+}
+
+void Editor::applyStyleToSelection(RefPtr<EditingStyle>&& style, EditAction editingAction)
+{
+    if (!style || style->isEmpty() || !canEditRichly())
+        return;
+
+    // FIXME: This is wrong for text decorations since m_mutableStyle is empty.
+    if (!client() || !client()->shouldApplyStyle(style->style(), m_frame.selection().toNormalizedRange().get()))
+        return;
+
+    applyStyle(WTF::move(style), editingAction);
 }
 
@@ -2953 +2982 @@
 }
 
-void Editor::computeAndSetTypingStyle(StyleProperties* style, EditAction editingAction)
-{
-    if (!style || style->isEmpty()) {
+void Editor::computeAndSetTypingStyle(EditingStyle& style, EditAction editingAction)
+{
+    if (style.isEmpty()) {
         m_frame.selection().clearTypingStyle();
         return;
@@ -2962 +2991 @@
     // Calculate the current typing style.
     RefPtr<EditingStyle> typingStyle;
-    if (m_frame.selection().typingStyle()) {
-        typingStyle = m_frame.selection().typingStyle()->copy();
-        typingStyle->overrideWithStyle(style);
-    } else
-        typingStyle = EditingStyle::create(style);
-
-    typingStyle->prepareToApplyAt(m_frame.selection().selection().visibleStart().deepEquivalent(), EditingStyle::PreserveWritingDirection);
+    if (auto existingTypingStyle = m_frame.selection().typingStyle())
+        typingStyle = existingTypingStyle->copy();
+    else
+        typingStyle = EditingStyle::create();
+    typingStyle->overrideTypingStyleAt(style, m_frame.selection().selection().visibleStart().deepEquivalent());
 
     // Handle block styles, substracting these from the typing style.
@@ -2977 +3004 @@
     // Set the remaining style as the typing style.
     m_frame.selection().setTypingStyle(typingStyle);
+}
+
+void Editor::computeAndSetTypingStyle(StyleProperties& properties, EditAction editingAction)
+{
+    return computeAndSetTypingStyle(EditingStyle::create(&properties), editingAction);
 }
 

trunk/Source/WebCore/editing/Editor.h

@@ -184 +184 @@
     
     WEBCORE_EXPORT void applyStyle(StyleProperties*, EditAction = EditActionUnspecified);
+    void applyStyle(RefPtr<EditingStyle>&&, EditAction);
     void applyParagraphStyle(StyleProperties*, EditAction = EditActionUnspecified);
     WEBCORE_EXPORT void applyStyleToSelection(StyleProperties*, EditAction);
+    void applyStyleToSelection(RefPtr<EditingStyle>&&, EditAction);
     void applyParagraphStyleToSelection(StyleProperties*, EditAction);
 
@@ -362 +364 @@
     void setMark(const VisibleSelection&);
 
-    WEBCORE_EXPORT void computeAndSetTypingStyle(StyleProperties* , EditAction = EditActionUnspecified);
+    void computeAndSetTypingStyle(EditingStyle& , EditAction = EditActionUnspecified);
+    WEBCORE_EXPORT void computeAndSetTypingStyle(StyleProperties& , EditAction = EditActionUnspecified);
     WEBCORE_EXPORT void applyEditingStyleToBodyElement() const;
     void applyEditingStyleToElement(Element*) const;

trunk/Source/WebCore/editing/EditorCommand.cpp

@@ -99 +99 @@
 }
 
-static bool applyCommandToFrame(Frame& frame, EditorCommandSource source, EditAction action, StyleProperties* style)
+static bool applyCommandToFrame(Frame& frame, EditorCommandSource source, EditAction action, RefPtr<EditingStyle>&& style)
 {
     // FIXME: We don't call shouldApplyStyle when the source is DOM; is there a good reason for that?
     switch (source) {
     case CommandFromMenuOrKeyBinding:
-        frame.editor().applyStyleToSelection(style, action);
+        frame.editor().applyStyleToSelection(WTF::move(style), action);
         return true;
     case CommandFromDOM:
     case CommandFromDOMWithUserInterface:
-        frame.editor().applyStyle(style);
+        frame.editor().applyStyle(WTF::move(style), EditActionUnspecified);
         return true;
     }
@@ -115 +115 @@
 }
 
-static bool executeApplyStyle(Frame& frame, EditorCommandSource source, EditAction action, CSSPropertyID propertyID, const String& propertyValue)
-{
-    RefPtr<MutableStyleProperties> style = MutableStyleProperties::create();
-    style->setProperty(propertyID, propertyValue);
-    return applyCommandToFrame(frame, source, action, style.get());
-}
-
-static bool executeApplyStyle(Frame& frame, EditorCommandSource source, EditAction action, CSSPropertyID propertyID, CSSValueID propertyValue)
-{
-    RefPtr<MutableStyleProperties> style = MutableStyleProperties::create();
-    style->setProperty(propertyID, propertyValue);
-    return applyCommandToFrame(frame, source, action, style.get());
-}
-
-// FIXME: executeToggleStyleInList does not handle complicated cases such as <b><u>hello</u>world</b> properly.
-//        This function must use Editor::selectionHasStyle to determine the current style but we cannot fix this
-//        until https://bugs.webkit.org/show_bug.cgi?id=27818 is resolved.
-static bool executeToggleStyleInList(Frame& frame, EditorCommandSource source, EditAction action, CSSPropertyID propertyID, CSSValue* value)
-{
-    RefPtr<EditingStyle> selectionStyle = EditingStyle::styleAtSelectionStart(frame.selection().selection());
-    if (!selectionStyle || !selectionStyle->style())
-        return false;
-
-    RefPtr<CSSValue> selectedCSSValue = selectionStyle->style()->getPropertyCSSValue(propertyID);
-    String newStyle = ASCIILiteral("none");
-    if (is<CSSValueList>(*selectedCSSValue)) {
-        RefPtr<CSSValueList> selectedCSSValueList = downcast<CSSValueList>(selectedCSSValue.get());
-        if (!selectedCSSValueList->removeAll(value))
-            selectedCSSValueList->append(*value);
-        if (selectedCSSValueList->length())
-            newStyle = selectedCSSValueList->cssText();
-
-    } else if (selectedCSSValue->cssText() == "none")
-        newStyle = value->cssText();
-
-    // FIXME: We shouldn't be having to convert new style into text.  We should have setPropertyCSSValue.
-    RefPtr<MutableStyleProperties> newMutableStyle = MutableStyleProperties::create();
-    newMutableStyle->setProperty(propertyID, newStyle);
-    return applyCommandToFrame(frame, source, action, newMutableStyle.get());
-}
-
-static bool executeToggleStyle(Frame& frame, EditorCommandSource source, EditAction action, CSSPropertyID propertyID, const char* offValue, const char* onValue)
+static bool isStylePresent(Editor& editor, CSSPropertyID propertyID, const char* onValue)
 {
     // Style is considered present when
     // Mac: present at the beginning of selection
-    // other: present throughout the selection
-
-    bool styleIsPresent;
-    if (frame.editor().behavior().shouldToggleStyleBasedOnStartOfSelection())
-        styleIsPresent = frame.editor().selectionStartHasStyle(propertyID, onValue);
-    else
-        styleIsPresent = frame.editor().selectionHasStyle(propertyID, onValue) == TrueTriState;
-
-    RefPtr<EditingStyle> style = EditingStyle::create(propertyID, styleIsPresent ? offValue : onValue);
-    return applyCommandToFrame(frame, source, action, style->style());
+    // Windows: present throughout the selection
+    if (editor.behavior().shouldToggleStyleBasedOnStartOfSelection())
+        return editor.selectionStartHasStyle(propertyID, onValue);
+    return editor.selectionHasStyle(propertyID, onValue) == TrueTriState;
+}
+
+static bool executeApplyStyle(Frame& frame, EditorCommandSource source, EditAction action, CSSPropertyID propertyID, const String& propertyValue)
+{
+    return applyCommandToFrame(frame, source, action, EditingStyle::create(propertyID, propertyValue));
+}
+
+static bool executeApplyStyle(Frame& frame, EditorCommandSource source, EditAction action, CSSPropertyID propertyID, CSSValueID propertyValue)
+{
+    return applyCommandToFrame(frame, source, action, EditingStyle::create(propertyID, propertyValue));
+}
+
+static bool executeToggleStyle(Frame& frame, EditorCommandSource source, EditAction action, CSSPropertyID propertyID, const char* offValue, const char* onValue)
+{
+    bool styleIsPresent = isStylePresent(frame.editor(), propertyID, onValue);
+    return applyCommandToFrame(frame, source, action, EditingStyle::create(propertyID, styleIsPresent ? offValue : onValue));
 }
 
@@ -1059 +1028 @@
 }
 
+static TextDecorationChange textDecorationChangeForToggling(Editor& editor, CSSPropertyID propertyID, const char* onValue)
+{
+    return isStylePresent(editor, propertyID, onValue) ? TextDecorationChange::Remove : TextDecorationChange::Add;
+}
+
 static bool executeStrikethrough(Frame& frame, Event*, EditorCommandSource source, const String&)
 {
-    RefPtr<CSSPrimitiveValue> lineThrough = CSSPrimitiveValue::createIdentifier(CSSValueLineThrough);
-    return executeToggleStyleInList(frame, source, EditActionUnderline, CSSPropertyWebkitTextDecorationsInEffect, lineThrough.get());
+    RefPtr<EditingStyle> style = EditingStyle::create();
+    style->setStrikeThroughChange(textDecorationChangeForToggling(frame.editor(), CSSPropertyWebkitTextDecorationsInEffect, "line-through"));
+    // FIXME: Needs a new EditAction!
+    return applyCommandToFrame(frame, source, EditActionUnderline, WTF::move(style));
 }
 
@@ -1126 +1102 @@
 static bool executeUnderline(Frame& frame, Event*, EditorCommandSource source, const String&)
 {
-    RefPtr<CSSPrimitiveValue> underline = CSSPrimitiveValue::createIdentifier(CSSValueUnderline);
-    return executeToggleStyleInList(frame, source, EditActionUnderline, CSSPropertyWebkitTextDecorationsInEffect, underline.get());
+    RefPtr<EditingStyle> style = EditingStyle::create();
+    TextDecorationChange change = textDecorationChangeForToggling(frame.editor(), CSSPropertyWebkitTextDecorationsInEffect, "underline");
+    style->setUnderlineChange(change);
+    return applyCommandToFrame(frame, source, EditActionUnderline, WTF::move(style));
 }
 

trunk/Source/WebKit/mac/ChangeLog

@@ +1 @@
+2015-05-04  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Toggling underline or strike through affects each other
+        https://bugs.webkit.org/show_bug.cgi?id=27818
+
+        Reviewed by Darin Adler.
+
+        * WebView/WebFrame.mm:
+        (-[WebFrame _setTypingStyle:withUndoAction:]):
+
 2015-05-01  Martin Robinson  <mrobinson@igalia.com>
 

trunk/Source/WebKit/mac/WebView/WebFrame.mm

@@ -942 +942 @@
     // FIXME: We shouldn't have to create a copy here.
     Ref<MutableStyleProperties> properties(core(style)->copyProperties());
-    _private->coreFrame->editor().computeAndSetTypingStyle(properties.ptr(), undoAction);
+    _private->coreFrame->editor().computeAndSetTypingStyle(properties.get(), undoAction);
 }