Changeset 63039 in webkit

Timestamp:

Jul 9, 2010 9:20:50 PM (14 years ago)

Author:

tony@chromium.org

Message:

2010-07-09 Tony Chang <​tony@chromium.org>

Reviewed by Ojan Vafai.

crash in WebCore::CompositeEditCommand::splitTreeToNode when indenting pre
​https://bugs.webkit.org/show_bug.cgi?id=38231

• editing/execCommand/indent-pre-expected.txt: Added.
• editing/execCommand/indent-pre.html: Added.

2010-07-09 Tony Chang <​tony@chromium.org>

Reviewed by Ojan Vafai.

crash in WebCore::CompositeEditCommand::splitTreeToNode when indenting pre
​https://bugs.webkit.org/show_bug.cgi?id=38231

Test: editing/execCommand/indent-pre.html

• editing/IndentOutdentCommand.cpp: (WebCore::countParagraphs): (WebCore::IndentOutdentCommand::indentRegion): Split text nodes into one node per paragraph

so moveParagraph doesn't get confused.

(WebCore::IndentOutdentCommand::splitTextNodes):

• editing/IndentOutdentCommand.h:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/editing/execCommand/indent-pre-expected.txt (added)
• LayoutTests/editing/execCommand/indent-pre.html (added)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/editing/IndentOutdentCommand.cpp (modified) (4 diffs)
• WebCore/editing/IndentOutdentCommand.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2010-07-09  Tony Chang  <tony@chromium.org>
+
+        Reviewed by Ojan Vafai.
+
+        crash in WebCore::CompositeEditCommand::splitTreeToNode when indenting pre
+        https://bugs.webkit.org/show_bug.cgi?id=38231
+
+        * editing/execCommand/indent-pre-expected.txt: Added.
+        * editing/execCommand/indent-pre.html: Added.
+
 2010-07-09  Erik Arvidsson  <arv@chromium.org>
 

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-07-09  Tony Chang  <tony@chromium.org>
+
+        Reviewed by Ojan Vafai.
+
+        crash in WebCore::CompositeEditCommand::splitTreeToNode when indenting pre
+        https://bugs.webkit.org/show_bug.cgi?id=38231
+
+        Test: editing/execCommand/indent-pre.html
+
+        * editing/IndentOutdentCommand.cpp:
+        (WebCore::countParagraphs):
+        (WebCore::IndentOutdentCommand::indentRegion): Split text nodes into one node per paragraph
+                                                       so moveParagraph doesn't get confused.
+        (WebCore::IndentOutdentCommand::splitTextNodes):
+        * editing/IndentOutdentCommand.h:
+
 2010-07-09  Erik Arvidsson  <arv@chromium.org>
 

trunk/WebCore/editing/IndentOutdentCommand.cpp

@@ -35 +35 @@
 #include "Range.h"
 #include "SplitElementCommand.h"
+#include "Text.h"
 #include "TextIterator.h"
 #include "htmlediting.h"
@@ -61 +62 @@
 {
     return node && (node->hasTagName(ulTag) || node->hasTagName(olTag) || node->hasTagName(blockquoteTag));
+}
+
+// This function can return -1 if we are unable to count the paragraphs between |start| and |end|.
+static int countParagraphs(const VisiblePosition& endOfFirstParagraph, const VisiblePosition& endOfLastParagraph)
+{
+    int count = 0;
+    VisiblePosition cur = endOfFirstParagraph;
+    while (cur != endOfLastParagraph) {
+        ++count;
+        cur = endOfParagraph(cur.next());
+        // If start is before a table and end is inside a table, we will never hit end because the
+        // whole table is considered a single paragraph.
+        if (cur.isNull())
+            return -1;
+    }
+    return count;
 }
 
@@ -148 +165 @@
     VisiblePosition endOfCurrentParagraph = endOfParagraph(startOfSelection);
     VisiblePosition endAfterSelection = endOfParagraph(endOfParagraph(endOfSelection).next());
+    int endOfCurrentParagraphIndex = indexForVisiblePosition(endOfCurrentParagraph);
+    int endAfterSelectionIndex = indexForVisiblePosition(endAfterSelection);
+
+    // When indenting within a <pre> tag, we need to split each paragraph into a separate node for moveParagraphWithClones to work.
+    // However, splitting text nodes can cause endOfCurrentParagraph and endAfterSelection to point to an invalid position if we
+    // changed the text node it was pointing at.  So we have to reset these positions.
+    int numParagraphs = countParagraphs(endOfCurrentParagraph, endAfterSelection);
+    if (splitTextNodes(startOfParagraph(startOfSelection), numParagraphs + 1)) {
+        RefPtr<Range> endOfCurrentParagraphRange = TextIterator::rangeFromLocationAndLength(document()->documentElement(), endOfCurrentParagraphIndex, 0, true);
+        RefPtr<Range> endAfterSelectionRange = TextIterator::rangeFromLocationAndLength(document()->documentElement(), endAfterSelectionIndex, 0, true);
+        if (!endOfCurrentParagraphRange.get() || !endAfterSelectionRange.get()) {
+            ASSERT_NOT_REACHED();
+            return;
+        }
+        endOfCurrentParagraph = VisiblePosition(endOfCurrentParagraphRange->startPosition(), DOWNSTREAM);
+        endAfterSelection = VisiblePosition(endAfterSelectionRange->startPosition(), DOWNSTREAM);
+    }
+
     while (endOfCurrentParagraph != endAfterSelection) {
         // Iterate across the selected paragraphs...
@@ -169 +204 @@
         endOfCurrentParagraph = endOfNextParagraph;
     }   
+}
+
+// Returns true if at least one text node was split.
+bool IndentOutdentCommand::splitTextNodes(const VisiblePosition& start, int numParagraphs)
+{
+    VisiblePosition currentParagraphStart = start;
+    bool hasSplit = false;
+    int paragraphCount;
+    for (paragraphCount = 0; paragraphCount < numParagraphs; ++paragraphCount) {
+        // If there are multiple paragraphs in a single text node, we split the text node into a separate node for each paragraph.
+        if (currentParagraphStart.deepEquivalent().node()->isTextNode() && currentParagraphStart.deepEquivalent().node() == startOfParagraph(currentParagraphStart.previous()).deepEquivalent().node()) {
+            Text* textNode = static_cast<Text *>(currentParagraphStart.deepEquivalent().node());
+            int offset = currentParagraphStart.deepEquivalent().offsetInContainerNode();
+            splitTextNode(textNode, offset);
+            currentParagraphStart = VisiblePosition(textNode, 0, VP_DEFAULT_AFFINITY);
+            hasSplit = true;
+        }
+        VisiblePosition nextParagraph = startOfParagraph(endOfParagraph(currentParagraphStart).next());
+        if (nextParagraph.isNull())
+            break;
+        currentParagraphStart = nextParagraph;
+    }
+    return hasSplit;
 }
 

trunk/WebCore/editing/IndentOutdentCommand.h

@@ -52 +52 @@
     bool tryIndentingAsListItem(const VisiblePosition&);
     void indentIntoBlockquote(const VisiblePosition&, const VisiblePosition&, RefPtr<Element>&);
+    bool splitTextNodes(const VisiblePosition& start, int numParagraphs);
 
     EIndentType m_typeOfAction;