487524 - Attempting to delete single item in list containing sublist also removes next item

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect, P2)

Description

[Annie Sullivan]

Steps to reproduce (Firefox 3.1 beta 3):

1. Go to midas demo: http://www.mozilla.org/editor/midasdemo/
2. Enter the following HTML:
<ul><li>one</li><li>two</li><ul><li>a</li></ul></ul>
3. Uncheck "View HTML Source" and put the cursor at the start of "one".
4. Press Shift+Down Arrow to select the entire item.
5. Press Delete or Backspace to remove the item.

Actual Result:
Both first and second list items are removed. Resulting HTML:
<ul><ul><li>a</li></ul></ul>

Expected Result:
Only the first list item (which was selected) is removed. Expected HTML:
<ul><li>two</li><ul><li>a</li></ul></ul>
(This is what I see in Firefox 3.0.8)

Comment 1

[Mats Palmgren (inactive)]

Regression window:
2008-10-15-13
  http://hg.mozilla.org/mozilla-central/rev/3b4a3bfc99a7
2008-10-16-08
  http://hg.mozilla.org/mozilla-central/rev/99dbf3b2688b

http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2008-10-15+12%3A00&enddate=2008-10-16+09%3A00

Backing out (needs a bit of work) bug 157546 locally seems to fix it.

Comment 2

[Mats Palmgren (inactive)]

It's the last hunk in nsHTMLEditRules.cpp that causes the problem:
https://bugzilla.mozilla.org/attachment.cgi?id=342183&action=diff#mozilla/editor/libeditor/html/nsHTMLEditRules.cpp_sec3
We're asking JoinBlocks() to join the 2nd <li> to the top <ul>, ie its
current parent.  This is a "right block is inside left block" situation,
so JoinBlocks() calls MoveBlock() which decides to first call MoveContents()
for the <li> and then delete it.  MoveContents() calls MoveNodeSmart()
which finds that <ul> can't contain #text so it calls MoveContents()
(for the #text node!) which just falls through with a NS_OK result
and then MoveNodeSmart() deletes it (the #text).

Comment 3

[Mats Palmgren (inactive)]

Attachment: wip3

Comment 4

[Mats Palmgren (inactive)]

Attachment: mochitest.diff

Comment 5

[Mats Palmgren (inactive)]

Comment on attachment 371794 [details] [diff] [review]
wip3

Passes regression tests, without triggering the added assertion (on Linux).

Comment 6

[David Baron :dbaron:]

Looks like this has been reviewed.  Mats, can you land it, or do you want somebody else to?

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/mozilla-central/rev/5b0651d5e421

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/bc66918ca0cb