Closed Bug 487524 Opened 15 years ago Closed 15 years ago

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


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

1.9.1 Branch





(Reporter: annie.sullivan, Assigned: MatsPalmgren_bugz)


(Depends on 1 open bug, )


(Keywords: fixed1.9.1, regression)


(2 files)

Steps to reproduce (Firefox 3.1 beta 3):

1. Go to midas demo:
2. Enter the following HTML:
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:

Expected Result:
Only the first list item (which was selected) is removed. Expected HTML:
(This is what I see in Firefox 3.0.8)
Flags: blocking1.9.1?
Regression window:

Backing out (needs a bit of work) bug 157546 locally seems to fix it.
Blocks: 157546
Keywords: regression
OS: Windows XP → All
Hardware: x86 → All
Summary: [Regression] Attempting to delete single item in list containing sublist also removes next item → Attempting to delete single item in list containing sublist also removes next item
It's the last hunk in nsHTMLEditRules.cpp that causes the problem:
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).
Attached patch wip3Splinter Review
Assignee: nobody → mats.palmgren
Attached patch mochitest.diffSplinter Review
Comment on attachment 371794 [details] [diff] [review]

Passes regression tests, without triggering the added assertion (on Linux).
Attachment #371794 - Flags: superreview?(peterv)
Attachment #371794 - Flags: review?(peterv)
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
Attachment #371794 - Flags: superreview?(peterv)
Attachment #371794 - Flags: superreview+
Attachment #371794 - Flags: review?(peterv)
Attachment #371794 - Flags: review+
Looks like this has been reviewed.  Mats, can you land it, or do you want somebody else to?
Keywords: checkin-needed
Whiteboard: [needs review] → [needs landing]
Closed: 15 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [needs landing] → [needs 191 landing]
Depends on: 493641
You need to log in before you can comment on or make changes to this bug.