[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc
rniwa at webkit.org
rniwa at webkit.org
Wed Dec 22 12:36:13 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit be835d152c2a1d34d6fb311a138ceb3c31abe9cb
Author: rniwa at webkit.org <rniwa at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Wed Aug 25 22:15:47 2010 +0000
2010-08-25 Ryosuke Niwa <rniwa at webkit.org>
Reviewed by Darin Adler.
WebCore::InsertListCommand::modifyRange infinite loop (100% CPU usage)
https://bugs.webkit.org/show_bug.cgi?id=33668
The bug was caused by enclosingListChild returning a list child whose enclosing list is
a sibling of the current list child. Fixed enclosingListChild to traverse upwards
in the DOM to find the list child which is a sibling of the current list child.
Also fixed adjacentEnclosingList to only returns the list that belongs to the same outer list.
In doApplyForSingleParagraph, if the start or the end of currentSelection existed inside a list content
moved by moveParagraphWithClones, either end could point to a wrong position after the move.
Fixed this problem by checking this condition upfront and restoring later.
In doApply, if moveParagraph or moveParagraphWithClones, endOfSelection or startOfLastParagraph
could be null or orphaned, fixed this problem by indexForVisiblePosition.
Test: editing/execCommand/insert-list-orphaned-item-with-nested-lists.html
* editing/InsertListCommand.cpp:
(WebCore::InsertListCommand::doApply):
(WebCore::enclosingListChild):
(WebCore::InsertListCommand::doApplyForSingleParagraph):
(WebCore::adjacentEnclosingList):
(WebCore::InsertListCommand::listifyParagraph):
2010-08-25 Ryosuke Niwa <rniwa at webkit.org>
Reviewed by Darin Adler.
WebCore::InsertListCommand::modifyRange infinite loop (100% CPU usage)
https://bugs.webkit.org/show_bug.cgi?id=33668
Added a test to convert nested lists with an orphaned list child to an ordered nested list.
Selection in switch-list-type-with-inner-list.html is restored correctly after inserting list.
* editing/execCommand/insert-list-orphaned-item-with-nested-lists-expected.txt: Added.
* editing/execCommand/insert-list-orphaned-item-with-nested-lists.html: Added.
* editing/execCommand/switch-list-type-with-inner-list-expected.txt: Selection is restored correctly.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@66048 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 9a11455..81c7daf 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,17 @@
+2010-08-25 Ryosuke Niwa <rniwa at webkit.org>
+
+ Reviewed by Darin Adler.
+
+ WebCore::InsertListCommand::modifyRange infinite loop (100% CPU usage)
+ https://bugs.webkit.org/show_bug.cgi?id=33668
+
+ Added a test to convert nested lists with an orphaned list child to an ordered nested list.
+ Selection in switch-list-type-with-inner-list.html is restored correctly after inserting list.
+
+ * editing/execCommand/insert-list-orphaned-item-with-nested-lists-expected.txt: Added.
+ * editing/execCommand/insert-list-orphaned-item-with-nested-lists.html: Added.
+ * editing/execCommand/switch-list-type-with-inner-list-expected.txt: Selection is restored correctly.
+
2010-08-24 Ryosuke Niwa <rniwa at webkit.org>
Reviewed by Tony Chang.
diff --git a/LayoutTests/editing/execCommand/insert-list-nested-with-orphaned-expected.txt b/LayoutTests/editing/execCommand/insert-list-nested-with-orphaned-expected.txt
new file mode 100644
index 0000000..403fe52
--- /dev/null
+++ b/LayoutTests/editing/execCommand/insert-list-nested-with-orphaned-expected.txt
@@ -0,0 +1,24 @@
+This tests hang when listifying nested lists with an orphaned list child in between (see bug 33668).
+| "
+"
+| <ol>
+| <ol>
+| <li>
+| "hello"
+| "
+ world
+ "
+| <ol>
+| <li>
+| "WebKit"
+| <li>
+| "rocks"
+| "
+ "
+| <ol>
+| <li>
+| "<#selection-caret>because of you"
+| "
+ "
+| "
+"
diff --git a/LayoutTests/editing/execCommand/insert-list-nested-with-orphaned.html b/LayoutTests/editing/execCommand/insert-list-nested-with-orphaned.html
new file mode 100644
index 0000000..d01721f
--- /dev/null
+++ b/LayoutTests/editing/execCommand/insert-list-nested-with-orphaned.html
@@ -0,0 +1,23 @@
+<script src="../../resources/dump-as-markup.js"></script>
+<div id="test" contenteditable>
+<ul>
+ <ul><li>hello</li></ul>
+ world
+ <ol><li>WebKit</li></ol>
+ <ul>
+ <li>rocks</li>
+ <ul><li>because of you</li></ul>
+ </ul>
+</ul>
+</div>
+<script type="text/javascript">
+
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+window.getSelection().selectAllChildren(document.getElementById('test'))
+document.execCommand('InsertOrderedList', false, null);
+
+Markup.description("This tests hang when listifying nested lists with an orphaned list child in between (see bug 33668).")
+Markup.dump('test')
+</script>
diff --git a/LayoutTests/editing/execCommand/switch-list-type-with-inner-list-expected.txt b/LayoutTests/editing/execCommand/switch-list-type-with-inner-list-expected.txt
index 1242fca..9c25045 100644
--- a/LayoutTests/editing/execCommand/switch-list-type-with-inner-list-expected.txt
+++ b/LayoutTests/editing/execCommand/switch-list-type-with-inner-list-expected.txt
@@ -3,7 +3,7 @@ This tests switching an unordered list with a nested list to an ordered list.
"
| <ol>
| <li>
-| "<#selection-caret>hello"
+| "<#selection-anchor>hello"
| "
"
| <ol>
@@ -16,6 +16,6 @@ This tests switching an unordered list with a nested list to an ordered list.
| "
"
| <li>
-| "webkit"
+| "webkit<#selection-focus>"
| "
"
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 06dc40e..252fe4c 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,31 @@
+2010-08-25 Ryosuke Niwa <rniwa at webkit.org>
+
+ Reviewed by Darin Adler.
+
+ WebCore::InsertListCommand::modifyRange infinite loop (100% CPU usage)
+ https://bugs.webkit.org/show_bug.cgi?id=33668
+
+ The bug was caused by enclosingListChild returning a list child whose enclosing list is
+ a sibling of the current list child. Fixed enclosingListChild to traverse upwards
+ in the DOM to find the list child which is a sibling of the current list child.
+ Also fixed adjacentEnclosingList to only returns the list that belongs to the same outer list.
+
+ In doApplyForSingleParagraph, if the start or the end of currentSelection existed inside a list content
+ moved by moveParagraphWithClones, either end could point to a wrong position after the move.
+ Fixed this problem by checking this condition upfront and restoring later.
+
+ In doApply, if moveParagraph or moveParagraphWithClones, endOfSelection or startOfLastParagraph
+ could be null or orphaned, fixed this problem by indexForVisiblePosition.
+
+ Test: editing/execCommand/insert-list-orphaned-item-with-nested-lists.html
+
+ * editing/InsertListCommand.cpp:
+ (WebCore::InsertListCommand::doApply):
+ (WebCore::enclosingListChild):
+ (WebCore::InsertListCommand::doApplyForSingleParagraph):
+ (WebCore::adjacentEnclosingList):
+ (WebCore::InsertListCommand::listifyParagraph):
+
2010-08-25 Brent Fulgham <bfulgham at webkit.org>
Build corrections, no review.
diff --git a/WebCore/editing/InsertListCommand.cpp b/WebCore/editing/InsertListCommand.cpp
index 79deb65..f90d5d3 100644
--- a/WebCore/editing/InsertListCommand.cpp
+++ b/WebCore/editing/InsertListCommand.cpp
@@ -37,6 +37,14 @@ namespace WebCore {
using namespace HTMLNames;
+static Node* enclosingListChild(Node* node, Node* listNode)
+{
+ Node* listChild = enclosingListChild(node);
+ while (listChild && enclosingList(listChild) != listNode)
+ listChild = enclosingListChild(listChild->parentNode());
+ return listChild;
+}
+
PassRefPtr<HTMLElement> InsertListCommand::insertList(Document* document, Type type)
{
RefPtr<InsertListCommand> insertCommand = create(document, type);
@@ -138,7 +146,19 @@ void InsertListCommand::doApply()
if (!startOfLastParagraph.deepEquivalent().node()->inDocument())
return;
setEndingSelection(startOfCurrentParagraph);
+
+ // Save and restore endOfSelection and startOfLastParagraph when necessary
+ // since moveParagraph and movePragraphWithClones can remove nodes.
+ // FIXME: This is an inefficient way to keep selection alive because indexForVisiblePosition walks from
+ // the beginning of the document to the endOfSelection everytime this code is executed.
+ // But not using index is hard because there are so many ways we can lose selection inside doApplyForSingleParagraph.
+ int indexForEndOfSelection = indexForVisiblePosition(endOfSelection);
doApplyForSingleParagraph(forceCreateList, listTag, currentSelection.get());
+ if (endOfSelection.isNull() || endOfSelection.isOrphan() || startOfLastParagraph.isNull() || startOfLastParagraph.isOrphan()) {
+ RefPtr<Range> lastSelectionRange = TextIterator::rangeFromLocationAndLength(document()->documentElement(), indexForEndOfSelection, 0, true);
+ endOfSelection = lastSelectionRange->startPosition();
+ startOfLastParagraph = startOfParagraph(endOfSelection);
+ }
// Fetch the start of the selection after moving the first paragraph,
// because moving the paragraph will invalidate the original start.
@@ -186,12 +206,35 @@ void InsertListCommand::doApplyForSingleParagraph(bool forceCreateList, const Qu
// If the entire list is selected, then convert the whole list.
if (switchListType && isNodeVisiblyContainedWithin(listNode.get(), currentSelection)) {
+ bool rangeStartIsInList = visiblePositionBeforeNode(listNode.get()) == currentSelection->startPosition();
+ bool rangeEndIsInList = visiblePositionAfterNode(listNode.get()) == currentSelection->endPosition();
+
RefPtr<HTMLElement> newList = createHTMLElement(document(), listTag);
insertNodeBefore(newList, listNode);
- Node* outerBlock = listChildNode->isBlockFlow() ? listChildNode : listNode.get();
+
+ Node* firstChildInList = enclosingListChild(VisiblePosition(Position(listNode, 0)).deepEquivalent().node(), listNode.get());
+ Node* outerBlock = firstChildInList->isBlockFlow() ? firstChildInList : listNode.get();
+
moveParagraphWithClones(firstPositionInNode(listNode.get()), lastPositionInNode(listNode.get()), newList.get(), outerBlock);
+
+ // Manually remove listNode because moveParagraphWithClones sometimes leaves it behind in the document.
+ // See the bug 33668 and editing/execCommand/insert-list-orphaned-item-with-nested-lists.html.
+ // FIXME: This might be a bug in moveParagraphWithClones or deleteSelection.
+ if (listNode && listNode->inDocument())
+ removeNode(listNode);
+
newList = mergeWithNeighboringLists(newList);
+
+ // Restore the start and the end of current selection if they started inside listNode
+ // because moveParagraphWithClones could have removed them.
+ ExceptionCode ec;
+ if (rangeStartIsInList && newList)
+ currentSelection->setStart(newList, 0, ec);
+ if (rangeEndIsInList && newList)
+ currentSelection->setEnd(newList, lastOffsetInNode(newList.get()), ec);
+
setEndingSelection(VisiblePosition(firstPositionInNode(newList.get())));
+
return;
}
@@ -202,14 +245,6 @@ void InsertListCommand::doApplyForSingleParagraph(bool forceCreateList, const Qu
m_listElement = listifyParagraph(endingSelection().visibleStart(), listTag);
}
-static Node* enclosingListChild(Node* node, Node* listNode)
-{
- Node* listChild = enclosingListChild(node);
- if (enclosingList(listChild) != listNode)
- return 0;
- return listChild;
-}
-
void InsertListCommand::unlistifyParagraph(const VisiblePosition& originalStart, HTMLElement* listNode, Node* listChildNode)
{
Node* nextListChild;
@@ -278,7 +313,8 @@ static Element* adjacentEnclosingList(const VisiblePosition& pos, const VisibleP
if (!listNode->hasTagName(listTag)
|| listNode->contains(pos.deepEquivalent().node())
- || previousCell != currentCell)
+ || previousCell != currentCell
+ || enclosingList(listNode) != enclosingList(pos.deepEquivalent().node()))
return 0;
return listNode;
@@ -288,6 +324,9 @@ PassRefPtr<HTMLElement> InsertListCommand::listifyParagraph(const VisiblePositio
{
VisiblePosition start = startOfParagraph(originalStart);
VisiblePosition end = endOfParagraph(start);
+
+ if (start.isNull() || end.isNull())
+ return 0;
// Check for adjoining lists.
RefPtr<HTMLElement> listItemElement = createListItemElement(document());
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list