Closed Bug 430392 Opened 12 years ago Closed 12 years ago

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

Categories

(Core :: Editor, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9

People

(Reporter: rpaplin, Assigned: peterv)

Details

(Keywords: testcase)

Attachments

(2 files, 1 obsolete file)

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.
Confirmed using an up-to-date Mac trunk build.  Nice testcase.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: testcase
OS: Windows Vista → All
Hardware: PC → All
Assignee: nobody → peterv
confirmed this with 0424 windows nightly build in windows xp
Attached patch v1 (obsolete) — Splinter Review
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla1.9
Version: unspecified → Trunk
Attached patch v1Splinter Review
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.
Attachment #317751 - Attachment is obsolete: true
Attachment #318413 - Flags: superreview?(jst)
Attachment #318413 - Flags: review?(jst)
Attachment #318413 - Flags: superreview?(jst)
Attachment #318413 - Flags: superreview+
Attachment #318413 - Flags: review?(jst)
Attachment #318413 - Flags: review+
Comment on attachment 318413 [details] [diff] [review]
v1

Low-risk fix for an annoying contenteditable bug. Has patch.
Attachment #318413 - Flags: approval1.9?
Comment on attachment 318413 [details] [diff] [review]
v1

I meant has testcase :-/.
Comment on attachment 318413 [details] [diff] [review]
v1

a1.9+=damons
Attachment #318413 - Flags: approval1.9? → approval1.9+
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.