Changeset 115520 in webkit


Ignore:
Timestamp:
Apr 27, 2012 5:10:23 PM (12 years ago)
Author:
yi.4.shen@nokia.com
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:
Location:
trunk
Files:
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r115518 r115520  
     12012-04-27  Yi Shen  <yi.4.shen@nokia.com>
     2
     3        REGRESSION(113723): Pressing enter in this list example deletes the whole list
     4        https://bugs.webkit.org/show_bug.cgi?id=85016
     5
     6        Reviewed by Enrica Casucci.
     7
     8        The bug was caused by CompositeEditCommand::breakOutOfEmptyListItem, which calls isListItem
     9        on the empty list's siblings to decide which part of the list should get removed. However,
     10        the check fails when the empty list's sibling is a text node, or a list element (e.g. ul, ol).
     11        Fixed it by skipping empty list's non-element sibling and calling isListElement to do further
     12        check.
     13
     14        * editing/execCommand/break-out-of-empty-list-item-expected.txt:
     15        * editing/execCommand/script-tests/break-out-of-empty-list-item.js:
     16
    1172012-04-27  Tim Horton  <timothy_horton@apple.com>
    218
  • trunk/LayoutTests/editing/execCommand/break-out-of-empty-list-item-expected.txt

    r113723 r115520  
    1313PASS enterAtTarget('<ul><li>hello</li><br id="target"></ul>') is '<ul><li>hello</li></ul><div><br></div>'
    1414PASS enterAtTarget('<ul><br id="target"></ul>') is '<div><br></div>'
     15PASS enterAtTarget('<ul><li>hello</li>abc<li id="target"></li></ul>') is '<ul><li>hello</li>abc</ul><div><br></div>'
     16PASS 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>'
     17PASS 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>'
     18PASS 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>'
     19PASS 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>'
    1520PASS successfullyParsed is true
    1621
  • trunk/LayoutTests/editing/execCommand/script-tests/break-out-of-empty-list-item.js

    r113723 r115520  
    5252testBreakOutOfEmptyListItem('<ul><li>hello</li><br id="target"></ul>', '<ul><li>hello</li></ul><div><br></div>');
    5353testBreakOutOfEmptyListItem('<ul><br id="target"></ul>', '<div><br></div>');
     54testBreakOutOfEmptyListItem('<ul><li>hello</li>abc<li id="target"></li></ul>', '<ul><li>hello</li>abc</ul><div><br></div>');
     55testBreakOutOfEmptyListItem('<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>');
     56testBreakOutOfEmptyListItem('<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>');
     57testBreakOutOfEmptyListItem('<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>');
     58testBreakOutOfEmptyListItem('<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>');
    5459
    5560document.body.removeChild(testContainer);
  • trunk/Source/WebCore/ChangeLog

    r115519 r115520  
     12012-04-27  Yi Shen  <yi.4.shen@nokia.com>
     2
     3        REGRESSION(113723): Pressing enter in this list example deletes the whole list
     4        https://bugs.webkit.org/show_bug.cgi?id=85016
     5
     6        Reviewed by Enrica Casucci.
     7
     8        The bug was caused by CompositeEditCommand::breakOutOfEmptyListItem, which calls isListItem
     9        on the empty list's siblings to decide which part of the list should get removed. However,
     10        the check fails when the empty list's sibling is a text node, or a list element (e.g. ul, ol).
     11        Fixed it by skipping empty list's non-element sibling and calling isListElement to do further
     12        check.
     13
     14        Test: added new test cases in the existing test (break-out-of-empty-list-item.html)
     15
     16        * editing/CompositeEditCommand.cpp:
     17        (WebCore::CompositeEditCommand::breakOutOfEmptyListItem):
     18
    1192012-04-27  Ian Vollick  <vollick@chromium.org>
    220
  • trunk/Source/WebCore/editing/CompositeEditCommand.cpp

    r113723 r115520  
    12681268        newBlock = createDefaultParagraphElement(document());
    12691269
    1270     if (isListItem(emptyListItem->nextSibling())) {
    1271         // If emptyListItem follows another list item, split the list node.
    1272         if (isListItem(emptyListItem->previousSibling()))
     1270    Node* previousListNode = emptyListItem->isElementNode() ? toElement(emptyListItem)->previousElementSibling(): emptyListItem->previousSibling();
     1271    Node* nextListNode = emptyListItem->isElementNode() ? toElement(emptyListItem)->nextElementSibling(): emptyListItem->nextSibling();
     1272    if (isListItem(nextListNode) || isListElement(nextListNode)) {
     1273        // If emptyListItem follows another list item or nested list, split the list node.
     1274        if (isListItem(previousListNode) || isListElement(previousListNode))
    12731275            splitElement(static_cast<Element*>(listNode), emptyListItem);
    12741276
    1275         // If emptyListItem is followed by other list item, then insert newBlock before the list node.
     1277        // If emptyListItem is followed by other list item or nested list, then insert newBlock before the list node.
    12761278        // Because we have splitted the element, emptyListItem is the first element in the list node.
    12771279        // i.e. insert newBlock before ul or ol whose first element is emptyListItem
     
    12791281        removeNode(emptyListItem);
    12801282    } else {
    1281         // When emptyListItem does not follow any list item, insert newBlock after the enclosing list node.
     1283        // When emptyListItem does not follow any list item or nested list, insert newBlock after the enclosing list node.
    12821284        // Remove the enclosing node if emptyListItem is the only child; otherwise just remove emptyListItem.
    12831285        insertNodeAfter(newBlock, listNode);
    1284         removeNode(isListItem(emptyListItem->previousSibling()) ? emptyListItem : listNode);
     1286        removeNode(isListItem(previousListNode) || isListElement(previousListNode) ? emptyListItem : listNode);
    12851287    }
    12861288
Note: See TracChangeset for help on using the changeset viewer.