Closed Bug 436703 Opened 12 years ago Closed 11 years ago

select all + Copy/paste in contenteditable div pastes the editable div inside itself


(Core :: DOM: Editor, defect)

Not set





(Reporter: stig, Assigned: liucougar)





(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9) Gecko/2008051202 Firefox/3.0
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9) Gecko/2008051202 Firefox/3.0

When a div has the contenteditable attribute set to true, and the user selects all contents (cmd + a, mouse selection, execCommand or range.selectAllChildren(DOMNode) ) and copies the contents: When you then paste the contents, the parent <div /> is pasted inside it self.

Reproducible: Always

Steps to Reproduce:
1. Go to: or any other demonstration of contenteditable in firefox 3
2. Select all contents of any of the editable elements
3. Copy the contents the paste it several times.
Actual Results:  
The editable element is pasted inside it self.

Expected Results:  
Selecting the contents of an editable element should not select the element itself.
Seeing the same on Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008052707 Minefield/3.0pre

-> core > editor
Component: General → Editor
Ever confirmed: true
Product: Firefox → Core
QA Contact: general → editor
Version: unspecified → Trunk
I have hit the same problem with:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/2008101315 Ubuntu/8.10 (intrepid) Firefox/3.0.3
As this bug persists in Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5, I deviced a workaround described at

Bodo Schulze
Attached patch proposed patch (obsolete) — Splinter Review
don't promote to an ancestor node if it has different isEditable state
Attachment #400912 - Flags: review?(roc)
Attachment #400912 - Flags: review?(roc) → review?(peterv)
Comment on attachment 400912 [details] [diff] [review]
proposed patch

Looks OK to me. However an editor person needs to review this. Also needs a test. Thanks!
Attached patch patch with test case (obsolete) — Splinter Review
add unit test to the patch
Assignee: nobody → liucougar
Attachment #400912 - Attachment is obsolete: true
Attachment #405639 - Flags: review?(peterv)
Attachment #400912 - Flags: review?(peterv)
Comment on attachment 405639 [details] [diff] [review]
patch with test case

Why does this only check the frontNode against its parent?
I meant against the node.
I don't think I am certain what you are asking, but here is my guess

if you are asking why the patch only checks isEditable on the frontNode, because when it evaluates the (iNode->IsEditable() != isEditable), both (frontNode != parent) and (endNode != parent) evaluate to false, which means frontNode==parent==endNode,
Comment on attachment 405639 [details] [diff] [review]
patch with test case

>diff -r b2057b7554e6 content/base/src/nsDocumentEncoder.cpp

>+  nsCOMPtr<nsINode> inINode = do_QueryInterface(*ioNode);

I'd just name this one |node|.

>+      nsCOMPtr<nsINode> iNode = do_QueryInterface(frontNode);

and make this frontINode.

>       // if both endpoints were promoted one level, keep looping - otherwise we are done.

Please add to this comment, probably making it something like: "Keep looping if both endpoints were promoted one level, or the common ancestor is editable and the node isn't or vice versa."

>-      if ( (frontNode != parent) || (endNode != parent) )
>+      if ( (frontNode != parent) || (endNode != parent) || (iNode->IsEditable() != isEditable) )

I think it's ok to check against the node's editability as opposed to just checking for editability because we only walk up one level at a time.
Attachment #405639 - Flags: review?(peterv) → review+
all the points in the last comment are addressed (the last point in the last comment does not look like a suggestion for a change, correct me if I am wrong)
Attachment #405639 - Attachment is obsolete: true
Attachment #407076 - Flags: review+
Keywords: checkin-needed
Closed: 11 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
This topic should be reopen.

Bug currently appears if (and only if) tag is a span :
See Also: → 1466417
You need to log in before you can comment on or make changes to this bug.