Bug 85016 - REGRESSION(113723): Pressing enter in this list example deletes the whole list
Summary: REGRESSION(113723): Pressing enter in this list example deletes the whole list
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P1 Major
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2012-04-26 16:51 PDT by Adele Peterson
Modified: 2012-04-27 17:09 PDT (History)
6 users (show)

See Also:


Attachments
testcase (198 bytes, text/html)
2012-04-26 16:51 PDT, Adele Peterson
no flags Details
first try (7.10 KB, patch)
2012-04-27 11:29 PDT, Yi Shen
no flags Details | Formatted Diff | Diff
fix the comments (7.63 KB, patch)
2012-04-27 13:33 PDT, Yi Shen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adele Peterson 2012-04-26 16:51:12 PDT
Created attachment 139098 [details]
testcase

Pressing enter in this list example deletes the whole list

Regression from:
https://bugs.webkit.org/show_bug.cgi?id=82690
http://trac.webkit.org/changeset/113723

<rdar://problem/11313662>
Comment 1 Ryosuke Niwa 2012-04-26 16:52:10 PDT
Excellent! My colleagues and I were investigating this issue but we couldn't get a reduction!
Comment 2 Yi Shen 2012-04-26 17:01:16 PDT
Sorry for causing the regression:( Will look at it and try to fix it ASAP.
Comment 3 Yi Shen 2012-04-27 11:29:37 PDT
Created attachment 139241 [details]
first try

Will do more manual testing before landing the patch
Comment 4 Ryosuke Niwa 2012-04-27 13:06:43 PDT
Comment on attachment 139241 [details]
first try

View in context: https://bugs.webkit.org/attachment.cgi?id=139241&action=review

> Source/WebCore/editing/CompositeEditCommand.cpp:1272
> +    Node* previousListNode = emptyListItem->isElementNode() ? toElement(emptyListItem)->previousElementSibling(): emptyListItem->previousSibling();
> +    Node* nextListNode = emptyListItem->isElementNode() ? toElement(emptyListItem)->nextElementSibling(): emptyListItem->nextSibling();
> +    if (isListItem(nextListNode) || isListElement(nextListNode)) {

Maybe you can use functions in http://trac.webkit.org/browser/trunk/Source/WebCore/editing/InsertListCommand.cpp?

> Source/WebCore/editing/CompositeEditCommand.cpp:1273
>          // If emptyListItem follows another list item, split the list node.

This comment is no longer accurate. Please update.

> Source/WebCore/editing/CompositeEditCommand.cpp:1285
>          // When emptyListItem does not follow any list item, insert newBlock after the enclosing list node.
>          // Remove the enclosing node if emptyListItem is the only child; otherwise just remove emptyListItem.
>          insertNodeAfter(newBlock, listNode);

Ditto.
Comment 5 Yi Shen 2012-04-27 13:33:43 PDT
Created attachment 139260 [details]
fix the comments
Comment 6 Enrica Casucci 2012-04-27 13:42:43 PDT
Comment on attachment 139260 [details]
fix the comments

The fix looks good to me. Thanks for fixing it quickly.
Comment 7 WebKit Review Bot 2012-04-27 17:09:41 PDT
Comment on attachment 139260 [details]
fix the comments

Clearing flags on attachment: 139260

Committed r115520: <http://trac.webkit.org/changeset/115520>
Comment 8 WebKit Review Bot 2012-04-27 17:09:48 PDT
All reviewed patches have been landed.  Closing bug.