WebCore:
authorjustin.garcia@apple.com <justin.garcia@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Nov 2008 18:30:36 +0000 (18:30 +0000)
committerjustin.garcia@apple.com <justin.garcia@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Nov 2008 18:30:36 +0000 (18:30 +0000)
2008-11-14  Justin Garcia  <justin.garcia@apple.com>

        Reviewed by Beth Dakin.

        <rdar://problem/4230923> "Make Plain Text" doesn't reset text alignment in single paragraph messages

        When applying block styles, we would add block properties to the body element, and Mail's
        Make Plain Text feature isn't equipped to remove those.  This could have been fixed on our side,
        but this change has the advantage that it fixes the bug on Tiger, where Mail does not plan future updates.

        We have code that puts the paragraphs that we're operating on into blocks of their own before
        adding or removing block properties from the blocks that enclose them.  We need to run this code
        when the enclosing block is the body element.

        * editing/CompositeEditCommand.cpp:
        (WebCore::CompositeEditCommand::insertNewDefaultParagraphElementAt): Added, moved code from moveParagraphContents
        to here.
        (WebCore::CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary): Renamed some variables for clarity.
        Added a few comments.  Fixed bug by not bailing out when the block about to be used is the body element.
        * editing/CompositeEditCommand.h:

LayoutTests:

2008-11-14  Justin Garcia  <justin.garcia@apple.com>

        Reviewed by Beth Dakin.

        <rdar://problem/4230923> "Make Plain Text" doesn't reset text alignment in single paragraph messages

        * editing/style/4230923-expected.txt: Added.
        * editing/style/4230923.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/editing/style/4230923-expected.txt [new file with mode: 0644]
LayoutTests/editing/style/4230923.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/editing/CompositeEditCommand.cpp
WebCore/editing/CompositeEditCommand.h

index 3c09bb9..9feb587 100644 (file)
@@ -1,3 +1,12 @@
+2008-11-14  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by Beth Dakin.
+
+        <rdar://problem/4230923> "Make Plain Text" doesn't reset text alignment in single paragraph messages
+
+        * editing/style/4230923-expected.txt: Added.
+        * editing/style/4230923.html: Added.
+
 2008-11-12  Alexey Proskuryakov  <ap@webkit.org>
 
         Reviewed by Darin Adler.
diff --git a/LayoutTests/editing/style/4230923-expected.txt b/LayoutTests/editing/style/4230923-expected.txt
new file mode 100644 (file)
index 0000000..3f5be37
--- /dev/null
@@ -0,0 +1,4 @@
+This tests to make sure that changing block properties of content whose enclosing block is the body doesn't put those properties on the body, since Mail finds it difficult to remove those properties later when necessary. Below is the DOM inside the body. There should be a div with text-align:center on it.
+
+<div style="text-align: center;"><br></div>
+
diff --git a/LayoutTests/editing/style/4230923.html b/LayoutTests/editing/style/4230923.html
new file mode 100644 (file)
index 0000000..44b66de
--- /dev/null
@@ -0,0 +1,15 @@
+<html>
+<head>
+<script>
+function runTest() {
+    if (window.layoutTestController)
+        window.layoutTestController.dumpAsText();
+    window.getSelection().setPosition(document.body, 0); 
+    document.execCommand("JustifyCenter");
+    document.body.innerText = "This tests to make sure that changing block properties of content whose enclosing block is the body doesn't put those properties on the body, since Mail finds it difficult to remove those properties later when necessary.  Below is the DOM inside the body.  There should be a div with text-align:center on it.\n\n" + document.body.innerHTML;
+        
+}
+</script>
+</head>
+<body contentEditable="true" onLoad="runTest();"></body>
+</html>
\ No newline at end of file
index be9b22a..659813c 100644 (file)
@@ -1,3 +1,24 @@
+2008-11-14  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by Beth Dakin.
+
+        <rdar://problem/4230923> "Make Plain Text" doesn't reset text alignment in single paragraph messages
+        
+        When applying block styles, we would add block properties to the body element, and Mail's
+        Make Plain Text feature isn't equipped to remove those.  This could have been fixed on our side,
+        but this change has the advantage that it fixes the bug on Tiger, where Mail does not plan future updates.
+        
+        We have code that puts the paragraphs that we're operating on into blocks of their own before
+        adding or removing block properties from the blocks that enclose them.  We need to run this code 
+        when the enclosing block is the body element.
+        
+        * editing/CompositeEditCommand.cpp:
+        (WebCore::CompositeEditCommand::insertNewDefaultParagraphElementAt): Added, moved code from moveParagraphContents
+        to here.
+        (WebCore::CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary): Renamed some variables for clarity.
+        Added a few comments.  Fixed bug by not bailing out when the block about to be used is the body element.
+        * editing/CompositeEditCommand.h:
+
 2008-11-14  Cameron Zwarich  <zwarich@apple.com>
 
         Reviewed by Darin Adler.
@@ -54,7 +75,7 @@
 
         * platform/text/AtomicString.cpp:
         (WebCore::AtomicString::add):
-
+        
 2008-11-13  Justin Garcia  <justin.garcia@apple.com>
 
         Reviewed by Darin Adler.
index 3700023..7e08f2c 100644 (file)
@@ -611,6 +611,16 @@ void CompositeEditCommand::removePlaceholderAt(const VisiblePosition& visiblePos
     }
 }
 
+PassRefPtr<Node> CompositeEditCommand::insertNewDefaultParagraphElementAt(const Position& position)
+{
+    RefPtr<Node> paragraphElement = createDefaultParagraphElement(document());
+    appendNode(createBreakElement(document()).get(), paragraphElement.get());
+    insertNodeAt(paragraphElement.get(), position);
+    return paragraphElement.release();
+}
+
+// If the paragraph is not entirely within it's own block, create one and move the paragraph into 
+// it, and return that block.  Otherwise return 0.
 PassRefPtr<Node> CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary(const Position& pos)
 {
     if (pos.isNull())
@@ -618,33 +628,43 @@ PassRefPtr<Node> CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessar
     
     updateLayout();
     
+    // It's strange that this function is responsible for verifying that pos has not been invalidated
+    // by an earlier call to this function.  The caller, applyBlockStyle, should do this.
     VisiblePosition visiblePos(pos, VP_DEFAULT_AFFINITY);
     VisiblePosition visibleParagraphStart(startOfParagraph(visiblePos));
     VisiblePosition visibleParagraphEnd = endOfParagraph(visiblePos);
     VisiblePosition next = visibleParagraphEnd.next();
     VisiblePosition visibleEnd = next.isNotNull() ? next : visibleParagraphEnd;
     
-    Position paragraphStart = visibleParagraphStart.deepEquivalent().upstream();
-    Position end = visibleEnd.deepEquivalent().upstream();
+    Position upstreamStart = visibleParagraphStart.deepEquivalent().upstream();
+    Position upstreamEnd = visibleEnd.deepEquivalent().upstream();
 
     // If there are no VisiblePositions in the same block as pos then 
-    // paragraphStart will be outside the paragraph
-    if (Range::compareBoundaryPoints(pos, paragraphStart) < 0)
+    // upstreamStart will be outside the paragraph
+    if (Range::compareBoundaryPoints(pos, upstreamStart) < 0)
         return 0;
 
     // Perform some checks to see if we need to perform work in this function.
-    if (isBlock(paragraphStart.node())) {
-        if (isBlock(end.node())) {
-            if (!end.node()->isDescendantOf(paragraphStart.node())) {
+    if (isBlock(upstreamStart.node())) {
+        // If the block is the body element, always move content to a new block, so that
+        // we avoid adding styles to the body element, since Mail's Make Plain Text feature
+        // can't handle those.
+        if (upstreamStart.node()->hasTagName(bodyTag)) {
+            // If the block is the body element and there is nothing insde of it, create a new
+            // block but don't try and move content into it, since there's nothing to move.
+            if (upstreamStart == upstreamEnd)
+                return insertNewDefaultParagraphElementAt(upstreamStart);
+        } else if (isBlock(upstreamEnd.node())) {
+            if (!upstreamEnd.node()->isDescendantOf(upstreamStart.node())) {
                 // If the paragraph end is a descendant of paragraph start, then we need to run
                 // the rest of this function. If not, we can bail here.
                 return 0;
             }
         }
-        else if (enclosingBlock(end.node()) != paragraphStart.node()) {
+        else if (enclosingBlock(upstreamEnd.node()) != upstreamStart.node()) {
             // The visibleEnd.  It must be an ancestor of the paragraph start.
             // We can bail as we have a full block to work with.
-            ASSERT(paragraphStart.node()->isDescendantOf(enclosingBlock(end.node())));
+            ASSERT(upstreamStart.node()->isDescendantOf(enclosingBlock(upstreamEnd.node())));
             return 0;
         }
         else if (isEndOfDocument(visibleEnd)) {
@@ -653,13 +673,11 @@ PassRefPtr<Node> CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessar
         }
     }
 
-    RefPtr<Node> newBlock = createDefaultParagraphElement(document());
-    appendNode(createBreakElement(document()).get(), newBlock.get());
-    insertNodeAt(newBlock.get(), paragraphStart);
+    RefPtr<Node> newBlock = insertNewDefaultParagraphElementAt(upstreamStart);
     
     moveParagraphs(visibleParagraphStart, visibleParagraphEnd, VisiblePosition(Position(newBlock.get(), 0)));
     
-    return newBlock.get();
+    return newBlock.release();
 }
 
 void CompositeEditCommand::pushAnchorElementDown(Node* anchorNode)
index bbf8b59..07f2323 100644 (file)
@@ -90,6 +90,8 @@ protected:
     PassRefPtr<Node> addBlockPlaceholderIfNeeded(Node*);
     void removePlaceholderAt(const VisiblePosition&);
 
+    PassRefPtr<Node> insertNewDefaultParagraphElementAt(const Position&);
+
     PassRefPtr<Node> moveParagraphContentsToNewBlockIfNecessary(const Position&);
     
     void pushAnchorElementDown(Node*);