99523 - Backspace/ForwardDelete should do merging of disimilar blocks

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[Joe Francis]

If you backspace through typical content, the expect thing happens.  If you 
backspace at the front of a block, like a <p>, that is following another block of 
the same type, the two blocks get merged.  But if you backspace or forward delete 
across a boundary between two disimilar blocks, we will delete the first 
character in the other block, without merging the blocks.  This is disconcerting 
to the user because what gets deleted is not something that was even in the same 
block as their cursor.

What should happen is that the blocks should be (perhaps partially) merged.  I 
will have to opine a treatise on the details later, but right now it is simply 
too big to fit in this small margin.

Comment 1

[Joe Francis]

*** Bug 96317 has been marked as a duplicate of this bug. ***

Comment 2

[Joe Francis]

This is hard to fix.  I want to fix it yesterday.  Probably can't get it done 
before 1.0

Comment 3

[Joe Francis]

Attachment: diffs to editor

This patch does 99% of the heavy lifting by volume, and 95% by content.  I will
update it to more recent sources soon, but I wanted to get it up here now so I
don't lose it.

Comment 4

[Joe Francis]

Attachment: updated patch; merged to tip; unrelated patches removed

still need to fix GetPriorNode() as per GetNextNode() fixes

Comment 5

[Joe Francis]

Attachment: yet another improved patch

Finishes GetNextNode() work.  Improves GetPriorNode() work considerably.  Fixes
forward deletion, which did not work poperly in prior patches.

Comment 6

[kinmoz]

Comment on attachment 60796 [details] [diff] [review]
yet another improved patch

This patch makes things much better! 2 questions:

In MoveBlock(), in the block case, is there ever a situation where the
MoveContents() can fail because it's content can't be moved, and we still
delete it anyways? If there is, lets figure out the situation and file a new
bug so we can land this anyways.

MoveContents() should just call MoveNodeSmart().

sr=[e-mail]

Comment 7

[Joe Francis]

several improvements were made to the version of this patch I just checked in at
the suggestion of [e-mail].  Comments added, common code factored out,
and extraneous parameters removed.

Comment 8

[michael wendell]

Looks good to me on Win 2k using trunk build 2001123008. 

I am verifying this as fixed.

If anyone is still seeing this problem, feel free to repopen this bug.