Deleting across multiple paragraphs can change the style of surrounding text
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Oct 2012 18:26:37 +0000 (18:26 +0000)
committerGrzegorz Czajkowski <g.czajkowski@samsung.com>
Mon, 3 Jun 2013 12:56:19 +0000 (14:56 +0200)
https://bugs.webkit.org/show_bug.cgi?id=97266

Reviewed by Levi Weintraub.

Source/WebCore:

Preserve editing styles from CSS rules in wrappingStyleForSerialization as well as inline styles
even when we're not annotating. We don't want to preserve all styles because it's against
the user expectation to keep borders, etc... when merging paragraphs. We also don't want to copy
styles from a mail blockquote because that's not a style the user has applied. See the comment
in EditingStyle::wrappingStyleForSerialization.

Test: editing/deleting/merge-paragraph-with-style-from-rule.html

* editing/EditingStyle.cpp:
(WebCore::EditingStyle::mergeInlineAndImplicitStyleOfElement):
(WebCore::EditingStyle::wrappingStyleForSerialization):

LayoutTests:

Added a regression test and rebaselined tests.

* editing/deleting/delete-before-block-image-2-expected.txt:
* editing/deleting/merge-paragraph-from-p-with-style-expected.txt:
* editing/deleting/merge-paragraph-from-p-with-style.html: We changed the behavior. The editing style of p
is now preserved when merging paragraphs.
* editing/deleting/merge-paragraph-with-style-from-rule-expected.txt: Added.
* editing/deleting/merge-paragraph-with-style-from-rule.html: Added.
* platform/mac/editing/deleting/delete-block-merge-contents-001-expected.txt:
* platform/mac/editing/pasteboard/merge-end-blockquote-expected.txt:

Change-Id: I1965220755b35616dbb6c48608047b241a3b3d5f
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@130532 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/editing/deleting/delete-before-block-image-2-expected.txt
LayoutTests/editing/deleting/merge-paragraph-from-p-with-style-expected.txt
LayoutTests/editing/deleting/merge-paragraph-from-p-with-style.html
LayoutTests/editing/deleting/merge-paragraph-with-style-from-rule-expected.txt [new file with mode: 0644]
LayoutTests/editing/deleting/merge-paragraph-with-style-from-rule.html [new file with mode: 0644]
LayoutTests/platform/mac/editing/deleting/delete-block-merge-contents-001-expected.txt
LayoutTests/platform/mac/editing/pasteboard/merge-end-blockquote-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/editing/EditingStyle.cpp

index 8160869..b127e7b 100644 (file)
@@ -1,3 +1,21 @@
+2012-10-05  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Deleting across multiple paragraphs can change the style of surrounding text
+        https://bugs.webkit.org/show_bug.cgi?id=97266
+
+        Reviewed by Levi Weintraub.
+
+        Added a regression test and rebaselined tests.
+
+        * editing/deleting/delete-before-block-image-2-expected.txt:
+        * editing/deleting/merge-paragraph-from-p-with-style-expected.txt:
+        * editing/deleting/merge-paragraph-from-p-with-style.html: We changed the behavior. The editing style of p
+        is now preserved when merging paragraphs.
+        * editing/deleting/merge-paragraph-with-style-from-rule-expected.txt: Added.
+        * editing/deleting/merge-paragraph-with-style-from-rule.html: Added.
+        * platform/mac/editing/deleting/delete-block-merge-contents-001-expected.txt:
+        * platform/mac/editing/pasteboard/merge-end-blockquote-expected.txt:
+
 2012-09-24  Tony Chang  <tony@chromium.org>
 
         flex-grow should be 1 when omitted from flex shorthand
index c65abd4..021b9ed 100644 (file)
@@ -1,3 +1,3 @@
-<img id="img" style="display:block;" src="../resources/abe.png">
+<img id="img" src="../resources/abe.png" style="display: block;">
 
 Selection: [[object HTMLDivElement], 0]
index f645db8..9553591 100644 (file)
@@ -8,11 +8,11 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: webViewDidEndEditing:WebViewDidEndEditingNotification
 This tests deleting line break before p.
-"helloworld" should both be in black text (no p element) and "world" should be italicized.
+"world" in "helloworld" should be italicized in red.
 | "
 hello<#selection-caret>"
 | <span>
-|   style="font-style: italic; "
+|   style="color: red; font-style: italic;"
 |   "world"
 | "
 "
index 1b7feed..c740d9e 100644 (file)
@@ -7,7 +7,7 @@ p {color: red;}
 </head>
 <body>
 <p id="description">This tests deleting line break before p.
-"helloworld" should both be in black text (no p element) and "world" should be italicized.</p>
+"world" in "helloworld" should be italicized in red.</p>
 <div id="test" contenteditable>
 hello
 <p style="font-style: italic;">world</p>
diff --git a/LayoutTests/editing/deleting/merge-paragraph-with-style-from-rule-expected.txt b/LayoutTests/editing/deleting/merge-paragraph-with-style-from-rule-expected.txt
new file mode 100644 (file)
index 0000000..550f66f
--- /dev/null
@@ -0,0 +1,18 @@
+You should see "hello webKit" all in blue:
+| "
+"
+| <p>
+|   <span>
+|     class="someClass"
+|     style="font-size: 12pt;"
+|     "hello"
+|   " "
+|   <span>
+|     class="someClass"
+|     style="font-size: 12pt;"
+|     "w<#selection-caret>"
+|   <span>
+|     style="color: blue; font-size: 12pt;"
+|     "ebKit"
+| "
+"
diff --git a/LayoutTests/editing/deleting/merge-paragraph-with-style-from-rule.html b/LayoutTests/editing/deleting/merge-paragraph-with-style-from-rule.html
new file mode 100644 (file)
index 0000000..d388c23
--- /dev/null
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+<body>
+<style type="text/css">
+.someClass {
+    color: blue;
+}
+</style>
+<p id="description">You should see "hello webKit" all in blue:</p>
+<div id="editor" contenteditable>
+<p><span style="font-size: 12pt;" class="someClass">hello</span> <span style="font-size: 12pt;" class="someClass">world</span></p>
+<p><span style="font-size: 12pt;" class="someClass">WebKit</span></p>
+</div>
+<div id="log"></div>
+<script src="../../resources/dump-as-markup.js"></script>
+<script>
+
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+document.getElementById('editor').focus();
+for (var i = 0; i < 7; i++)
+    getSelection().modify('move', 'forward', 'character');
+for (var i = 0; i < 6; i++)
+    getSelection().modify('extend', 'forward', 'character');
+document.execCommand('delete');
+
+Markup.description(document.getElementById('description').textContent);
+Markup.dump('editor');
+
+</script>
+</body>
+</html>
index 70b47de..925b810 100644 (file)
@@ -18,4 +18,5 @@ layer at (0,0) size 800x600
         RenderBlock {DIV} at (14,14) size 756x28
           RenderText {#text} at (0,0) size 84x28
             text run at (0,0) width 84: "OneTwo"
+          RenderText {#text} at (0,0) size 0x0
 caret: position 4 of child 0 {#text} of child 1 {DIV} of child 1 {DIV} of body
index b3bed73..4e3199d 100644 (file)
@@ -1,7 +1,7 @@
 EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 1 of DIV > BODY > HTML > #document
 EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document toDOMRange:range from 16 of #text > DIV > BODY > HTML > #document to 16 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 17 of #text > DIV > BODY > HTML > #document to 17 of #text > DIV > BODY > HTML > #document toDOMRange:range from 16 of #text > DIV > BODY > HTML > #document to 16 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: shouldEndEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 1 of DIV > BODY > HTML > #document
@@ -11,7 +11,7 @@ EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 14 of #text > DIV > BLOCKQUOTE > DIV > BODY > HTML > #document to 14 of #text > DIV > BLOCKQUOTE > DIV > BODY > HTML > #document toDOMRange:range from 4 of #text > DIV > BLOCKQUOTE > DIV > BODY > HTML > #document to 4 of #text > DIV > BLOCKQUOTE > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 14 of #text > DIV > BLOCKQUOTE > DIV > BODY > HTML > #document to 14 of #text > DIV > BLOCKQUOTE > DIV > BODY > HTML > #document toDOMRange:range from 3 of #text > DIV > BLOCKQUOTE > DIV > BODY > HTML > #document to 3 of #text > DIV > BLOCKQUOTE > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 layer at (0,0) size 800x600
@@ -41,4 +41,4 @@ layer at (0,0) size 800x600
           RenderBlock {DIV} at (0,0) size 704x18
             RenderText {#text} at (0,0) size 49x18
               text run at (0,0) width 49: "barText"
-caret: position 4 of child 0 {#text} of child 0 {DIV} of child 1 {BLOCKQUOTE} of child 10 {DIV} of body
+caret: position 3 of child 0 {#text} of child 0 {DIV} of child 1 {BLOCKQUOTE} of child 10 {DIV} of body
index 1a1078c..7fedc05 100644 (file)
@@ -1,3 +1,22 @@
+2012-10-05  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Deleting across multiple paragraphs can change the style of surrounding text
+        https://bugs.webkit.org/show_bug.cgi?id=97266
+
+        Reviewed by Levi Weintraub.
+
+        Preserve editing styles from CSS rules in wrappingStyleForSerialization as well as inline styles
+        even when we're not annotating. We don't want to preserve all styles because it's against
+        the user expectation to keep borders, etc... when merging paragraphs. We also don't want to copy
+        styles from a mail blockquote because that's not a style the user has applied. See the comment
+        in EditingStyle::wrappingStyleForSerialization.
+
+        Test: editing/deleting/merge-paragraph-with-style-from-rule.html
+
+        * editing/EditingStyle.cpp:
+        (WebCore::EditingStyle::mergeInlineAndImplicitStyleOfElement):
+        (WebCore::EditingStyle::wrappingStyleForSerialization):
+
 2012-09-24  Tony Chang  <tony@chromium.org>
 
         flex-grow should be 1 when omitted from flex shorthand
index a486e17..d166f9d 100644 (file)
@@ -980,6 +980,11 @@ static inline bool elementMatchesAndPropertyIsNotInInlineStyleDecl(const HTMLEle
 
 void EditingStyle::mergeInlineAndImplicitStyleOfElement(StyledElement* element, CSSPropertyOverrideMode mode, PropertiesToInclude propertiesToInclude)
 {
+    RefPtr<EditingStyle> styleFromRules = EditingStyle::create();
+    styleFromRules->mergeStyleFromRulesForSerialization(element);
+    styleFromRules->removeNonEditingProperties();
+    mergeStyle(styleFromRules->m_mutableStyle.get(), mode);
+
     mergeInlineStyleOfElement(element, mode, propertiesToInclude);
 
     const Vector<OwnPtr<HTMLElementEquivalent> >& elementEquivalents = htmlElementEquivalents();
@@ -1018,7 +1023,7 @@ PassRefPtr<EditingStyle> EditingStyle::wrappingStyleForSerialization(Node* conte
 
     // When not annotating for interchange, we only preserve inline style declarations.
     for (Node* node = context; node && !node->isDocumentNode(); node = node->parentNode()) {
-        if (node->isStyledElement()) {
+        if (node->isStyledElement() && !isMailBlockquote(node)) {
             wrappingStyle->mergeInlineAndImplicitStyleOfElement(static_cast<StyledElement*>(node), EditingStyle::DoNotOverrideValues,
                 EditingStyle::EditingPropertiesInEffect);
         }