[Webkit-unassigned] [Bug 38231] crash in WebCore::CompositeEditCommand::splitTreeToNode when indenting pre
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Jun 22 10:02:23 PDT 2010
https://bugs.webkit.org/show_bug.cgi?id=38231
--- Comment #13 from Enrica Casucci <enrica at apple.com> 2010-06-22 10:02:23 PST ---
(From update of attachment 55814)
> diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
> index 4dc9e8e05791fadb3e720b13f0e8e89db4975aa3..00771643fc6e56ee7c894d53a5a71e543806c352 100644
> --- a/LayoutTests/ChangeLog
> +++ b/LayoutTests/ChangeLog
> @@ -1,3 +1,13 @@
> +2010-05-12 Tony Chang <tony at chromium.org>
> +
> + Reviewed by NOBODY (OOPS!).
> +
> + 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-05-11 Tony Chang <tony at chromium.org>
>
> Reviewed by Darin Fisher.
> diff --git a/LayoutTests/editing/execCommand/indent-pre-expected.txt b/LayoutTests/editing/execCommand/indent-pre-expected.txt
> new file mode 100644
> index 0000000000000000000000000000000000000000..883a704e8bd4e99bba9300fe30813c566363a586
> --- /dev/null
> +++ b/LayoutTests/editing/execCommand/indent-pre-expected.txt
> @@ -0,0 +1,179 @@
> +
> +<HTML>
> +<HEAD>
> +</HEAD>
> +<BODY>
> +<DIV contentEditable="">
> +<#text>
> +</#text>
> +<BLOCKQUOTE style="margin: 0 0 0 40px; border: none; padding: 0px;" class="webkit-indent-blockquote">
> +<PRE id="pre-basic">
> +<#text>line one
> +</#text>
> +</PRE>
> +</BLOCKQUOTE>
> +<PRE id="pre-basic">
> +<#text>line two
> +</#text>
> +</PRE>
> +<BLOCKQUOTE style="margin: 0 0 0 40px; border: none; padding: 0px;" class="webkit-indent-blockquote">
> +<PRE id="pre-basic">
> +<#text>line three
> +</#text>
> +</PRE>
> +<PRE id="pre-basic">
> +<#text>line four</#text>
> +</PRE>
> +</BLOCKQUOTE>
> +<#text>
> +
> +</#text>
> +<UL>
> +<LI>
> +<PRE id="pre-list">
> +<#text>list one
> +</#text>
> +<BLOCKQUOTE style="margin: 0 0 0 40px; border: none; padding: 0px;" class="webkit-indent-blockquote">
> +<#text>list two
> +</#text>
> +<#text>list three
> +</#text>
> +</BLOCKQUOTE>
> +<#text>list four
> +</#text>
> +</PRE>
> +</LI>
> +</UL>
> +<#text>
> +
> +</#text>
> +<BLOCKQUOTE style="margin: 0 0 0 40px; border: none; padding: 0px;" class="webkit-indent-blockquote">
> +<TABLE>
> +<#text>
> +</#text>
> +<TBODY>
> +<TR>
> +<TD>
> +<PRE id="pre-table">
> +<#text>table one
> +</#text>
> +<#text><selection-anchor>table two
> +</#text>
> +</PRE>
> +<BLOCKQUOTE style="margin: 0 0 0 40px; border: none; padding: 0px;" class="webkit-indent-blockquote">
> +<PRE id="pre-table">
> +<#text>table three<selection-focus></#text>
> +</PRE>
> +<PRE id="pre-table">
> +<#text>table three</#text>
> +</PRE>
> +</BLOCKQUOTE>
> +</TD>
> +<TD>
> +<BLOCKQUOTE style="margin: 0 0 0 40px; border: none; padding: 0px;" class="webkit-indent-blockquote">
> +<#text>right cell</#text>
> +</BLOCKQUOTE>
> +</TD>
> +</TR>
> +</TBODY>
> +</TABLE>
> +<DIV id="results">
> +<#text>PASSED</#text>
> +</DIV>
> +</BLOCKQUOTE>
> +<#text>
> +
> +</#text>
> +<#text>
> +</#text>
> +</DIV>
> +<#text>
> +
> +</#text>
> +<A href="javascript:document.execCommand('indent')">
> +<#text>indent</#text>
> +</A>
> +<#text>
> +</#text>
> +<A href="javascript:document.execCommand('outdent')">
> +<#text>outdent</#text>
> +</A>
> +<#text>
> +</#text>
> +<SCRIPT src="../../resources/dump-as-markup.js"></SCRIPT>
> +<#text>
> +</#text>
> +<SCRIPT src="../editing.js"></SCRIPT>
> +<#text>
> +</#text>
> +<SCRIPT>
> +function setSelection(node, startOffset, endOffset)
> +{
> +var textNode = node.firstChild;
> +if (textNode.nodeType != Node.TEXT_NODE)
> +throw "Wrong node type: " + textNode;
> +execSetSelectionCommand(textNode, startOffset, endOffset);
> +}
> +
> +function verifyTextSelection(startNode, startOffset, endNode, endOffset)
> +{
> +if (startNode.nodeType != Node.TEXT_NODE)
> +console.log("Wrong start node type: " + startNode);
> +if (endNode.nodeType != Node.TEXT_NODE)
> +console.log("Wrong end node type: " + endNode);
> +var sel = window.getSelection();
> +if (sel.anchorNode != startNode || sel.focusNode != endNode)
> +console.log("Wrong node selected.");
> +if (sel.anchorOffset != startOffset)
> +console.log("Wrong anchor offset: " + sel.anchorOffset + " instead of " + startOffset);
> +if (sel.focusOffset != endOffset)
> +console.log("Wrong focus offset: " + sel.focusOffset + " instead of " + endOffset);
> +}
> +
> +// Indent a single line in a pre and make sure the selection is correctly preserved.
> +var pre = document.getElementById("pre-basic");
> +setSelection(pre, 0, 0);
> +execMoveSelectionForwardByCharacterCommand();
> +execExtendSelectionForwardByWordCommand();
> +document.execCommand("indent");
> +verifyTextSelection(document.getElementsByTagName("pre")[0].firstChild, 1,
> +document.getElementsByTagName("pre")[0].firstChild, 4);
> +
> +// Indent 2 lines.
> +setSelection(pre, 0, 0);
> +execMoveSelectionForwardByLineCommand();
> +execExtendSelectionForwardByLineCommand();
> +execExtendSelectionForwardByWordCommand();
> +document.execCommand("indent");
> +if (document.getElementsByTagName("pre").length > 3) {
> +// FIXME: The selection for the anchorNode is wrong. It should be (pre[2], 0).
> +verifyTextSelection(document.getElementsByTagName("pre")[1].firstChild, 8,
> +document.getElementsByTagName("pre")[3].firstChild, 4);
> +} else {
> +console.log("Wrong number of pre nodes.");
> +}
> +// Indent <pre> lines in a list.
> +pre = document.getElementById("pre-list");
> +setSelection(pre, 0, 0);
> +execMoveSelectionForwardByLineCommand();
> +execExtendSelectionForwardByLineCommand();
> +execExtendSelectionForwardByLineCommand();
> +document.execCommand("indent");
> +verifyTextSelection(document.getElementsByTagName("blockquote")[2].firstChild, 0,
> +document.getElementsByTagName("blockquote")[2].firstChild.nextSibling, 10);
> +
> +// Indenting <pre> lines in a table.
> +pre = document.getElementById("pre-table");
> +setSelection(pre, 0, 0);
> +execMoveSelectionForwardByLineCommand();
> +execExtendSelectionForwardByLineCommand();
> +execExtendSelectionForwardByLineCommand();
> +// FIXME: This is wrong.
> +document.execCommand("indent");
> +
> +document.getElementById("results").innerText = "PASSED";
> +</SCRIPT>
> +<#text>
> +</#text>
> +</BODY>
> +</HTML>
> diff --git a/LayoutTests/editing/execCommand/indent-pre.html b/LayoutTests/editing/execCommand/indent-pre.html
> new file mode 100644
> index 0000000000000000000000000000000000000000..0641c844de9fdd8538d60461b5c1525798ae1f90
> --- /dev/null
> +++ b/LayoutTests/editing/execCommand/indent-pre.html
> @@ -0,0 +1,91 @@
> +<div contentEditable>
> +<pre id="pre-basic">line one
> +line two
> +line three
> +line four</pre>
> +
> +<ul><li><pre id="pre-list">list one
> +list two
> +list three
> +list four
> +</pre></li></ul>
> +
> +<table>
> +<tr><td><pre id="pre-table">table one
> +table two
> +table three</pre></td><td>right cell</td></tr></table>
> +
> +<div id="results">FAILED</div>
> +</div>
> +
> +<a href="javascript:document.execCommand('indent')">indent</a>
> +<a href="javascript:document.execCommand('outdent')">outdent</a>
> +<script src="../../resources/dump-as-markup.js"></script>
> +<script src="../editing.js"></script>
> +<script>
> +function setSelection(node, startOffset, endOffset)
> +{
> + var textNode = node.firstChild;
> + if (textNode.nodeType != Node.TEXT_NODE)
> + throw "Wrong node type: " + textNode;
> + execSetSelectionCommand(textNode, startOffset, endOffset);
> +}
> +
> +function verifyTextSelection(startNode, startOffset, endNode, endOffset)
> +{
> + if (startNode.nodeType != Node.TEXT_NODE)
> + console.log("Wrong start node type: " + startNode);
> + if (endNode.nodeType != Node.TEXT_NODE)
> + console.log("Wrong end node type: " + endNode);
> + var sel = window.getSelection();
> + if (sel.anchorNode != startNode || sel.focusNode != endNode)
> + console.log("Wrong node selected.");
> + if (sel.anchorOffset != startOffset)
> + console.log("Wrong anchor offset: " + sel.anchorOffset + " instead of " + startOffset);
> + if (sel.focusOffset != endOffset)
> + console.log("Wrong focus offset: " + sel.focusOffset + " instead of " + endOffset);
> +}
> +
> +// Indent a single line in a pre and make sure the selection is correctly preserved.
> +var pre = document.getElementById("pre-basic");
> +setSelection(pre, 0, 0);
> +execMoveSelectionForwardByCharacterCommand();
> +execExtendSelectionForwardByWordCommand();
> +document.execCommand("indent");
> +verifyTextSelection(document.getElementsByTagName("pre")[0].firstChild, 1,
> + document.getElementsByTagName("pre")[0].firstChild, 4);
> +
> +// Indent 2 lines.
> +setSelection(pre, 0, 0);
> +execMoveSelectionForwardByLineCommand();
> +execExtendSelectionForwardByLineCommand();
> +execExtendSelectionForwardByWordCommand();
> +document.execCommand("indent");
> +if (document.getElementsByTagName("pre").length > 3) {
> + // FIXME: The selection for the anchorNode is wrong. It should be (pre[2], 0).
> + verifyTextSelection(document.getElementsByTagName("pre")[1].firstChild, 8,
> + document.getElementsByTagName("pre")[3].firstChild, 4);
> +} else {
> + console.log("Wrong number of pre nodes.");
> +}
> +// Indent <pre> lines in a list.
> +pre = document.getElementById("pre-list");
> +setSelection(pre, 0, 0);
> +execMoveSelectionForwardByLineCommand();
> +execExtendSelectionForwardByLineCommand();
> +execExtendSelectionForwardByLineCommand();
> +document.execCommand("indent");
> +verifyTextSelection(document.getElementsByTagName("blockquote")[2].firstChild, 0,
> + document.getElementsByTagName("blockquote")[2].firstChild.nextSibling, 10);
> +
> +// Indenting <pre> lines in a table.
> +pre = document.getElementById("pre-table");
> +setSelection(pre, 0, 0);
> +execMoveSelectionForwardByLineCommand();
> +execExtendSelectionForwardByLineCommand();
> +execExtendSelectionForwardByLineCommand();
> +// FIXME: This is wrong.
> +document.execCommand("indent");
> +
> +document.getElementById("results").innerText = "PASSED";
> +</script>
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index 482ee438dd8109f3a9b4b4105770931a21c640fc..266ea14b48c21c1f7d97320797651fc5f3f033bf 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,19 @@
> +2010-05-12 Tony Chang <tony at chromium.org>
> +
> + Reviewed by NOBODY (OOPS!).
> +
> + 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-05-11 David Hyatt <hyatt at apple.com>
>
> Reviewed by Maciej Stachowiak.
> diff --git a/WebCore/editing/IndentOutdentCommand.cpp b/WebCore/editing/IndentOutdentCommand.cpp
> index 0f3975b0c13f6e8e93e32e8c659ab3b77b463f39..b68527d044ee3af7d12d7d6f2b605fba4f912290 100644
> --- a/WebCore/editing/IndentOutdentCommand.cpp
> +++ b/WebCore/editing/IndentOutdentCommand.cpp
> @@ -34,6 +34,7 @@
> #include "InsertListCommand.h"
> #include "Range.h"
> #include "SplitElementCommand.h"
> +#include "Text.h"
> #include "TextIterator.h"
> #include "htmlediting.h"
> #include "visible_units.h"
> @@ -62,6 +63,22 @@ static bool isListOrIndentBlockquote(const Node* node)
> 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& start, const VisiblePosition& end)
> +{
> + int count = 0;
> + VisiblePosition cur = start;
> + while (cur != end) {
> + ++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;
> +}
> +
> IndentOutdentCommand::IndentOutdentCommand(Document* document, EIndentType typeOfAction, int marginInPixels)
> : CompositeEditCommand(document), m_typeOfAction(typeOfAction), m_marginInPixels(marginInPixels)
> {
> @@ -151,6 +168,18 @@ void IndentOutdentCommand::indentRegion(const VisiblePosition& startOfSelection,
> RefPtr<Element> blockquoteForNextIndent;
> 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)) {
> + endOfCurrentParagraph = VisiblePosition(TextIterator::rangeFromLocationAndLength(document()->documentElement(), endOfCurrentParagraphIndex, 0)->startPosition(), UPSTREAM);
> + endAfterSelection = VisiblePosition(TextIterator::rangeFromLocationAndLength(document()->documentElement(), endAfterSelectionIndex, 0)->startPosition(), UPSTREAM);
> + }
> +
> while (endOfCurrentParagraph != endAfterSelection) {
> // Iterate across the selected paragraphs...
> VisiblePosition endOfNextParagraph = endOfParagraph(endOfCurrentParagraph.next());
> @@ -174,6 +203,28 @@ void IndentOutdentCommand::indentRegion(const VisiblePosition& startOfSelection,
> }
> }
>
> +// 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 (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;
> +}
> +
> void IndentOutdentCommand::outdentParagraph()
> {
> VisiblePosition visibleStartOfParagraph = startOfParagraph(endingSelection().visibleStart());
> diff --git a/WebCore/editing/IndentOutdentCommand.h b/WebCore/editing/IndentOutdentCommand.h
> index 8705bf103c22122298d1e8495dc007e9c2580911..8644cc56aa82083052e52e7a654a460fef3db964 100644
> --- a/WebCore/editing/IndentOutdentCommand.h
> +++ b/WebCore/editing/IndentOutdentCommand.h
> @@ -51,6 +51,7 @@ private:
> void outdentParagraph();
> bool tryIndentingAsListItem(const VisiblePosition&);
> void indentIntoBlockquote(const VisiblePosition&, const VisiblePosition&, RefPtr<Element>&);
> + bool splitTextNodes(const VisiblePosition& start, int numParagraphs);
>
> EIndentType m_typeOfAction;
> int m_marginInPixels;
The patch looks good to me.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list