Bug 82690

Summary: Pressing enter on blank line after bullet deletes entire bulleted line
Product: WebKit Reporter: Adele Peterson <adele>
Component: HTML EditingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: adele, enrica, max.hong.shen, rniwa, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
test
none
First try
none
use isListItem none

Adele Peterson
Reported 2012-03-29 22:44:00 PDT
Created attachment 134726 [details] test Pressing enter on blank line after bullet deletes entire bulleted line See the attached test case. When you hit enter, the text disappears. <rdar://problem/10701992>
Attachments
test (261 bytes, text/html)
2012-03-29 22:44 PDT, Adele Peterson 		no flags
Details
First try (6.03 KB, patch)
2012-04-04 08:13 PDT, Yi Shen 		no flags
DetailsFormatted DiffDiff
use isListItem (5.58 KB, patch)
2012-04-04 14:08 PDT, Yi Shen 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Yi Shen
Comment 1 2012-04-04 08:13:11 PDT
Created attachment 135594 [details] First try
Ryosuke Niwa
Comment 2 2012-04-04 09:33:39 PDT
Comment on attachment 135594 [details] First try View in context: https://bugs.webkit.org/attachment.cgi?id=135594&action=review > Source/WebCore/editing/CompositeEditCommand.cpp:1278 > - if (emptyListItem->renderer()->previousSibling()) > + if (emptyListItemRenderer->previousSibling()) Why can't we just call isListItem on the previous node? > Source/WebCore/editing/CompositeEditCommand.cpp:1290 > - removeNode(emptyListItem->renderer()->previousSibling() ? emptyListItem : listNode); > + removeNode(emptyListItemRenderer->previousSibling() ? emptyListItem : listNode); Ditto.
Yi Shen
Comment 3 2012-04-04 14:08:47 PDT
Created attachment 135675 [details] use isListItem
Yi Shen
Comment 4 2012-04-04 14:11:48 PDT
Thanks for the review and the suggestion. I agree that we should use isListItem instead, which is more clear :) (In reply to comment #2) > (From update of attachment 135594 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=135594&action=review > > > Source/WebCore/editing/CompositeEditCommand.cpp:1278 > > - if (emptyListItem->renderer()->previousSibling()) > > + if (emptyListItemRenderer->previousSibling()) > > Why can't we just call isListItem on the previous node? > > > Source/WebCore/editing/CompositeEditCommand.cpp:1290 > > - removeNode(emptyListItem->renderer()->previousSibling() ? emptyListItem : listNode); > > + removeNode(emptyListItemRenderer->previousSibling() ? emptyListItem : listNode); > > Ditto.
Enrica Casucci
Comment 5 2012-04-06 15:59:56 PDT
Comment on attachment 135675 [details] use isListItem Looks good to me. Thanks for fixing this!
Yi Shen
Comment 6 2012-04-10 06:31:08 PDT
Comment on attachment 135675 [details] use isListItem Thanks for the review!
WebKit Review Bot
Comment 7 2012-04-10 09:20:00 PDT
Comment on attachment 135675 [details] use isListItem Clearing flags on attachment: 135675 Committed r113723: <http://trac.webkit.org/changeset/113723>
Ryosuke Niwa
Comment 8 2012-04-30 11:53:32 PDT
Closing the bug since it's fixed in http://trac.webkit.org/changeset/113723.
Note You need to log in before you can comment on or make changes to this bug.