430392 - Pressing enter in div w/ contenteditable = true causes text nodes to move unexpectedly

Status: RESOLVED FIXED

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

Description

[Robbie Paplin]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b5) Gecko/2008032620 Firefox/3.0b5
Build Identifier: 

In the attached document, pressing enter twice causes the semicolons (a/k/a text nodes) to move between the Peyton Manning & Eli Manning span, instead of remaining in place between each span.

Reproducible: Always

Steps to Reproduce:
1.  Set the caret to the left of the semicolon between Tom Brady & Matt Hasselbeck
2.  Press the Enter key twice

Actual Results:  
The semi-colons between the contenteditable=false spans to migrate to text node between Peyton & Eli.


Expected Results:  
The semi-colons between the contenteditable=false spans should not move (other than the ones effected by the line breaks).

On Safari 3.1.1 pressing enter causes the browser to insert <div> wrapped <br>'s.
On IE 7.0 pressing Enter causes the browser to insert <p> tags and pressing Shift-Enter causes the browser to insert <br> tags.
However, despite the implementation differences, it seems to work as expected on other browsers.

Comment 1

[Robbie Paplin]

Attachment: Page that reproduces document corruption issue when pressing Enter

Comment 2

[Jesse Ruderman]

Confirmed using an up-to-date Mac trunk build.  Nice testcase.

Comment 3

[liucougar]

confirmed this with 0424 windows nightly build in windows xp

Comment 4

[Peter Van der Beken [:peterv]]

Attachment: v1

Comment 5

[Peter Van der Beken [:peterv]]

Attachment: v1

Now with mochitest.

CollapseAdjacentTextNodes should only collapse adjacent text nodes. GetPriorHTMLSibling finds the previous sibling that is editable, skipping siblings that are not editable, so the node that it returns can be a non-adjacent sibling. We should just use GetPreviousSibling instead.

Comment 6

[Peter Van der Beken [:peterv]]

Comment on attachment 318413 [details] [diff] [review]
v1

Low-risk fix for an annoying contenteditable bug. Has patch.

Comment 7

[Peter Van der Beken [:peterv]]

Comment on attachment 318413 [details] [diff] [review]
v1

I meant has testcase :-/.

Comment 8

[Damon Sicore (:damons)]

Comment on attachment 318413 [details] [diff] [review]
v1

a1.9+=damons