Need a way to produce leaner markup when pasting a fragment containing verbose markup
authorenrica@apple.com <enrica@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Dec 2011 00:32:27 +0000 (00:32 +0000)
committerenrica@apple.com <enrica@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Dec 2011 00:32:27 +0000 (00:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=74514
<rdar://problem/10208653>

Reviewed by Ryosuke Niwa.

Source/WebCore:

This patch is another step in the direction of reducing the verbosity of the markup
produced with editing operations.
After the copied fragment is inserted in the document, it is analyzed to remove all
the elements that don't contribute to the style. The decision is made comparing the
render styles. As part of the cleanup, unstyled divs with single child element are
removed. The logic to determine the blocks that can be removed is the same used in
DeleteSelectionCommand and has been moved in CompositeEditCommand.

Test: editing/pasteboard/paste-and-sanitize.html

* editing/CompositeEditCommand.cpp:
(WebCore::CompositeEditCommand::isRemovableBlock): Implements logic to determine
if a block can be removed.
* editing/CompositeEditCommand.h: Added isRemovableBlock declaration.
* editing/DeleteSelectionCommand.cpp:
(WebCore::DeleteSelectionCommand::removeRedundantBlocks): Implemented using
isRemovableBlock from CompositeEditCommand.
* editing/Editor.cpp:
(WebCore::Editor::replaceSelectionWithFragment): Added SanitizeFragment option.
* editing/ReplaceSelectionCommand.cpp:
(WebCore::ReplaceSelectionCommand::ReplaceSelectionCommand): Added initialization
of m_sanitizeFragment member.
(WebCore::ReplaceSelectionCommand::removeRedundantMarkup): New method implementing
the cleanup logic.
(WebCore::ReplaceSelectionCommand::doApply): Added call to removeRedundantMarkup
after the fragment is inserted in the document.
* editing/ReplaceSelectionCommand.h: Added new value to the enum CommandOption,
a new member variable and the new method declaration.

LayoutTests:

* editing/pasteboard/paste-and-sanitize-expected.txt: Added.
* editing/pasteboard/paste-and-sanitize.html: Added.
* editing/pasteboard/paste-text-012-expected.txt: Updated to reflect cleanup.
* editing/pasteboard/testcase-9507-expected.txt: Updated to reflect cleanup.
* platform/mac/editing/pasteboard/paste-text-013-expected.txt: Updated to reflect cleanup.
* platform/mac/editing/pasteboard/paste-text-014-expected.txt: Updated to reflect cleanup.

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

14 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/pasteboard/paste-and-sanitize-expected.txt [new file with mode: 0644]
LayoutTests/editing/pasteboard/paste-and-sanitize.html [new file with mode: 0644]
LayoutTests/editing/pasteboard/paste-text-012-expected.txt
LayoutTests/editing/pasteboard/testcase-9507-expected.txt
LayoutTests/platform/mac/editing/pasteboard/paste-text-013-expected.txt
LayoutTests/platform/mac/editing/pasteboard/paste-text-014-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/editing/CompositeEditCommand.cpp
Source/WebCore/editing/CompositeEditCommand.h
Source/WebCore/editing/DeleteSelectionCommand.cpp
Source/WebCore/editing/Editor.cpp
Source/WebCore/editing/ReplaceSelectionCommand.cpp
Source/WebCore/editing/ReplaceSelectionCommand.h

index 4a19455..83acc1d 100644 (file)
@@ -1,3 +1,18 @@
+2011-12-14  Enrica Casucci  <enrica@apple.com>
+
+        Need a way to produce leaner markup when pasting a fragment containing verbose markup
+        https://bugs.webkit.org/show_bug.cgi?id=74514
+        <rdar://problem/10208653>
+
+        Reviewed by Ryosuke Niwa.
+
+        * editing/pasteboard/paste-and-sanitize-expected.txt: Added.
+        * editing/pasteboard/paste-and-sanitize.html: Added.
+        * editing/pasteboard/paste-text-012-expected.txt: Updated to reflect cleanup.
+        * editing/pasteboard/testcase-9507-expected.txt: Updated to reflect cleanup.
+        * platform/mac/editing/pasteboard/paste-text-013-expected.txt: Updated to reflect cleanup.
+        * platform/mac/editing/pasteboard/paste-text-014-expected.txt: Updated to reflect cleanup.
+
 2011-12-14  Kenneth Russell  <kbr@google.com>
 
         Unreviewed Chromium test expectations update. Skip a problematic
diff --git a/LayoutTests/editing/pasteboard/paste-and-sanitize-expected.txt b/LayoutTests/editing/pasteboard/paste-and-sanitize-expected.txt
new file mode 100644 (file)
index 0000000..31b3f04
--- /dev/null
@@ -0,0 +1,17 @@
+This test checks that the paste operation trims the pasted fragment to reduce the verbosity of the markup without affecting the style.
+
+PASS confirmedMarkup is 'Hello'
+PASS confirmedMarkup is '<b><i>Hello</i></b>'
+PASS confirmedMarkup is '<b><i>Hello</i></b>'
+PASS confirmedMarkup is 'Hello'
+PASS confirmedMarkup is '<b><i>Hello</i></b>'
+PASS confirmedMarkup is '<div style="text-align: center;"><b>Hello</b></div>'
+PASS confirmedMarkup is '<div><b><i>hello</i></b></div><div><b><i>world</i></b></div>'
+PASS confirmedMarkup is '<b><i><span style="font-weight: normal; "><b><i>hello1</i></b><b><i>&nbsp;hello2</i></b></span></i></b>'
+PASS confirmedMarkup is '<i style="margin-top: 10px; margin-right: 10px; margin-bottom: 10px; margin-left: 10px; "><b><i style="margin-top: 10px; margin-right: 10px; margin-bottom: 10px; margin-left: 10px; ">hello</i></b></i>'
+PASS confirmedMarkup is '<b><i>Hello&nbsp;world</i></b>'
+PASS confirmedMarkup is '<b><i><span style="font-weight: normal; ">plain text<b><i>bold italic text</i></b></span></i></b>'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/editing/pasteboard/paste-and-sanitize.html b/LayoutTests/editing/pasteboard/paste-and-sanitize.html
new file mode 100644 (file)
index 0000000..1724553
--- /dev/null
@@ -0,0 +1,56 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../../fast/js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description">This test checks that the paste operation trims the pasted fragment to reduce the verbosity of the markup without affecting the style. </p>
+<div id="console"></div>
+<script>
+
+var sel = document.getSelection();
+var root = document.createElement("root");
+document.body.appendChild(root);
+
+
+function createEditable(tagName, markup) {
+    var node = document.createElement(tagName);
+    node.contentEditable = true;
+    node.innerHTML = markup;
+    return node;
+}
+
+function testPaste(tagName, originalMarkup, expected) {
+    var node = createEditable(tagName, originalMarkup);
+    root.appendChild(node);
+
+    node.focus();
+    document.execCommand("SelectAll", false);
+    document.execCommand("Copy", false);
+    document.execCommand("Paste", false);
+
+    confirmedMarkup = node.innerHTML;
+
+    shouldBe("confirmedMarkup", "'" + expected + "'");
+}
+
+testPaste("div", "Hello", "Hello");
+testPaste("div", "<b><i>Hello</i></b>", "<b><i>Hello</i></b>");
+testPaste("div", "<div><b><i><span style=\"font-weight: normal\"><b><i>Hello</i></b></span></i></b></div>", "<b><i>Hello</i></b>");
+testPaste("div", "<div><div><div>Hello</div></div></div>", "Hello");
+testPaste("div", "<div><b><div><i>Hello</i></div></b></div>", "<b><i>Hello</i></b>");
+testPaste("div", "<div><div style=\"text-align: center;\"><b>Hello</b></div></div>", "<div style=\"text-align: center;\"><b>Hello</b></div>");
+testPaste("div", "<div><b><i><span style=\"font-weight: normal\"><b><i>hello</i></b></span></i></b></div><div><b><i><span style=\"font-weight: normal\"><b><i>world</i></b></span></i></b></div>", 
+          "<div><b><i>hello</i></b></div><div><b><i>world</i></b></div>");
+testPaste("div", "<div><b><i><span style=\"font-weight: normal; \"><b><i>hello1</i></b><b><i> hello2</i></b></span></i></b></div>", "<b><i><span style=\"font-weight: normal; \"><b><i>hello1</i></b><b><i>&nbsp;hello2</i></b></span></i></b>");
+testPaste("div", "<i style=\"margin: 10px;\"><b><i style=\"margin: 10px;\">hello</i></b></i>",
+          "<i style=\"margin-top: 10px; margin-right: 10px; margin-bottom: 10px; margin-left: 10px; \"><b><i style=\"margin-top: 10px; margin-right: 10px; margin-bottom: 10px; margin-left: 10px; \">hello</i></b></i>");
+testPaste("div", "<div><b><i><span style=\"font-weight: normal\"><b><i>Hello <!-- comment -->world</i></b></span></i></b></div>", "<b><i>Hello&nbsp;world</i></b>");
+testPaste("div", "<div><b><i><span style=\"font-weight: normal\">plain text<b><i>bold italic text</i></b></span></i></b></div>", "<b><i><span style=\"font-weight: normal; \">plain text<b><i>bold italic text</i></b></span></i></b>");
+
+root.style.display = "none";
+
+</script>
+<script src="../../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
index 1138ddd..ebce665 100644 (file)
@@ -6,4 +6,4 @@ foo
 foo
 
 execCopyCommand: <div id="test" class="editing"><div><blockquote>foo</blockquote></div></div> <div class="editing"></div>
-execPasteCommand: <div id="test" class="editing"><div><blockquote>foo</blockquote></div></div> <div class="editing"><div id="test" class="editing"><div><blockquote>foo</blockquote></div><div><br></div></div></div>
+execPasteCommand: <div id="test" class="editing"><div><blockquote>foo</blockquote></div></div> <div class="editing"><div id="test" class="editing"><blockquote>foo</blockquote><div><br></div></div></div>
index 05d4fe7..7ce1f1a 100644 (file)
@@ -31,8 +31,7 @@ After paste:
 |   "foo"
 |   <div>
 |     style="color: rgb(255, 0, 0); "
-|     <div>
-|       "bar<#selection-caret>"
+|     "bar<#selection-caret>"
 |   <div>
 |     style="color: rgb(255, 0, 0);"
 |     "
index 074431a..f60b2a9 100644 (file)
@@ -40,10 +40,9 @@ layer at (0,0) size 800x600
         RenderText {#text} at (14,14) size 12x28
           text run at (14,14) width 12: "x"
       RenderBlock {DIV} at (0,238) size 784x132 [border: (2px solid #FF0000)]
-        RenderBlock {DIV} at (14,38) size 756x28
-          RenderBlock {BLOCKQUOTE} at (40,0) size 676x28
-            RenderText {#text} at (0,0) size 32x28
-              text run at (0,0) width 32: "foo"
+        RenderBlock {BLOCKQUOTE} at (54,38) size 676x28
+          RenderText {#text} at (0,0) size 32x28
+            text run at (0,0) width 32: "foo"
         RenderBlock {DIV} at (14,90) size 756x28
           RenderBR {BR} at (0,0) size 0x28
 caret: position 0 of child 0 {BR} of child 1 {DIV} of child 8 {DIV} of body
index c408e04..de6388b 100644 (file)
@@ -35,10 +35,9 @@ layer at (0,0) size 800x600
             RenderText {#text} at (0,0) size 32x28
               text run at (0,0) width 32: "foo"
       RenderBlock {DIV} at (0,164) size 784x104 [border: (2px solid #FF0000)]
-        RenderBlock {DIV} at (14,38) size 756x28
-          RenderBlock {BLOCKQUOTE} at (40,0) size 676x28
-            RenderText {#text} at (0,0) size 32x28
-              text run at (0,0) width 32: "foo"
+        RenderBlock {BLOCKQUOTE} at (54,38) size 676x28
+          RenderText {#text} at (0,0) size 32x28
+            text run at (0,0) width 32: "foo"
       RenderBlock {DIV} at (0,268) size 784x56 [border: (2px solid #FF0000)]
         RenderText {#text} at (14,14) size 12x28
           text run at (14,14) width 12: "x"
index f32b1ee..544b807 100644 (file)
@@ -1,3 +1,40 @@
+2011-12-14  Enrica Casucci  <enrica@apple.com>
+
+        Need a way to produce leaner markup when pasting a fragment containing verbose markup
+        https://bugs.webkit.org/show_bug.cgi?id=74514
+        <rdar://problem/10208653>
+
+        Reviewed by Ryosuke Niwa.
+
+        This patch is another step in the direction of reducing the verbosity of the markup
+        produced with editing operations.
+        After the copied fragment is inserted in the document, it is analyzed to remove all
+        the elements that don't contribute to the style. The decision is made comparing the
+        render styles. As part of the cleanup, unstyled divs with single child element are
+        removed. The logic to determine the blocks that can be removed is the same used in
+        DeleteSelectionCommand and has been moved in CompositeEditCommand.
+        
+        Test: editing/pasteboard/paste-and-sanitize.html
+
+        * editing/CompositeEditCommand.cpp:
+        (WebCore::CompositeEditCommand::isRemovableBlock): Implements logic to determine
+        if a block can be removed.
+        * editing/CompositeEditCommand.h: Added isRemovableBlock declaration.
+        * editing/DeleteSelectionCommand.cpp:
+        (WebCore::DeleteSelectionCommand::removeRedundantBlocks): Implemented using
+        isRemovableBlock from CompositeEditCommand.
+        * editing/Editor.cpp:
+        (WebCore::Editor::replaceSelectionWithFragment): Added SanitizeFragment option.
+        * editing/ReplaceSelectionCommand.cpp:
+        (WebCore::ReplaceSelectionCommand::ReplaceSelectionCommand): Added initialization
+        of m_sanitizeFragment member.
+        (WebCore::ReplaceSelectionCommand::removeRedundantMarkup): New method implementing
+        the cleanup logic.
+        (WebCore::ReplaceSelectionCommand::doApply): Added call to removeRedundantMarkup
+        after the fragment is inserted in the document.
+        * editing/ReplaceSelectionCommand.h: Added new value to the enum CommandOption,
+        a new member variable and the new method declaration.
+
 2011-12-14  Adrienne Walker  <enne@google.com>
 
         [chromium] Refactor tile drawing to be more data-driven
index 1599364..e05616d 100644 (file)
@@ -244,6 +244,19 @@ void CompositeEditCommand::insertLineBreak()
     applyCommandToComposite(InsertLineBreakCommand::create(document()));
 }
 
+bool CompositeEditCommand::isRemovableBlock(const Node* node)
+{
+    Node* parentNode = node->parentNode();
+    if ((parentNode && parentNode->firstChild() != parentNode->lastChild()) || !node->hasTagName(divTag))
+        return false;
+
+    const NamedNodeMap* attributeMap = node->attributes();
+    if (!attributeMap || attributeMap->isEmpty())
+        return true;
+    
+    return false;
+}
+
 void CompositeEditCommand::insertNodeBefore(PassRefPtr<Node> insertChild, PassRefPtr<Node> refChild)
 {
     ASSERT(!refChild->hasTagName(bodyTag));
index 6ec3127..6af74b4 100644 (file)
@@ -97,6 +97,7 @@ protected:
     void deleteSelection(bool smartDelete = false, bool mergeBlocksAfterDelete = true, bool replace = false, bool expandForSpecialElements = true);
     void deleteSelection(const VisibleSelection&, bool smartDelete = false, bool mergeBlocksAfterDelete = true, bool replace = false, bool expandForSpecialElements = true);
     virtual void deleteTextFromNode(PassRefPtr<Text>, unsigned offset, unsigned count);
+    bool isRemovableBlock(const Node*);
     void insertNodeAfter(PassRefPtr<Node>, PassRefPtr<Node> refChild);
     void insertNodeAt(PassRefPtr<Node>, const Position&);
     void insertNodeAtTabSpanPosition(PassRefPtr<Node>, const Position&);
index 3381de2..27f679f 100644 (file)
@@ -753,20 +753,14 @@ void DeleteSelectionCommand::removeRedundantBlocks()
     Node* rootNode = node->rootEditableElement();
    
     while (node != rootNode) {
-        Node* parentNode = node->parentNode();
-        if ((parentNode && parentNode->firstChild() != parentNode->lastChild()) || !node->hasTagName(divTag)) {
-            node = parentNode;
-            continue;
-        }
-        const NamedNodeMap* attributeMap = node->attributes();
-        if (!attributeMap || attributeMap->isEmpty()) {
+        if (isRemovableBlock(node)) {
             if (node == m_endingPosition.anchorNode())
                 updatePositionForNodeRemoval(m_endingPosition, node);
             
             CompositeEditCommand::removeNodePreservingChildren(node);
             node = m_endingPosition.anchorNode();
         } else
-            node = parentNode;
+            node = node->parentNode();
     }
 }
 
index 261f562..9856d99 100644 (file)
@@ -402,7 +402,7 @@ void Editor::replaceSelectionWithFragment(PassRefPtr<DocumentFragment> fragment,
     if (m_frame->selection()->isNone() || !fragment)
         return;
 
-    ReplaceSelectionCommand::CommandOptions options = ReplaceSelectionCommand::PreventNesting;
+    ReplaceSelectionCommand::CommandOptions options = ReplaceSelectionCommand::PreventNesting | ReplaceSelectionCommand::SanitizeFragment;
     if (selectReplacement)
         options |= ReplaceSelectionCommand::SelectReplacement;
     if (smartReplace)
index ebaac29..f61e0d1 100644 (file)
@@ -46,6 +46,7 @@
 #include "HTMLInterchange.h"
 #include "HTMLNames.h"
 #include "NodeList.h"
+#include "RenderInline.h"
 #include "RenderObject.h"
 #include "RenderText.h"
 #include "SmartReplace.h"
@@ -372,6 +373,7 @@ ReplaceSelectionCommand::ReplaceSelectionCommand(Document* document, PassRefPtr<
     , m_preventNesting(options & PreventNesting)
     , m_movingParagraph(options & MovingParagraph)
     , m_editAction(editAction)
+    , m_sanitizeFragment(options & SanitizeFragment)
     , m_shouldMergeEnd(false)
 {
 }
@@ -536,6 +538,54 @@ void ReplaceSelectionCommand::removeRedundantStylesAndKeepStyleSpanInline(Insert
     }
 }
 
+void ReplaceSelectionCommand::removeRedundantMarkup(InsertedNodes& insertedNodes)
+{
+    Node* pastEndNode = insertedNodes.pastLastLeaf();
+    Node* rootNode = insertedNodes.firstNodeInserted()->parentNode();
+    Vector<Node*> nodesToRemove;
+    
+    // Walk through the inserted nodes, to see if there are elements that could be removed
+    // without affecting the style. The goal is to produce leaner markup even when starting
+    // from a verbose fragment.
+    // We look at inline elements as well as non top level divs that don't have attributes. 
+    for (Node* node = insertedNodes.firstNodeInserted(); node && node != pastEndNode; node = node->traverseNextNode()) {
+        if (node->firstChild() || (node->isTextNode() && node->nextSibling()))
+            continue;
+        
+        Node* startingNode = node->parentNode();
+        RenderStyle* startingStyle = startingNode->renderStyle();
+        if (!startingStyle)
+            continue;
+        Node* currentNode = startingNode;
+        Node* topNodeWithStartingStyle = 0;
+        while (currentNode != rootNode) {
+            if (currentNode->parentNode() != rootNode && isRemovableBlock(currentNode))
+                nodesToRemove.append(currentNode);
+
+            currentNode = currentNode->parentNode();
+            if (!currentNode->renderer() || !currentNode->renderer()->isRenderInline() || toRenderInline(currentNode->renderer())->alwaysCreateLineBoxes())
+                continue;
+
+            if (currentNode && currentNode->firstChild() != currentNode->lastChild()) {
+                topNodeWithStartingStyle = 0;
+                break;
+            }
+            
+            unsigned context;
+            if (currentNode->renderStyle()->diff(startingStyle, context) == StyleDifferenceEqual)
+                topNodeWithStartingStyle = currentNode;
+            
+        }
+        if (topNodeWithStartingStyle) {
+            for (Node* node = startingNode; node != topNodeWithStartingStyle; node = node->parentNode())
+                nodesToRemove.append(node);
+        }
+    }
+    // we perform all the DOM mutations at once.
+    for (size_t i = 0; i < nodesToRemove.size(); ++i)
+        removeNodePreservingChildren(nodesToRemove[i]);
+}
+
 static inline bool nodeHasVisibleRenderText(Text* text)
 {
     return text->renderer() && toRenderText(text->renderer())->renderedTextLength() > 0;
@@ -1001,6 +1051,9 @@ void ReplaceSelectionCommand::doApply()
 
     removeRedundantStylesAndKeepStyleSpanInline(insertedNodes);
 
+    if (m_sanitizeFragment)
+        removeRedundantMarkup(insertedNodes);
+
     // Setup m_startOfInsertedContent and m_endOfInsertedContent. This should be the last two lines of code that access insertedNodes.
     m_startOfInsertedContent = firstPositionInOrBeforeNode(insertedNodes.firstNodeInserted());
     m_endOfInsertedContent = lastPositionInOrAfterNode(insertedNodes.lastLeafInserted());
index 7bdf78d..fcb55cc 100644 (file)
@@ -43,7 +43,8 @@ public:
         SmartReplace = 1 << 1,
         MatchStyle = 1 << 2,
         PreventNesting = 1 << 3,
-        MovingParagraph = 1 << 4
+        MovingParagraph = 1 << 4,
+        SanitizeFragment = 1 << 5
     };
 
     typedef unsigned CommandOptions;
@@ -88,6 +89,7 @@ private:
     void removeUnrenderedTextNodesAtEnds(InsertedNodes&);
     
     void removeRedundantStylesAndKeepStyleSpanInline(InsertedNodes&);
+    void removeRedundantMarkup(InsertedNodes&);
     void handleStyleSpans(InsertedNodes&);
     void handlePasteAsQuotationNode();
     
@@ -110,6 +112,7 @@ private:
     bool m_preventNesting;
     bool m_movingParagraph;
     EditAction m_editAction;
+    bool m_sanitizeFragment;
     bool m_shouldMergeEnd;
 };