41862 - nsEditor::CloneAttributes should not use undoable transactions if target element isn't in document

Status: VERIFIED FIXED

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

Description

[Charles Manske]

Change nsEditor::CloneAttributes(nsIDOMNode *aDestNode, nsIDOMNode *aSourceNode)
to test if aDestNode is in the current document. If it isn't, we should go 
through the transaction system.

Comment 1

[rubydoo123]

assigning this one to Simon

Comment 2

[Simon Fraser [no longer active]]

Over to charley

Comment 3

[Charles Manske]

Sorry, forgot to assign it to me when created.

Comment 4

[Charles Manske]

Attachment: Fix for the bug

Comment 5

[Charles Manske]

After thinking about this bug, it is an important architecture issue.
If we don't fix this, the setting attributes on an element before it is 
inserted in the page will still use the transaction system, which is shouldn't.
This will through off the undo stack so undo won't undo the last real action
and the user will be confused. There is also the risk of crashing since the 
saved undo information won't point to elements in the page and undo/redo may
try to access released elements.
The fix is very straightforward -- it is attached.
Nominating for beta2 -- I've already discussed the issues with beppe and she
agrees it should be considered.

Comment 6

[Charles Manske]

Approved to checkin by beppe.

Comment 7

[Charles Manske]

Checked in last week.

Comment 8

[sujay]

Charley, can you please verify this one?

thanks...

Comment 9

[Charles Manske]

Traced through along the 2 possible code paths and it looks good.