[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373
enrica at apple.com
enrica at apple.com
Wed Apr 7 23:23:01 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit e2bfdba31c2c0ad823008e92ef46394ca15a4d5e
Author: enrica at apple.com <enrica at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Thu Nov 5 01:42:25 2009 +0000
WebCore: Hang in Mail on attempting to change indent level.
<rdar://problem/7131805>
https://bugs.webkit.org/show_bug.cgi?id=31127
Reviewed by Adele Peterson.
The hang was caused by an infinite loop inside outdentRegion.
The code did not account for the fact that, when a list item
cointains multiple paragraphs, outdent moves all paragraphs at
once, invalidating some of the position we keep track of in the loop.
Some code refactoring has also been done to minimize duplicate code.
Test: editing/execCommand/outdent-multiparagraph-list.html
* editing/IndentOutdentCommand.cpp:
(WebCore::IndentOutdentCommand::indentRegion): Moved code in common with
outdentRegion to doApply.
(WebCore::IndentOutdentCommand::outdentRegion): Fixed endless loop.
(WebCore::IndentOutdentCommand::doApply): Some code refactoring.
* editing/IndentOutdentCommand.h: Added VisiblePosition parameters to
indentRegion and outdentRegion.
LayoutTests: Hang in Mail on attempting to change indent level
<rdar://problem/7131805>
https://bugs.webkit.org/show_bug.cgi?id=31127
Reviewed by Adele Peterson.
Added test to cover all the different code paths.
* editing/execCommand/outdent-multiparagraph-list-expected.txt: Added.
* editing/execCommand/outdent-multiparagraph-list.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@50536 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 915264d..62bd0bb 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2009-11-04 Enrica Casucci <enrica at apple.com>
+
+ Reviewed by Adele Peterson.
+
+ Hang in Mail on attempting to change indent level
+ <rdar://problem/7131805>
+ https://bugs.webkit.org/show_bug.cgi?id=31127
+
+ Added test to cover all the different code paths.
+
+ * editing/execCommand/outdent-multiparagraph-list-expected.txt: Added.
+ * editing/execCommand/outdent-multiparagraph-list.html: Added.
+
2009-11-04 Daniel Bates <dbates at webkit.org>
Reviewed by Eric Seidel.
diff --git a/LayoutTests/editing/execCommand/outdent-multiparagraph-list-expected.txt b/LayoutTests/editing/execCommand/outdent-multiparagraph-list-expected.txt
new file mode 100644
index 0000000..844aa59
--- /dev/null
+++ b/LayoutTests/editing/execCommand/outdent-multiparagraph-list-expected.txt
@@ -0,0 +1,25 @@
+This tests outdenting different selections in the lists. The test should not hang.
+Bugzilla bug
+Radar bug
+
+hello world
+ciao
+how are you?
+good
+
+hello world
+ciao
+how are you?
+good
+
+hello world
+ciao
+how are you?
+good
+
+hello world
+ciao
+how are you?
+good
+
+
diff --git a/LayoutTests/editing/execCommand/outdent-multiparagraph-list.html b/LayoutTests/editing/execCommand/outdent-multiparagraph-list.html
new file mode 100644
index 0000000..a88bb8a
--- /dev/null
+++ b/LayoutTests/editing/execCommand/outdent-multiparagraph-list.html
@@ -0,0 +1,55 @@
+<html>
+<body>
+<div>
+This tests outdenting different selections in the lists. The test should not hang.
+<p>
+<a href="https://bugs.webkit.org/show_bug.cgi?id=31127">Bugzilla bug</a>
+<br>
+<a href="<rdar://problem/7131805>">Radar bug</a>
+</p>
+<div id="test1" contenteditable="true">
+ <ul>
+ <li>hello world<br>ciao</li>
+ <li>how are you?<br>good<br></li>
+ <div><br></div>
+ </ul>
+</div>
+<div id="test2" contenteditable="true">
+ <ul>
+ <li>hello world<br>ciao</li>
+ <li>how are you?<br>good<br></li>
+ <div><br></div>
+ </ul>
+</div>
+<div id="test3" contenteditable="true">
+ <ul>
+ <li id="li1">hello world<br>ciao</li>
+ <li>how are you?<br>good<br></li>
+ <div><br></div>
+ </ul>
+</div>
+<div id="test4" contenteditable="true">
+ <ul>
+ <li id="li2">hello world<br>ciao</li>
+ <li id="li3">how are you?<br>good<br></li>
+ <div><br></div>
+ </ul>
+</div>
+</div>
+
+<script type="text/javascript">
+
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+var s = window.getSelection();
+s.setBaseAndExtent(document.getElementById("test1"), 0, document.getElementById("test1"), 0);
+document.execCommand("Outdent", false, "");
+s.setBaseAndExtent(document.getElementById("test2"), 0, document.getElementById("test2"), 2);
+document.execCommand("Outdent", false, "");
+s.setBaseAndExtent(document.getElementById("li1").firstChild, 0, document.getElementById("li1").firstChild, 4);
+document.execCommand("Outdent", false, "");
+s.setBaseAndExtent(document.getElementById("li2").firstChild, 0, document.getElementById("li3").firstChild, 3);
+document.execCommand("Outdent", false, "");
+
+</script>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 1c0bc57..244c045 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,27 @@
+2009-11-04 Enrica Casucci <enrica at apple.com>
+
+ Reviewed by Adele Peterson.
+
+ Hang in Mail on attempting to change indent level.
+ <rdar://problem/7131805>
+ https://bugs.webkit.org/show_bug.cgi?id=31127
+
+ The hang was caused by an infinite loop inside outdentRegion.
+ The code did not account for the fact that, when a list item
+ cointains multiple paragraphs, outdent moves all paragraphs at
+ once, invalidating some of the position we keep track of in the loop.
+ Some code refactoring has also been done to minimize duplicate code.
+
+ Test: editing/execCommand/outdent-multiparagraph-list.html
+
+ * editing/IndentOutdentCommand.cpp:
+ (WebCore::IndentOutdentCommand::indentRegion): Moved code in common with
+ outdentRegion to doApply.
+ (WebCore::IndentOutdentCommand::outdentRegion): Fixed endless loop.
+ (WebCore::IndentOutdentCommand::doApply): Some code refactoring.
+ * editing/IndentOutdentCommand.h: Added VisiblePosition parameters to
+ indentRegion and outdentRegion.
+
2009-11-04 Alpha Lam <hclam at chromium.org>
Reviewed by Eric Carlson.
diff --git a/WebCore/editing/IndentOutdentCommand.cpp b/WebCore/editing/IndentOutdentCommand.cpp
index 808a2f8..1098791 100644
--- a/WebCore/editing/IndentOutdentCommand.cpp
+++ b/WebCore/editing/IndentOutdentCommand.cpp
@@ -129,17 +129,8 @@ void IndentOutdentCommand::indentIntoBlockquote(const VisiblePosition& endOfCurr
targetBlockquote = 0;
}
-void IndentOutdentCommand::indentRegion()
+void IndentOutdentCommand::indentRegion(const VisiblePosition& startOfSelection, const VisiblePosition& endOfSelection)
{
- VisibleSelection selection = selectionForParagraphIteration(endingSelection());
- VisiblePosition startOfSelection = selection.visibleStart();
- VisiblePosition endOfSelection = selection.visibleEnd();
- int startIndex = indexForVisiblePosition(startOfSelection);
- int endIndex = indexForVisiblePosition(endOfSelection);
-
- ASSERT(!startOfSelection.isNull());
- ASSERT(!endOfSelection.isNull());
-
// Special case empty unsplittable elements because there's nothing to split
// and there's nothing to move.
Position start = startOfSelection.deepEquivalent().downstream();
@@ -169,14 +160,7 @@ void IndentOutdentCommand::indentRegion()
return;
}
endOfCurrentParagraph = endOfNextParagraph;
- }
-
- updateLayout();
-
- RefPtr<Range> startRange = TextIterator::rangeFromLocationAndLength(document()->documentElement(), startIndex, 0, true);
- RefPtr<Range> endRange = TextIterator::rangeFromLocationAndLength(document()->documentElement(), endIndex, 0, true);
- if (startRange && endRange)
- setEndingSelection(VisibleSelection(startRange->startPosition(), endRange->startPosition(), DOWNSTREAM));
+ }
}
void IndentOutdentCommand::outdentParagraph()
@@ -242,36 +226,40 @@ void IndentOutdentCommand::outdentParagraph()
moveParagraph(startOfParagraph(visibleStartOfParagraph), endOfParagraph(visibleEndOfParagraph), VisiblePosition(Position(placeholder.get(), 0)), true);
}
-void IndentOutdentCommand::outdentRegion()
+void IndentOutdentCommand::outdentRegion(const VisiblePosition& startOfSelection, const VisiblePosition& endOfSelection)
{
- VisiblePosition startOfSelection = endingSelection().visibleStart();
- VisiblePosition endOfSelection = endingSelection().visibleEnd();
VisiblePosition endOfLastParagraph = endOfParagraph(endOfSelection);
- ASSERT(!startOfSelection.isNull());
- ASSERT(!endOfSelection.isNull());
-
if (endOfParagraph(startOfSelection) == endOfLastParagraph) {
outdentParagraph();
return;
}
-
+
Position originalSelectionEnd = endingSelection().end();
- setEndingSelection(endingSelection().visibleStart());
- outdentParagraph();
- Position originalSelectionStart = endingSelection().start();
- VisiblePosition endOfCurrentParagraph = endOfParagraph(endOfParagraph(endingSelection().visibleStart()).next(true));
+ VisiblePosition endOfCurrentParagraph = endOfParagraph(startOfSelection);
VisiblePosition endAfterSelection = endOfParagraph(endOfParagraph(endOfSelection).next());
+
while (endOfCurrentParagraph != endAfterSelection) {
VisiblePosition endOfNextParagraph = endOfParagraph(endOfCurrentParagraph.next());
if (endOfCurrentParagraph == endOfLastParagraph)
setEndingSelection(VisibleSelection(originalSelectionEnd, DOWNSTREAM));
else
setEndingSelection(endOfCurrentParagraph);
+
outdentParagraph();
+
+ // outdentParagraph could move more than one paragraph if the paragraph
+ // is in a list item. As a result, endAfterSelection and endOfNextParagraph
+ // could refer to positions no longer in the document.
+ if (endAfterSelection.isNotNull() && !endAfterSelection.deepEquivalent().node()->inDocument())
+ break;
+
+ if (endOfNextParagraph.isNotNull() && !endOfNextParagraph.deepEquivalent().node()->inDocument()) {
+ endOfCurrentParagraph = endingSelection().end();
+ endOfNextParagraph = endOfParagraph(endOfCurrentParagraph.next());
+ }
endOfCurrentParagraph = endOfNextParagraph;
}
- setEndingSelection(VisibleSelection(originalSelectionStart, endingSelection().end(), DOWNSTREAM));
}
void IndentOutdentCommand::doApply()
@@ -295,10 +283,27 @@ void IndentOutdentCommand::doApply()
if (visibleEnd != visibleStart && isStartOfParagraph(visibleEnd))
setEndingSelection(VisibleSelection(visibleStart, visibleEnd.previous(true)));
+ VisibleSelection selection = selectionForParagraphIteration(endingSelection());
+ VisiblePosition startOfSelection = selection.visibleStart();
+ VisiblePosition endOfSelection = selection.visibleEnd();
+
+ int startIndex = indexForVisiblePosition(startOfSelection);
+ int endIndex = indexForVisiblePosition(endOfSelection);
+
+ ASSERT(!startOfSelection.isNull());
+ ASSERT(!endOfSelection.isNull());
+
if (m_typeOfAction == Indent)
- indentRegion();
+ indentRegion(startOfSelection, endOfSelection);
else
- outdentRegion();
+ outdentRegion(startOfSelection, endOfSelection);
+
+ updateLayout();
+
+ RefPtr<Range> startRange = TextIterator::rangeFromLocationAndLength(document()->documentElement(), startIndex, 0, true);
+ RefPtr<Range> endRange = TextIterator::rangeFromLocationAndLength(document()->documentElement(), endIndex, 0, true);
+ if (startRange && endRange)
+ setEndingSelection(VisibleSelection(startRange->startPosition(), endRange->startPosition(), DOWNSTREAM));
}
}
diff --git a/WebCore/editing/IndentOutdentCommand.h b/WebCore/editing/IndentOutdentCommand.h
index 817b4c8..8705bf1 100644
--- a/WebCore/editing/IndentOutdentCommand.h
+++ b/WebCore/editing/IndentOutdentCommand.h
@@ -46,8 +46,8 @@ private:
virtual void doApply();
virtual EditAction editingAction() const { return m_typeOfAction == Indent ? EditActionIndent : EditActionOutdent; }
- void indentRegion();
- void outdentRegion();
+ void indentRegion(const VisiblePosition&, const VisiblePosition&);
+ void outdentRegion(const VisiblePosition&, const VisiblePosition&);
void outdentParagraph();
bool tryIndentingAsListItem(const VisiblePosition&);
void indentIntoBlockquote(const VisiblePosition&, const VisiblePosition&, RefPtr<Element>&);
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list