The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in Firefox 17

Status

()

Core
Editor
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Matthew DeLambo, Assigned: ayg)

Tracking

({regression, testcase})

15 Branch
mozilla18
x86
Mac OS X
regression, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox17+ fixed, firefox18+ fixed)

Details

(Whiteboard: [qa-])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
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:

http://jsfiddle.net/rFuxj/2/

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
(Reporter)

Comment 2

5 years ago
(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:

http://jsfiddle.net/rFuxj/3/

Comment 3

5 years ago
Created attachment 663936 [details]
Reporter's testcase

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.
(Reporter)

Comment 4

5 years ago
Created attachment 664030 [details]
Range Test Case

Attached...

Updated

5 years ago
Attachment #664030 - Attachment mime type: text/plain → text/html

Updated

5 years ago
Attachment #663936 - Attachment is obsolete: true

Comment 5

5 years ago
Regression range:

m-c
good=2012-05-18
bad=2012-09-19
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e794cef56df6&tochange=642d1a36702f

There are some bugs about core:editor (Ms2ger & Aryeh Gregor) in the changelog, surely one of them is the culprit. Bisection needed.
Status: UNCONFIRMED → NEW
Component: Untriaged → Editor
Ever confirmed: true
Keywords: regression, regressionwindow-wanted, testcase
Product: Firefox → Core

Comment 6

5 years ago
Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/3b86eaa02d91
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120518080514
Bad:
http://hg.mozilla.org/mozilla-central/rev/17422a2d0c70
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120518104452
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3b86eaa02d91&tochange=17422a2d0c70


Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/11780e80c8c3
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120517220915
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/e8ebc8f1825e
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120517232316
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=11780e80c8c3&tochange=e8ebc8f1825e

Comment 7

5 years ago
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>
<script>
getSelection().collapse(document.querySelector("span"), 0);
getSelection().extend(document.querySelector("span"), 1);
document.execCommand("inserttext", false, "x");
document.body.textContent = document.body.firstChild.innerHTML;
</script>

This should output

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

but in current Gecko it outputs

  x<br>

Removing the <br> fixes the problem, oddly!

Bug 590640 is probably the culprit, yeah.  I'll investigate.
Status: NEW → ASSIGNED
Created attachment 665378 [details] [diff] [review]
Patch

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): https://tbpl.mozilla.org/?tree=Try&rev=3834a7a9488f
Attachment #665378 - Flags: review?(ehsan)

Updated

5 years ago
Blocks: 590640
Keywords: regressionwindow-wanted
Comment on attachment 665378 [details] [diff] [review]
Patch

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

Looks great!
Attachment #665378 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/5ef5ee5e312c
Target Milestone: --- → mozilla18
Comment on attachment 665378 [details] [diff] [review]
Patch

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?
https://hg.mozilla.org/mozilla-central/rev/5ef5ee5e312c
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Comment on attachment 665378 [details] [diff] [review]
Patch

[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+
https://hg.mozilla.org/releases/mozilla-aurora/rev/0ca632fe6289
status-firefox17: --- → fixed
status-firefox18: --- → fixed
tracking-firefox17: --- → +
tracking-firefox18: --- → +
Keywords: verifyme
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.