Title: [115520] trunk
- Revision
- 115520
- Author
- [email protected]
- Date
- 2012-04-27 17:10:23 -0700 (Fri, 27 Apr 2012)
Log Message
REGRESSION(113723): Pressing enter in this list example deletes the whole list
https://bugs.webkit.org/show_bug.cgi?id=85016
Reviewed by Enrica Casucci.
The bug was caused by CompositeEditCommand::breakOutOfEmptyListItem, which calls isListItem
on the empty list's siblings to decide which part of the list should get removed. However,
the check fails when the empty list's sibling is a text node, or a list element (e.g. ul, ol).
Fixed it by skipping empty list's non-element sibling and calling isListElement to do further
check.
Source/WebCore:
Test: added new test cases in the existing test (break-out-of-empty-list-item.html)
* editing/CompositeEditCommand.cpp:
(WebCore::CompositeEditCommand::breakOutOfEmptyListItem):
LayoutTests:
* editing/execCommand/break-out-of-empty-list-item-expected.txt:
* editing/execCommand/script-tests/break-out-of-empty-list-item.js:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (115519 => 115520)
--- trunk/LayoutTests/ChangeLog 2012-04-28 00:07:49 UTC (rev 115519)
+++ trunk/LayoutTests/ChangeLog 2012-04-28 00:10:23 UTC (rev 115520)
@@ -1,3 +1,19 @@
+2012-04-27 Yi Shen <[email protected]>
+
+ REGRESSION(113723): Pressing enter in this list example deletes the whole list
+ https://bugs.webkit.org/show_bug.cgi?id=85016
+
+ Reviewed by Enrica Casucci.
+
+ The bug was caused by CompositeEditCommand::breakOutOfEmptyListItem, which calls isListItem
+ on the empty list's siblings to decide which part of the list should get removed. However,
+ the check fails when the empty list's sibling is a text node, or a list element (e.g. ul, ol).
+ Fixed it by skipping empty list's non-element sibling and calling isListElement to do further
+ check.
+
+ * editing/execCommand/break-out-of-empty-list-item-expected.txt:
+ * editing/execCommand/script-tests/break-out-of-empty-list-item.js:
+
2012-04-27 Tim Horton <[email protected]>
SMIL animation causes leak of the related Document (and many elements)
Modified: trunk/LayoutTests/editing/execCommand/break-out-of-empty-list-item-expected.txt (115519 => 115520)
--- trunk/LayoutTests/editing/execCommand/break-out-of-empty-list-item-expected.txt 2012-04-28 00:07:49 UTC (rev 115519)
+++ trunk/LayoutTests/editing/execCommand/break-out-of-empty-list-item-expected.txt 2012-04-28 00:10:23 UTC (rev 115520)
@@ -12,6 +12,11 @@
PASS enterAtTarget('<ul><li><ul><li id="target"><br></li></ul></li></ul>') is '<ul><li></li><li><br></li></ul>'
PASS enterAtTarget('<ul><li>hello</li><br id="target"></ul>') is '<ul><li>hello</li></ul><div><br></div>'
PASS enterAtTarget('<ul><br id="target"></ul>') is '<div><br></div>'
+PASS enterAtTarget('<ul><li>hello</li>abc<li id="target"></li></ul>') is '<ul><li>hello</li>abc</ul><div><br></div>'
+PASS enterAtTarget('<ul><li>1</li><ul><li>2.1</li></ul><li id="target"></li></ul>') is '<ul><li>1</li><ul><li>2.1</li></ul></ul><div><br></div>'
+PASS enterAtTarget('<ul><li>1</li><ul><li>2.1</li><li>2.2</li><li id="target"></li></ul><li>3</li></ul>') is '<ul><li>1</li><ul><li>2.1</li><li>2.2</li></ul><li><br></li><li>3</li></ul>'
+PASS enterAtTarget('<ul><li>1</li><ul><li>2.1</li><li>2.2</li>abc<li id="target"></li></ul><li>3</li></ul>') is '<ul><li>1</li><ul><li>2.1</li><li>2.2</li>abc</ul><li><br></li><li>3</li></ul>'
+PASS enterAtTarget('<ul><li>1</li><li id="target"></li><li>3</li></ul>') is '<ul><li>1</li></ul><div><br></div><ul><li>3</li></ul>'
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/editing/execCommand/script-tests/break-out-of-empty-list-item.js (115519 => 115520)
--- trunk/LayoutTests/editing/execCommand/script-tests/break-out-of-empty-list-item.js 2012-04-28 00:07:49 UTC (rev 115519)
+++ trunk/LayoutTests/editing/execCommand/script-tests/break-out-of-empty-list-item.js 2012-04-28 00:10:23 UTC (rev 115520)
@@ -51,6 +51,11 @@
testBreakOutOfEmptyListItem('<ul><li><ul><li id="target"><br></li></ul></li></ul>', '<ul><li></li><li><br></li></ul>');
testBreakOutOfEmptyListItem('<ul><li>hello</li><br id="target"></ul>', '<ul><li>hello</li></ul><div><br></div>');
testBreakOutOfEmptyListItem('<ul><br id="target"></ul>', '<div><br></div>');
+testBreakOutOfEmptyListItem('<ul><li>hello</li>abc<li id="target"></li></ul>', '<ul><li>hello</li>abc</ul><div><br></div>');
+testBreakOutOfEmptyListItem('<ul><li>1</li><ul><li>2.1</li></ul><li id="target"></li></ul>', '<ul><li>1</li><ul><li>2.1</li></ul></ul><div><br></div>');
+testBreakOutOfEmptyListItem('<ul><li>1</li><ul><li>2.1</li><li>2.2</li><li id="target"></li></ul><li>3</li></ul>', '<ul><li>1</li><ul><li>2.1</li><li>2.2</li></ul><li><br></li><li>3</li></ul>');
+testBreakOutOfEmptyListItem('<ul><li>1</li><ul><li>2.1</li><li>2.2</li>abc<li id="target"></li></ul><li>3</li></ul>', '<ul><li>1</li><ul><li>2.1</li><li>2.2</li>abc</ul><li><br></li><li>3</li></ul>');
+testBreakOutOfEmptyListItem('<ul><li>1</li><li id="target"></li><li>3</li></ul>', '<ul><li>1</li></ul><div><br></div><ul><li>3</li></ul>');
document.body.removeChild(testContainer);
Modified: trunk/Source/WebCore/ChangeLog (115519 => 115520)
--- trunk/Source/WebCore/ChangeLog 2012-04-28 00:07:49 UTC (rev 115519)
+++ trunk/Source/WebCore/ChangeLog 2012-04-28 00:10:23 UTC (rev 115520)
@@ -1,3 +1,21 @@
+2012-04-27 Yi Shen <[email protected]>
+
+ REGRESSION(113723): Pressing enter in this list example deletes the whole list
+ https://bugs.webkit.org/show_bug.cgi?id=85016
+
+ Reviewed by Enrica Casucci.
+
+ The bug was caused by CompositeEditCommand::breakOutOfEmptyListItem, which calls isListItem
+ on the empty list's siblings to decide which part of the list should get removed. However,
+ the check fails when the empty list's sibling is a text node, or a list element (e.g. ul, ol).
+ Fixed it by skipping empty list's non-element sibling and calling isListElement to do further
+ check.
+
+ Test: added new test cases in the existing test (break-out-of-empty-list-item.html)
+
+ * editing/CompositeEditCommand.cpp:
+ (WebCore::CompositeEditCommand::breakOutOfEmptyListItem):
+
2012-04-27 Ian Vollick <[email protected]>
[chromium] Add pause and resume support for accelerated css animations.
Modified: trunk/Source/WebCore/editing/CompositeEditCommand.cpp (115519 => 115520)
--- trunk/Source/WebCore/editing/CompositeEditCommand.cpp 2012-04-28 00:07:49 UTC (rev 115519)
+++ trunk/Source/WebCore/editing/CompositeEditCommand.cpp 2012-04-28 00:10:23 UTC (rev 115520)
@@ -1267,21 +1267,23 @@
if (!newBlock)
newBlock = createDefaultParagraphElement(document());
- if (isListItem(emptyListItem->nextSibling())) {
- // If emptyListItem follows another list item, split the list node.
- if (isListItem(emptyListItem->previousSibling()))
+ Node* previousListNode = emptyListItem->isElementNode() ? toElement(emptyListItem)->previousElementSibling(): emptyListItem->previousSibling();
+ Node* nextListNode = emptyListItem->isElementNode() ? toElement(emptyListItem)->nextElementSibling(): emptyListItem->nextSibling();
+ if (isListItem(nextListNode) || isListElement(nextListNode)) {
+ // If emptyListItem follows another list item or nested list, split the list node.
+ if (isListItem(previousListNode) || isListElement(previousListNode))
splitElement(static_cast<Element*>(listNode), emptyListItem);
- // If emptyListItem is followed by other list item, then insert newBlock before the list node.
+ // If emptyListItem is followed by other list item or nested list, then insert newBlock before the list node.
// Because we have splitted the element, emptyListItem is the first element in the list node.
// i.e. insert newBlock before ul or ol whose first element is emptyListItem
insertNodeBefore(newBlock, listNode);
removeNode(emptyListItem);
} else {
- // When emptyListItem does not follow any list item, insert newBlock after the enclosing list node.
+ // When emptyListItem does not follow any list item or nested list, insert newBlock after the enclosing list node.
// Remove the enclosing node if emptyListItem is the only child; otherwise just remove emptyListItem.
insertNodeAfter(newBlock, listNode);
- removeNode(isListItem(emptyListItem->previousSibling()) ? emptyListItem : listNode);
+ removeNode(isListItem(previousListNode) || isListElement(previousListNode) ? emptyListItem : listNode);
}
appendBlockPlaceholder(newBlock);
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
