- Revision
- 151772
- Author
- [email protected]
- Date
- 2013-06-20 00:39:43 -0700 (Thu, 20 Jun 2013)
Log Message
background-color and text-decoration are not preserved when merging paragraphs
https://bugs.webkit.org/show_bug.cgi?id=116215
Reviewed by Ryosuke Niwa.
Source/WebCore:
In mergeInlineAndImplicitStyleOfElement(), style from matched rules are retrieved.
If they contain either background-color or text-decoration they will be removed
by removeNonEditingProperties() due to copyEditingProperties() implicit
'OnlyInheritableEditingProperties' parameter.
We have already had similar issue at https://bugs.webkit.org/show_bug.cgi?id=53196
Tests: editing/deleting/merge-div-from-span-with-style.html
editing/deleting/merge-paragraph-from-span-with-multiple-text-decoration.html
editing/deleting/merge-paragraph-from-span-with-style.html
* editing/EditingStyle.cpp:
(WebCore::copyEditingProperties):
(WebCore::editingStyleFromComputedStyle):
Add EditingStyle:: due to move EditingPropertiesToInclude to EditingStyle.h.
(WebCore::toEditingProperties):
Add a helper to easy convert PropertiesToInclude to EditingPropertiesToInclude,
needed in mergeInlineAndImplicitStyleOfElement().
(WebCore::EditingStyle::removeAllButEditingProperties):
Add a parameter to choose which editing properties should be preserved while
removing all properties (OnlyInheritableEditingProperties implicite).
(WebCore::EditingStyle::mergeInlineAndImplicitStyleOfElement):
Call removeAllButEditingProperties with parameter.
* editing/EditingStyle.h:
Move EditingPropertiesType to the header as it's needed by removeAllButEditingProperties().
Change EditingPropertiesType to EditingPropertiesToInclude to be consistent with
PropertiesToInclude.
Change removeNonEditingProperties() to removeAllButEditingProperties().
LayoutTests:
Added tests for verifying merge of paragraph/div from span with non-inline styles.
* editing/deleting/merge-div-from-span-with-style-expected.txt: Added.
* editing/deleting/merge-div-from-span-with-style.html: Added.
* editing/deleting/merge-paragraph-from-span-with-multiple-text-decoration-expected.txt: Added.
* editing/deleting/merge-paragraph-from-span-with-multiple-text-decoration.html: Added.
* editing/deleting/merge-paragraph-from-span-with-style-expected.txt: Added.
* editing/deleting/merge-paragraph-from-span-with-style.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (151771 => 151772)
--- trunk/LayoutTests/ChangeLog 2013-06-20 05:44:42 UTC (rev 151771)
+++ trunk/LayoutTests/ChangeLog 2013-06-20 07:39:43 UTC (rev 151772)
@@ -1,3 +1,19 @@
+2013-06-20 Grzegorz Czajkowski <[email protected]>
+
+ background-color and text-decoration are not preserved when merging paragraphs
+ https://bugs.webkit.org/show_bug.cgi?id=116215
+
+ Reviewed by Ryosuke Niwa.
+
+ Added tests for verifying merge of paragraph/div from span with non-inline styles.
+
+ * editing/deleting/merge-div-from-span-with-style-expected.txt: Added.
+ * editing/deleting/merge-div-from-span-with-style.html: Added.
+ * editing/deleting/merge-paragraph-from-span-with-multiple-text-decoration-expected.txt: Added.
+ * editing/deleting/merge-paragraph-from-span-with-multiple-text-decoration.html: Added.
+ * editing/deleting/merge-paragraph-from-span-with-style-expected.txt: Added.
+ * editing/deleting/merge-paragraph-from-span-with-style.html: Added.
+
2013-06-19 Beth Dakin <[email protected]>
Adding a Mac pixel result.
Added: trunk/LayoutTests/editing/deleting/merge-div-from-span-with-style-expected.txt (0 => 151772)
--- trunk/LayoutTests/editing/deleting/merge-div-from-span-with-style-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/deleting/merge-div-from-span-with-style-expected.txt 2013-06-20 07:39:43 UTC (rev 151772)
@@ -0,0 +1,9 @@
+The span style should be preserved when merging div element.
+The test passes if "bar" contatins the yellow background and it's underlined.
+| "
+"
+| <div>
+| "foo<#selection-caret>"
+| <span>
+| style="background-color: yellow; text-decoration: underline;"
+| "bar"
Added: trunk/LayoutTests/editing/deleting/merge-div-from-span-with-style.html (0 => 151772)
--- trunk/LayoutTests/editing/deleting/merge-div-from-span-with-style.html (rev 0)
+++ trunk/LayoutTests/editing/deleting/merge-div-from-span-with-style.html 2013-06-20 07:39:43 UTC (rev 151772)
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style type="text/css">
+.NonInheritableProperties {
+background-color: yellow;
+text-decoration: underline;
+}
+</style>
+</head>
+<body>
+<div id="test" contenteditable="true">
+<div>foo</div>
+<span id="spanToMerge" class="NonInheritableProperties">bar</span>
+</div>
+<script src=""
+<script>
+Markup.description('The span style should be preserved when merging div element.\n'
+ + 'The test passes if "bar" contatins the yellow background and it\'s underlined.');
+
+var spanToMerge = document.getElementById("spanToMerge");
+spanToMerge.focus();
+getSelection().collapse(spanToMerge, 0);
+document.execCommand("Delete");
+Markup.dump(document.getElementById("test"));
+</script>
+ </body>
+</html>
Added: trunk/LayoutTests/editing/deleting/merge-paragraph-from-span-with-multiple-text-decoration-expected.txt (0 => 151772)
--- trunk/LayoutTests/editing/deleting/merge-paragraph-from-span-with-multiple-text-decoration-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/deleting/merge-paragraph-from-span-with-multiple-text-decoration-expected.txt 2013-06-20 07:39:43 UTC (rev 151772)
@@ -0,0 +1,9 @@
+The span style should be preserved when merging a pragraph.
+The test passes if the "bar" has underline, overline and line through.
+| "
+"
+| <p>
+| "foo<#selection-caret>"
+| <span>
+| style="text-decoration: underline overline line-through;"
+| "bar"
Added: trunk/LayoutTests/editing/deleting/merge-paragraph-from-span-with-multiple-text-decoration.html (0 => 151772)
--- trunk/LayoutTests/editing/deleting/merge-paragraph-from-span-with-multiple-text-decoration.html (rev 0)
+++ trunk/LayoutTests/editing/deleting/merge-paragraph-from-span-with-multiple-text-decoration.html 2013-06-20 07:39:43 UTC (rev 151772)
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style type="text/css">
+.MultipleTextDecorations {
+text-decoration: underline overline line-through;
+}
+</style>
+</head>
+<body>
+<div id="test" contenteditable="true">
+<p>foo</p>
+<span id="spanToMerge" class="MultipleTextDecorations">bar</span>
+</div>
+<script src=""
+<script>
+Markup.description('The span style should be preserved when merging a pragraph.\n'
+ + 'The test passes if the "bar" has underline, overline and line through.');
+
+var spanToMerge = document.getElementById("spanToMerge");
+spanToMerge.focus();
+getSelection().collapse(spanToMerge, 0);
+document.execCommand("Delete");
+Markup.dump(document.getElementById("test"));
+</script>
+ </body>
+</html>
Added: trunk/LayoutTests/editing/deleting/merge-paragraph-from-span-with-style-expected.txt (0 => 151772)
--- trunk/LayoutTests/editing/deleting/merge-paragraph-from-span-with-style-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/deleting/merge-paragraph-from-span-with-style-expected.txt 2013-06-20 07:39:43 UTC (rev 151772)
@@ -0,0 +1,9 @@
+The span style should be preserved when merging a pragraph.
+The test passes if "bar" contatins the yellow background and it's underlined.
+| "
+"
+| <p>
+| "foo<#selection-caret>"
+| <span>
+| style="background-color: yellow; text-decoration: underline;"
+| "bar"
Added: trunk/LayoutTests/editing/deleting/merge-paragraph-from-span-with-style.html (0 => 151772)
--- trunk/LayoutTests/editing/deleting/merge-paragraph-from-span-with-style.html (rev 0)
+++ trunk/LayoutTests/editing/deleting/merge-paragraph-from-span-with-style.html 2013-06-20 07:39:43 UTC (rev 151772)
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style type="text/css">
+.NonInheritableProperties {
+background-color: yellow;
+text-decoration: underline;
+}
+</style>
+</head>
+<body>
+<div id="test" contenteditable="true">
+<p>foo</p>
+<span id="spanToMerge" class="NonInheritableProperties">bar</span>
+</div>
+<script src=""
+<script>
+Markup.description('The span style should be preserved when merging a pragraph.\n'
+ + 'The test passes if "bar" contatins the yellow background and it\'s underlined.');
+
+var spanToMerge = document.getElementById("spanToMerge");
+spanToMerge.focus();
+getSelection().collapse(spanToMerge, 0);
+document.execCommand("Delete");
+Markup.dump(document.getElementById("test"));
+</script>
+ </body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (151771 => 151772)
--- trunk/Source/WebCore/ChangeLog 2013-06-20 05:44:42 UTC (rev 151771)
+++ trunk/Source/WebCore/ChangeLog 2013-06-20 07:39:43 UTC (rev 151772)
@@ -1,3 +1,42 @@
+2013-06-20 Grzegorz Czajkowski <[email protected]>
+
+ background-color and text-decoration are not preserved when merging paragraphs
+ https://bugs.webkit.org/show_bug.cgi?id=116215
+
+ Reviewed by Ryosuke Niwa.
+
+ In mergeInlineAndImplicitStyleOfElement(), style from matched rules are retrieved.
+ If they contain either background-color or text-decoration they will be removed
+ by removeNonEditingProperties() due to copyEditingProperties() implicit
+ 'OnlyInheritableEditingProperties' parameter.
+ We have already had similar issue at https://bugs.webkit.org/show_bug.cgi?id=53196
+
+ Tests: editing/deleting/merge-div-from-span-with-style.html
+ editing/deleting/merge-paragraph-from-span-with-multiple-text-decoration.html
+ editing/deleting/merge-paragraph-from-span-with-style.html
+
+ * editing/EditingStyle.cpp:
+ (WebCore::copyEditingProperties):
+ (WebCore::editingStyleFromComputedStyle):
+ Add EditingStyle:: due to move EditingPropertiesToInclude to EditingStyle.h.
+
+ (WebCore::toEditingProperties):
+ Add a helper to easy convert PropertiesToInclude to EditingPropertiesToInclude,
+ needed in mergeInlineAndImplicitStyleOfElement().
+
+ (WebCore::EditingStyle::removeAllButEditingProperties):
+ Add a parameter to choose which editing properties should be preserved while
+ removing all properties (OnlyInheritableEditingProperties implicite).
+
+ (WebCore::EditingStyle::mergeInlineAndImplicitStyleOfElement):
+ Call removeAllButEditingProperties with parameter.
+
+ * editing/EditingStyle.h:
+ Move EditingPropertiesType to the header as it's needed by removeAllButEditingProperties().
+ Change EditingPropertiesType to EditingPropertiesToInclude to be consistent with
+ PropertiesToInclude.
+ Change removeNonEditingProperties() to removeAllButEditingProperties().
+
2013-06-19 Christophe Dumez <[email protected]>
The order of the generated supplemental code is not guaranteed
Modified: trunk/Source/WebCore/editing/EditingStyle.cpp (151771 => 151772)
--- trunk/Source/WebCore/editing/EditingStyle.cpp 2013-06-20 05:44:42 UTC (rev 151771)
+++ trunk/Source/WebCore/editing/EditingStyle.cpp 2013-06-20 07:39:43 UTC (rev 151772)
@@ -83,12 +83,10 @@
CSSPropertyWebkitTextStrokeWidth,
};
-enum EditingPropertiesType { OnlyInheritableEditingProperties, AllEditingProperties };
-
template <class StyleDeclarationType>
-static PassRefPtr<MutableStylePropertySet> copyEditingProperties(StyleDeclarationType* style, EditingPropertiesType type = OnlyInheritableEditingProperties)
+static PassRefPtr<MutableStylePropertySet> copyEditingProperties(StyleDeclarationType* style, EditingStyle::EditingPropertiesToInclude type = EditingStyle::OnlyInheritableEditingProperties)
{
- if (type == AllEditingProperties)
+ if (type == EditingStyle::AllEditingProperties)
return style->copyPropertiesInSet(editingProperties, WTF_ARRAY_LENGTH(editingProperties));
return style->copyPropertiesInSet(editingProperties + 2, WTF_ARRAY_LENGTH(editingProperties) - 2);
}
@@ -102,7 +100,7 @@
return false;
}
-static PassRefPtr<MutableStylePropertySet> editingStyleFromComputedStyle(PassRefPtr<Node> node, EditingPropertiesType type = OnlyInheritableEditingProperties)
+static PassRefPtr<MutableStylePropertySet> editingStyleFromComputedStyle(PassRefPtr<Node> node, EditingStyle::EditingPropertiesToInclude type = EditingStyle::OnlyInheritableEditingProperties)
{
ComputedStyleExtractor computedStyle(node);
return copyEditingProperties(&computedStyle, type);
@@ -118,6 +116,20 @@
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)
{
@@ -608,10 +620,10 @@
m_mutableStyle->removeProperty(nodeStyle->propertyAt(i).id());
}
-void EditingStyle::removeNonEditingProperties()
+void EditingStyle::removeAllButEditingProperties(EditingPropertiesToInclude propertiesToInclude)
{
if (m_mutableStyle)
- m_mutableStyle = copyEditingProperties(m_mutableStyle.get());
+ m_mutableStyle = copyEditingProperties(m_mutableStyle.get(), propertiesToInclude);
}
void EditingStyle::collapseTextDecorationProperties()
@@ -974,7 +986,7 @@
{
RefPtr<EditingStyle> styleFromRules = EditingStyle::create();
styleFromRules->mergeStyleFromRulesForSerialization(element);
- styleFromRules->removeNonEditingProperties();
+ styleFromRules->removeAllButEditingProperties(toEditingProperties(propertiesToInclude));
mergeStyle(styleFromRules->m_mutableStyle.get(), mode);
mergeInlineStyleOfElement(element, mode, propertiesToInclude);
Modified: trunk/Source/WebCore/editing/EditingStyle.h (151771 => 151772)
--- trunk/Source/WebCore/editing/EditingStyle.h 2013-06-20 05:44:42 UTC (rev 151771)
+++ trunk/Source/WebCore/editing/EditingStyle.h 2013-06-20 07:39:43 UTC (rev 151772)
@@ -63,6 +63,8 @@
public:
enum PropertiesToInclude { AllProperties, OnlyEditingInheritableProperties, EditingPropertiesInEffect };
+ enum EditingPropertiesToInclude { OnlyInheritableEditingProperties, AllEditingProperties };
+
enum ShouldPreserveWritingDirection { PreserveWritingDirection, DoNotPreserveWritingDirection };
enum ShouldExtractMatchingStyle { ExtractMatchingStyle, DoNotExtractMatchingStyle };
static float NoFontDelta;
@@ -106,7 +108,7 @@
void removeBlockProperties();
void removeStyleAddedByNode(Node*);
void removeStyleConflictingWithStyleOfNode(Node*);
- void removeNonEditingProperties();
+ void removeAllButEditingProperties(EditingPropertiesToInclude = OnlyInheritableEditingProperties);
void collapseTextDecorationProperties();
enum ShouldIgnoreTextOnlyProperties { IgnoreTextOnlyProperties, DoNotIgnoreTextOnlyProperties };
TriState triStateOfStyle(EditingStyle*) const;
