Bug 85016

Summary: REGRESSION(113723): Pressing enter in this list example deletes the whole list
Product: WebKit Reporter: Adele Peterson <adele>
Component: HTML EditingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Major CC: adele, enrica, max.hong.shen, rniwa, tony, webkit.review.bot
Priority: P1 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
testcase
none
first try
none
fix the comments none

Adele Peterson
Reported 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>
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
DetailsFormatted DiffDiff
fix the comments (7.63 KB, patch)
2012-04-27 13:33 PDT, Yi Shen 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2012-04-26 16:52:10 PDT
Excellent! My colleagues and I were investigating this issue but we couldn't get a reduction!
Yi Shen
Comment 2 2012-04-26 17:01:16 PDT
Sorry for causing the regression:( Will look at it and try to fix it ASAP.
Yi Shen
Comment 3 2012-04-27 11:29:37 PDT
Created attachment 139241 [details] first try Will do more manual testing before landing the patch
Ryosuke Niwa
Comment 4 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.
Yi Shen
Comment 5 2012-04-27 13:33:43 PDT
Created attachment 139260 [details] fix the comments
Enrica Casucci
Comment 6 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.
WebKit Review Bot
Comment 7 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>
WebKit Review Bot
Comment 8 2012-04-27 17:09:48 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.