514487 - Cloning a document doesn't update the ID map

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

Attachment: testcase

Because cloning doesn't notify, the ID map isn't updated in the cloned doc.
Only cloning of XML documents was supported earlier, and for those documents
this is regression from bug 344258 (if I read hg blame correctly).

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

I need to get this fixed.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Ah, hmm.  So post-cloning you could walk the DOM and register all the IDs, I guess, right?

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Or update the ID map even when not notifying (have to check if that causes some problems).

Comment 4

[Boris Zbarsky [:bzbarsky]]

Yes; unless you remove the updates on notification.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

(In reply to comment #4)
> Yes; unless you remove the updates on notification.
Ofc I'd remove the updates on notification in that case.

I was thinking if someone listening ID changes may do something evil.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

They won't. In nsIDocument.h:
   * This fires at difficult times. It is generally not safe to do anything
   * which could modify the DOM in any way. Use
   * nsContentUtils::AddScriptRunner.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

A bit ugly, but this way I don't have care about whether the element has been notified etc. and this shouldn't trigger bug 515080.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 399120 [details] [diff] [review]
patch

I suppose this is doable, though I wonder why you need the GetOwnerDoc() call.  Take that out and r=me.

I don't know what bug 515080 has to do with this, though.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

(In reply to comment #8)
> I don't know what bug 515080 has to do with this, though.
Actually it doesn't have anymore.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

http://hg.mozilla.org/mozilla-central/rev/394140c609d5