Unexpose EditingPropertiesToInclude in EditingStyle.h after r151772
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Jul 2013 12:20:22 +0000 (12:20 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 2 Jul 2013 12:20:22 +0000 (12:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=118271

Reviewed by Andreas Kling.

Don't expose EditingPropertiesToInclude in EditingStyle.h and do other cleanups as described below.

No new tests since there should be no behavioral change.

* editing/EditingStyle.cpp:
(WebCore::copyEditingProperties): Use newly defined numAllEditingProperties and numInheritableEditingProperties
instead of a magic number of 2.
(WebCore::copyPropertiesFromComputedStyle): Renamed from editingStyleFromComputedStyle. It now takes ComputedStyleExtractor and
PropertiesToInclude instead of Node* and EditingPropertiesToInclude and handles AllProperties. Also added a helper that takes Node*.
(WebCore::EditingStyle::init): Calls copyPropertiesFromComputedStyle. Unfortunately, we need to special-case EditingPropertiesInEffect
and not set background-color and text-decoration. This is required probably due to a bug elsewhere.
(WebCore::EditingStyle::removeStyleAddedByNode):
(WebCore::EditingStyle::removeStyleConflictingWithStyleOfNode):
(WebCore::extractEditingProperties): Renamed from EditingStyle::removeAllButEditingProperties; takes PropertiesToInclude instead of
EditingPropertiesToInclude since the only caller of this function, EditingStyle::mergeInlineAndImplicitStyleOfElement, was converting
the type.
(WebCore::EditingStyle::mergeInlineAndImplicitStyleOfElement):
* editing/EditingStyle.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@152286 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/editing/EditingStyle.cpp
Source/WebCore/editing/EditingStyle.h

index 038341ef4a83b1e76ca3320c6b7f191e119b0e3e..6ed4bd1d7d6cffacd79e48e405ae4538caaab6c0 100644 (file)
@@ -1,3 +1,29 @@
+2013-07-02  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Unexpose EditingPropertiesToInclude in EditingStyle.h after r151772
+        https://bugs.webkit.org/show_bug.cgi?id=118271
+
+        Reviewed by Andreas Kling.
+
+        Don't expose EditingPropertiesToInclude in EditingStyle.h and do other cleanups as described below.
+
+        No new tests since there should be no behavioral change.
+
+        * editing/EditingStyle.cpp:
+        (WebCore::copyEditingProperties): Use newly defined numAllEditingProperties and numInheritableEditingProperties
+        instead of a magic number of 2.
+        (WebCore::copyPropertiesFromComputedStyle): Renamed from editingStyleFromComputedStyle. It now takes ComputedStyleExtractor and
+        PropertiesToInclude instead of Node* and EditingPropertiesToInclude and handles AllProperties. Also added a helper that takes Node*.
+        (WebCore::EditingStyle::init): Calls copyPropertiesFromComputedStyle. Unfortunately, we need to special-case EditingPropertiesInEffect
+        and not set background-color and text-decoration. This is required probably due to a bug elsewhere.
+        (WebCore::EditingStyle::removeStyleAddedByNode):
+        (WebCore::EditingStyle::removeStyleConflictingWithStyleOfNode):
+        (WebCore::extractEditingProperties): Renamed from EditingStyle::removeAllButEditingProperties; takes PropertiesToInclude instead of
+        EditingPropertiesToInclude since the only caller of this function, EditingStyle::mergeInlineAndImplicitStyleOfElement, was converting
+        the type.
+        (WebCore::EditingStyle::mergeInlineAndImplicitStyleOfElement):
+        * editing/EditingStyle.h:
+
 2013-07-02  Gyuyoung Kim  <gyuyoung.kim@samsung.com>
 
         Introduce toSVGInlineTextBox
index 456e7c687968805d6d27fbf26b8f37c8f595749c..d9ca0255ac4fe022e121cc9625eb6a49f7d94391 100644 (file)
@@ -57,10 +57,6 @@ 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.
 static const CSSPropertyID editingProperties[] = {
-    CSSPropertyBackgroundColor,
-    CSSPropertyTextDecoration,
-
-    // CSS inheritable properties
     CSSPropertyColor,
     CSSPropertyFontFamily,
     CSSPropertyFontSize,
@@ -80,14 +76,22 @@ static const CSSPropertyID editingProperties[] = {
     CSSPropertyWebkitTextFillColor,
     CSSPropertyWebkitTextStrokeColor,
     CSSPropertyWebkitTextStrokeWidth,
+
+    // Non-inheritable properties
+    CSSPropertyBackgroundColor,
+    CSSPropertyTextDecoration,
 };
 
+const unsigned numAllEditingProperties = WTF_ARRAY_LENGTH(editingProperties);
+const unsigned numInheritableEditingProperties = numAllEditingProperties - 2;
+
+enum EditingPropertiesToInclude { OnlyInheritableEditingProperties, AllEditingProperties };
 template <class StyleDeclarationType>
-static PassRefPtr<MutableStylePropertySet> copyEditingProperties(StyleDeclarationType* style, EditingStyle::EditingPropertiesToInclude type = EditingStyle::OnlyInheritableEditingProperties)
+static PassRefPtr<MutableStylePropertySet> copyEditingProperties(StyleDeclarationType* style, EditingPropertiesToInclude type)
 {
-    if (type == EditingStyle::AllEditingProperties)
-        return style->copyPropertiesInSet(editingProperties, WTF_ARRAY_LENGTH(editingProperties));
-    return style->copyPropertiesInSet(editingProperties + 2, WTF_ARRAY_LENGTH(editingProperties) - 2);
+    if (type == AllEditingProperties)
+        return style->copyPropertiesInSet(editingProperties, numAllEditingProperties);
+    return style->copyPropertiesInSet(editingProperties, numInheritableEditingProperties);
 }
 
 static inline bool isEditingProperty(int id)
@@ -99,10 +103,24 @@ static inline bool isEditingProperty(int id)
     return false;
 }
 
-static PassRefPtr<MutableStylePropertySet> editingStyleFromComputedStyle(PassRefPtr<Node> node, EditingStyle::EditingPropertiesToInclude type = EditingStyle::OnlyInheritableEditingProperties)
+static PassRefPtr<MutableStylePropertySet> copyPropertiesFromComputedStyle(ComputedStyleExtractor& computedStyle, EditingStyle::PropertiesToInclude propertiesToInclude)
+{
+    switch (propertiesToInclude) {
+    case EditingStyle::AllProperties:
+        return computedStyle.copyProperties();
+    case EditingStyle::OnlyEditingInheritableProperties:
+        return copyEditingProperties(&computedStyle, OnlyInheritableEditingProperties);
+    case EditingStyle::EditingPropertiesInEffect:
+        return copyEditingProperties(&computedStyle, AllEditingProperties);
+    }
+    ASSERT_NOT_REACHED();
+    return 0;
+}
+
+static PassRefPtr<MutableStylePropertySet> copyPropertiesFromComputedStyle(Node* node, EditingStyle::PropertiesToInclude propertiesToInclude)
 {
     ComputedStyleExtractor computedStyle(node);
-    return copyEditingProperties(&computedStyle, type);
+    return copyPropertiesFromComputedStyle(computedStyle, propertiesToInclude);
 }
 
 static PassRefPtr<CSSValue> extractPropertyValue(const StylePropertySet* style, CSSPropertyID propertyID)
@@ -115,20 +133,6 @@ static PassRefPtr<CSSValue> extractPropertyValue(ComputedStyleExtractor* compute
     return computedStyle->propertyValue(propertyID);
 }
 
-static inline EditingStyle::EditingPropertiesToInclude toEditingProperties(EditingStyle::PropertiesToInclude properties)
-{
-    switch (properties) {
-    case EditingStyle::AllProperties:
-    case EditingStyle::EditingPropertiesInEffect:
-        return EditingStyle::AllEditingProperties;
-    case EditingStyle::OnlyEditingInheritableProperties:
-        return EditingStyle::OnlyInheritableEditingProperties;
-    }
-
-    ASSERT_NOT_REACHED();
-    return EditingStyle::OnlyInheritableEditingProperties;
-}
-
 template<typename T>
 int identifierForStyleProperty(T* style, CSSPropertyID propertyID)
 {
@@ -431,7 +435,10 @@ void EditingStyle::init(Node* node, PropertiesToInclude propertiesToInclude)
         node = node->parentNode();
 
     ComputedStyleExtractor computedStyleAtPosition(node);
-    m_mutableStyle = propertiesToInclude == AllProperties ? computedStyleAtPosition.copyProperties() : editingStyleFromComputedStyle(node);
+    // FIXME: It's strange to not set background-color and text-decoration when propertiesToInclude is EditingPropertiesInEffect.
+    // However editing/selection/contains-boundaries.html fails without this ternary.
+    m_mutableStyle = copyPropertiesFromComputedStyle(computedStyleAtPosition,
+        propertiesToInclude == EditingPropertiesInEffect ? OnlyEditingInheritableProperties : propertiesToInclude);
 
     if (propertiesToInclude == EditingPropertiesInEffect) {
         if (RefPtr<CSSValue> value = backgroundColorInEffect(node))
@@ -599,8 +606,8 @@ void EditingStyle::removeStyleAddedByNode(Node* node)
 {
     if (!node || !node->parentNode())
         return;
-    RefPtr<MutableStylePropertySet> parentStyle = editingStyleFromComputedStyle(node->parentNode(), AllEditingProperties);
-    RefPtr<MutableStylePropertySet> nodeStyle = editingStyleFromComputedStyle(node, AllEditingProperties);
+    RefPtr<MutableStylePropertySet> parentStyle = copyPropertiesFromComputedStyle(node->parentNode(), EditingPropertiesInEffect);
+    RefPtr<MutableStylePropertySet> nodeStyle = copyPropertiesFromComputedStyle(node, EditingPropertiesInEffect);
     nodeStyle->removeEquivalentProperties(parentStyle.get());
     m_mutableStyle->removeEquivalentProperties(nodeStyle.get());
 }
@@ -610,8 +617,8 @@ void EditingStyle::removeStyleConflictingWithStyleOfNode(Node* node)
     if (!node || !node->parentNode() || !m_mutableStyle)
         return;
 
-    RefPtr<MutableStylePropertySet> parentStyle = editingStyleFromComputedStyle(node->parentNode(), AllEditingProperties);
-    RefPtr<MutableStylePropertySet> nodeStyle = editingStyleFromComputedStyle(node, AllEditingProperties);
+    RefPtr<MutableStylePropertySet> parentStyle = copyPropertiesFromComputedStyle(node->parentNode(), EditingPropertiesInEffect);
+    RefPtr<MutableStylePropertySet> nodeStyle = copyPropertiesFromComputedStyle(node, EditingPropertiesInEffect);
     nodeStyle->removeEquivalentProperties(parentStyle.get());
 
     unsigned propertyCount = nodeStyle->propertyCount();
@@ -619,12 +626,6 @@ void EditingStyle::removeStyleConflictingWithStyleOfNode(Node* node)
         m_mutableStyle->removeProperty(nodeStyle->propertyAt(i).id());
 }
 
-void EditingStyle::removeAllButEditingProperties(EditingPropertiesToInclude propertiesToInclude)
-{
-    if (m_mutableStyle)
-        m_mutableStyle = copyEditingProperties(m_mutableStyle.get(), propertiesToInclude);
-}
-
 void EditingStyle::collapseTextDecorationProperties()
 {
     if (!m_mutableStyle)
@@ -981,11 +982,28 @@ static inline bool elementMatchesAndPropertyIsNotInInlineStyleDecl(const HTMLEle
         && (mode == EditingStyle::OverrideValues || !equivalent->propertyExistsInStyle(style));
 }
 
+static PassRefPtr<MutableStylePropertySet> extractEditingProperties(const StylePropertySet* style, EditingStyle::PropertiesToInclude propertiesToInclude)
+{
+    if (!style)
+        return 0;
+
+    switch (propertiesToInclude) {
+    case EditingStyle::AllProperties:
+    case EditingStyle::EditingPropertiesInEffect:
+        return copyEditingProperties(style, AllEditingProperties);
+    case EditingStyle::OnlyEditingInheritableProperties:
+        return copyEditingProperties(style, OnlyInheritableEditingProperties);
+    }
+
+    ASSERT_NOT_REACHED();
+    return 0;
+}
+
 void EditingStyle::mergeInlineAndImplicitStyleOfElement(StyledElement* element, CSSPropertyOverrideMode mode, PropertiesToInclude propertiesToInclude)
 {
     RefPtr<EditingStyle> styleFromRules = EditingStyle::create();
     styleFromRules->mergeStyleFromRulesForSerialization(element);
-    styleFromRules->removeAllButEditingProperties(toEditingProperties(propertiesToInclude));
+    styleFromRules->m_mutableStyle = extractEditingProperties(styleFromRules->m_mutableStyle.get(), propertiesToInclude);
     mergeStyle(styleFromRules->m_mutableStyle.get(), mode);
 
     mergeInlineStyleOfElement(element, mode, propertiesToInclude);
index 45c4ed0033ffdfd2c2775c1d5fa2df1cf91a2b28..d17a90dffc55380c7e0f88901bee6b9205918ff2 100644 (file)
@@ -64,7 +64,6 @@ class EditingStyle : public RefCounted<EditingStyle> {
 public:
 
     enum PropertiesToInclude { AllProperties, OnlyEditingInheritableProperties, EditingPropertiesInEffect };
-    enum EditingPropertiesToInclude { OnlyInheritableEditingProperties, AllEditingProperties };
 
     enum ShouldPreserveWritingDirection { PreserveWritingDirection, DoNotPreserveWritingDirection };
     enum ShouldExtractMatchingStyle { ExtractMatchingStyle, DoNotExtractMatchingStyle };
@@ -109,7 +108,6 @@ public:
     void removeBlockProperties();
     void removeStyleAddedByNode(Node*);
     void removeStyleConflictingWithStyleOfNode(Node*);
-    void removeAllButEditingProperties(EditingPropertiesToInclude = OnlyInheritableEditingProperties);
     void collapseTextDecorationProperties();
     enum ShouldIgnoreTextOnlyProperties { IgnoreTextOnlyProperties, DoNotIgnoreTextOnlyProperties };
     TriState triStateOfStyle(EditingStyle*) const;