Closed Bug 487524 Opened 15 years ago Closed 15 years ago

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

Categories

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

1.9.1 Branch
defect

Tracking

()

RESOLVED FIXED

People

(Reporter: annie.sullivan, Assigned: MatsPalmgren_bugz)

References

(Depends on 1 open bug, )

Details

(Keywords: fixed1.9.1, regression)

Attachments

(2 files)

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)
Flags: blocking1.9.1?
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.
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:
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).
Attached patch wip3Splinter Review
Assignee: nobody → mats.palmgren
Attached patch mochitest.diffSplinter Review
Comment on attachment 371794 [details] [diff] [review]
wip3

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]
http://hg.mozilla.org/mozilla-central/rev/5b0651d5e421
Status: NEW → RESOLVED
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.

Attachment

General

Created:
Updated:
Size: