Closed Bug 787432 Opened 8 years ago Closed 7 years ago

After keyup in contenteditable, Firefox 15 replaces full node of inner text selection


(Core :: DOM: Editor, defect)

15 Branch
Not set



Tracking Status
firefox17 + fixed
firefox18 + fixed


(Reporter: delambo, Assigned: ayg)



(Keywords: regression, testcase, Whiteboard: [qa-])


(2 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_6_8) AppleWebKit/537.1 (KHTML, like Gecko) Chrome/21.0.1180.57 Safari/537.1

Steps to reproduce:

Please reference this jsfiddle:

In a contenteditable, in a keypress event handler, I use the current selection/range to insert a new node (span) with one character ("%"). Following that, I set the range to select the new character - startContainer and endContainer both equal new node, startPos and endPos equal 0 and 1, respectively.

Actual results:

The newly inserted node gets deleted on character insertion by the firefox browser.

Expected results:

Before FF 15 and still in Webkit, the browser should delete the selection and insert the newly typed character in place, without deleting the container (the newly inserted node).
Please describe what I have to do at your testcase to reproduce the bug, thanks
(In reply to Matthias Versen (Matti) from comment #1)
> Please describe what I have to do at your testcase to reproduce the bug,
> thanks

The result pane (bottom right pane) is a contenteditable region. Click in the editable and type a character. The new character should be green (wrapped in a <span.insert>) - this is the case for Firefox before version 15, IE9+, and Webkit.

I updated the fiddle with better comments at the following version:
Attached file Reporter's testcase (obsolete) —
Could you attach a simple html testcase instead of jsfiddle testcase, please.
Sometimes the result is different between jsfiddle and a local html file open in the same Firefox version.

It looks like it's the case if I transform the jsfiddle testcase into a local file.
Attached file Range Test Case
Attachment #664030 - Attachment mime type: text/plain → text/html
Attachment #663936 - Attachment is obsolete: true
Regression range:


There are some bugs about core:editor (Ms2ger & Aryeh Gregor) in the changelog, surely one of them is the culprit. Bisection needed.
Component: Untriaged → Editor
Ever confirmed: true
Product: Firefox → Core
Regression window(m-c)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120518080514
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120518104452

Regression window(m-i)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120517220915
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120517232316
Thanks. I bet my 2 cents on
Aryeh Gregor — Bug 590640 part 6 - Don't create empty style tags unless we're about to insert text in them; r=ehsan :D
Aryeh, can you look into this, please?
Assignee: nobody → ayg
More minimal test-case (uses insertText, so probably only works in Gecko/WebKit):

data:text/html,<!doctype html>
<div contenteditable><span class="insert">%</span><br></div>
getSelection().collapse(document.querySelector("span"), 0);
getSelection().extend(document.querySelector("span"), 1);
document.execCommand("inserttext", false, "x");
document.body.textContent = document.body.firstChild.innerHTML;

This should output

  <span class="insert">x</span><br>

but in current Gecko it outputs


Removing the <br> fixes the problem, oddly!

Bug 590640 is probably the culprit, yeah.  I'll investigate.
Attached patch PatchSplinter Review
One-line fix.  This has the negative side effect that it might leave empty wrappers around in some corner cases, namely if the text doesn't actually wind up being inserted for some reason, but that doesn't seem worth worrying about.  After writing a test, I found this was already tested by runtest.html anyway (yay for exhaustive test suites!).

Try, Linux64 opt only (since I don't anticipate platform-specific issues):
Attachment #665378 - Flags: review?(ehsan)
Comment on attachment 665378 [details] [diff] [review]

Review of attachment 665378 [details] [diff] [review]:

Looks great!
Attachment #665378 - Flags: review?(ehsan) → review+
Comment on attachment 665378 [details] [diff] [review]

This is a very simple fix for a regression which we shipped in 15.  I would like us to get it fixed in 16 and above.
Attachment #665378 - Flags: approval-mozilla-beta?
Attachment #665378 - Flags: approval-mozilla-aurora?
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Comment on attachment 665378 [details] [diff] [review]

[Triage Comment]
We have a full handle on this regression as it occurred in FF15, and we can't take non-critical bug fixes in our final beta. Approving for Aurora 17.
Attachment #665378 - Flags: approval-mozilla-beta?
Attachment #665378 - Flags: approval-mozilla-beta-
Attachment #665378 - Flags: approval-mozilla-aurora?
Attachment #665378 - Flags: approval-mozilla-aurora+
Actually, changing verifyme to [qa-] since we have a landed testcase in-testsuite.
Keywords: verifyme
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.