47903 - WRONG_DOCUMENT_ERR not being thrown

Status: RESOLVED FIXED

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

Description

[Bob Clary [:bc] (inactive)]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; m18) Gecko/20000730
BuildID:    2000073020

Creating a node from one Document and placing it in a different Document should
throw a WRONG_DOCUMENT_ERR Exception. This does not happen using appendChild or
insertBefore.  I assume replaceChild has the same problem but I have not tested
it yet.


Reproducible: Always
Steps to Reproduce:
1.doc = document.implementation.createDocument(null, 'DOC', null);
2.elm = document.createElement('p');
3.doc.documentElement.appendChild(elm);

Actual Results:  Does not throw an Exception

Expected Results:  Should throw WRONG_DOCUMENT_ERR

Please see: http://www.w3.org/TR/2000/CR-DOM-Level-2-20000510/core.html#ID-184E7107

I believe this is related to Bug http://bugzilla.mozilla.org/show_bug.cgi?id=27382

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Confirming.  I was actually thinking of filing this bug yesterday...

I don't think we throw WRONG_DOCUMENT_ERR anywhere.  At least I haven't seen
it...

Comment 2

[Johnny Stenback (:jst)]

This bug has been marked "future" because the original netscape engineer
working on this is over-burdened. If you feel this is an error, that you or
another known resource will be working on this bug,or if it blocks your work
in some way -- please attach your concern to the bug for reconsideration.

Comment 3

[Jan Carpenter]

Mass update of qa contact

Comment 4

[Prashant Desale]

QA contact Update

Comment 5

[Prashant Desale]

Updating QA contact to Shivakiran Tummala.

Comment 6

[Fabian Guisset]

Attachment: Testcase for bug 47903 (URL is down)

Here's the testcase. Currently the node created by a document can be
successfully appended to another document. I'm guessing comparing the two
documents returned by nsIContent::GetDocument() should be enough to fix this if
we really to fix it...

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

No, when we remove content from a document we null its document pointer.  See
bug 52732.

Comment 8

[Johnny Stenback (:jst)]

A element's nsINodeInfo knows the document that created it, so this bug is
fixable by comparing those pointers, but I'm not sure we want to fix this...

Comment 9

[Jim Ley]

This bug is getting rather long in the tooth, and there increasing discussion 
in newsgroups etc. believing that Mozilla's behaviour is correct and 
implementing solutions based on it (and IE's wrong)  This bug needs to be fixed 
urgently to avoid the situation where a large number of pages will break when 
it is fixed, it's also apparant (and more common) when moving nodes from one 
document to another across frames or windows.

I agree from a nice-and-easy javascript perspective it would be better to not 
have the same Document requirement, but standards conformance require it, if 
you had a problem with it - you should've raised at the relevent DOM Review 
process.

Comment 10

[Sven]

Mozilla Developers should consider to fix this bug.
When reading the comments i feel like "it's not a bug, it's a feature".
The next thing is, that i don't understand what's so difficult in fixing this bug.
There are only two conditions when someone is allowed to insert a node:
- the node "owned" by the same document
- the node isn't "owned" by any document.

BTW: is it allowed to insert a node twice into the same document? or do i have
to clone it first?

Comment 11

[Johnny Stenback (:jst)]

Mass-reassigning bugs to [e-mail]

Comment 12

[Peter Van der Beken [:peterv]]

Taking.

Comment 13

[Boris Zbarsky [:bzbarsky]]

So... if we want to fix this bug, it's pretty simple, yes.  The problem is that
a lot of code depends on this bug (including chrome code)....  Does IE actually
throw this exception?

Comment 14

[Peter Van der Beken [:peterv]]

Well, first we need to ensure everything has an ownerDocument. I have a tree
that makes it so and fixes this (it's still in merge-hell right now though).
However, my testing has shown that we should probably not throw this exception,
it lead to too many things breaking.

Comment 15

[Lars Huttar]

(In reply to comment #13)
> So... if we want to fix this bug, it's pretty simple, yes.  The problem is that
> a lot of code depends on this bug (including chrome code)....  Does IE actually
> throw this exception?

Yes, IE does, although the error message is "Interface not supported".

This bug really should be fixed. One of the big selling points for
Mozilla/Firefox is standards compliance. I sympathize with the fact that this
functionality is convenient for coding; but:
(1) you can easily work around the different-document limitation by using
importNode() (DOM Level 2) if it is implemented.
(2) when Microsoft adds nonstandard features that encourage web authors to write
nonstandard code that breaks other browsers, open-source people usually cry
fowl, and justifiably so. It would be hypocritical for Mozilla to wilfully
continue to support nonstandard features.

Personally, I was bitten by this bug in the past couple of weeks. I wrote code
for Firefox, since that is my main browser. Being fairly new to DOM, I wrote the
code by looking up the references and trial-and-error. Once it worked, I thought
I had it right. Then I tried it in IE and had a real hard time figuring out why
it didn't work. The disappointing answer: FF has nonstandard behavior.

So please, if you claim to support DOM Level 1, conform to the spec.

One alternative would be to supply extension functions, e.g.
_mozilla_appendChild(), that are more permissive, but are clearly marked as
being browser-specific.

Comment 16

[Peter Van der Beken [:peterv]]

Attachment: Patch

Here's a patch that does this, albeit turned of with a define. We still need to
fix textnode's, comments, etc to have a ownerDocument even when they're not
attached to a document (bug 27382). After that we'll need to decide if the
things that this'll break are worth it, I'm personally sceptical.

Comment 17

[Peter Van der Beken [:peterv]]

Attachment: Patch

Updated to trunk. Still unsure whether we'll be able to do this (though I think the recent fix for the ownerDocument of |new Option()| and |new Image()| might help this).

Comment 18

[Boris Zbarsky [:bzbarsky]]

I think we want to do this...  Not doing it would make various XBL2 stuff down the road a lot more painful.

Comment 19

[Brendan Eich [:brendan]]

See http://lists.w3.org/Archives/Public/www-dom/2005OctDec/0073.html and its thread, and if you do fix this, don't break existing website compatibility.

/be

Comment 20

[Boris Zbarsky [:bzbarsky]]

So what websites are doing this, given that IE throws WRONG_DOCUMENT_ERR?  Or does it not always throw?

Comment 21

[Boris Zbarsky [:bzbarsky]]

Brendan, see http://lists.w3.org/Archives/Public/www-dom/2005OctDec/0095.html

I can't see how we can fix this bug without breaking compat with sites that have Gecko-specific code that relies on this bug.  I don't propose we try keeping such compat.  Further, it looks to me like the fact that we have this bug is in fact screwing over other browsers who are trying to be spec-compliant (Safari, iCab, etc), and being harmful to the web as a whole.

So I say let's fix it.  If our UI sucks too much in terms of this and we can't get any traction on fixing the UI, I say fix this for everything except XUL and stop breaking the web to support XUL.  Wouldn't be the first time our XUL dom is buggy where for other languages everything works.  :(  But I do suspect that this is not that huge a deal for our UI...  And now that we have a functioning importNode any fallout can be easily fixed.

Comment 22

[Brendan Eich [:brendan]]

bz: yeah, saw that.  Too bad, and we need evang bugs, or at least some bclary spidering to try to see whether fixing this bug will break notable content (AJAX apps, in particular).

/be

Comment 23

[Bob Clary [:bc] (inactive)]

I could try my normal approaches with a patched build that actually threw the exception but, again, for this type of investigation I'm not sure how much coverage of the important cases I would get. It wouldn't hurt though to try.

Another approach is to recruit interested extension/web app developers (especially enterprise class) and see how it flies with them. It would be nice if we had such a group already identified that we could ping about issues like this.

Comment 24

[Peter Van der Beken [:peterv]]

Attachment: Patch

Updated to trunk.

Comment 25

[Peter Van der Beken [:peterv]]

Attachment: Patch

Save first, then attach.

Comment 26

[Boris Zbarsky [:bzbarsky]]

Fwiw I think we should try checking this in and see how it goes...

Comment 27

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 214741 [details] [diff] [review]
Patch

>Index: content/base/src/nsDOMAttributeMap.cpp
>+    nsCOMPtr<nsIDOMNode> thisNode = do_QueryInterface(mContent);
>+    nsCOMPtr<nsIDOMDocument> thisDocument;
>+    thisNode->GetOwnerDocument(getter_AddRefs(thisDocument));

Why not:

  nsCOMPtr<nsIDOMDocument> thisDocument =
    do_QueryInterface(mContent->GetOwnerDoc());

?

Other than that, looks good!

sr=bzbarsky

Comment 28

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I'm very sceptical that this won't break the web. I wonder which documents were tested in IE. I suspect it was only tested for moving nodes between HTML and XML documents. Not HTML to HTML or XML to XML.

Also, see http://lists.w3.org/Archives/Public/www-dom/2005OctDec/0024.html

Comment 29

[Peter Van der Beken [:peterv]]

Attachment: Testcase

IE 6 throws.

Comment 30

[Peter Van der Beken [:peterv]]

(In reply to comment #28)
> I'm very sceptical that this won't break the web. I wonder which documents were
> tested in IE. I suspect it was only tested for moving nodes between HTML and
> XML documents. Not HTML to HTML or XML to XML.

See attachment 214886 [details], IE 6 throws for HTML to HTML at least. The problem will not be with pages written for IE but with Mozilla-specific parts of pages. We're not arguing for turning this on for any of the branches but for an alpha release. Given that it improves compatibility with IE and conformance with the DOM spec, I think we should bite this bullet for 1.9a.

Comment 31

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

We have to implement AdoptNode before we can do this. Otherwise there is no way to move a node between documents.

Additionally, can't you remove remove the code in a few places that update the nodeinfo and rebind parent when a node is moved from one doc to another.

Comment 32

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 214741 [details] [diff] [review]
Patch

Oh, and please compare nsIDocument pointers instead of nsIDOMDocument pointers, that should make for smaller and faster code.

And you can remove the comments about QIing to nsISupports, we rely elsewhere on that QIing to nsIDocument always return the same pointer.

Comment 33

[Boris Zbarsky [:bzbarsky]]

> Otherwise there is no way to move a node between documents.

importNode...  In fact, we could make adoptNode just return importNode for now if we want -- that's perfectly OK per DOM spec last I checked.  That said, I'd be happy enough to impl adoptNode.

> Additionally, can't you remove remove the code in a few places

Not while the nsIContent apis are used cross-document (esp. for XBL). If we fixed the XBL use case to use a non-cloning adoptNode and added asserts to catch current cases that relied on auto-adoption, then we could maybe remove that code.

Comment 34

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I don't really consider importNode a good alternative since it's much heavier-weight then adoptNode is. Additionally I bet a lot of users will fail to use the returned node rather then the original node.

Comment 35

[Boris Zbarsky [:bzbarsky]]

Would you be ok with implementing adoptNode in terms of importNode for now so we can give this change bake time and committing to a better impl of adoptNode for 1.9?  I'd be happy to implement adoptNode, but I just won't have time till I get back at the end of March and I'd really like us to get as much testing of this change as possible...

Comment 36

[Boris Zbarsky [:bzbarsky]]

Hmm... Then again, maybe implementing in terms of importNode is not OK... I was looking at:

  When it fails, applications should use Document.importNode() instead.

But that's not talking about the DOM impl but the DOM consumer, eh?

Comment 37

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Oh, yeah, I read it that way at first too. But the the definition of the returnvalue exlicitly says that the method returns null if adoption fails.

Comment 38

[Boris Zbarsky [:bzbarsky]]

OK.  Peter, if you're willing to do adoptNode, that's great and I promise expeditious review (I'll be out of town and treeless, but I'll have e-mail).  If you'd rather, I can do it when I get back.  I suspect you know that code better than I do, fwiw.  ;)

Comment 39

[Boris Zbarsky [:bzbarsky]]

We should probably add JS console warnings on the 1.8 branch in the spots where we plan to throw WRONG_DOCUMENT_ERR on trunk....

Comment 40

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

At the very least. I would still really prefer to have adoptNode implemented as well.

Comment 41

[Mike Schroepfer]

If you have a patch to get the JS console warnings we'd happily take this for the 1.8 branch.  Until then we are not going to block on this.

Comment 42

[Jesse Ruderman]

adoptNode (bug 330677) is in now.

Comment 43

[Peter Van der Beken [:peterv]]

Attachment: Patch

Comment 44

[Manfred Staudinger]

I'm incorrectly using
		range = document.createRange();
		range.selectNodeContents(src);
where src is a node from a document created by XMLHttpRequest.
Can you give an example (or a pointer to) what the user is
supposed to do? 

Comment 45

[Hallvord R. M. Steen]

What about this?

                range = src.ownerDocument.createRange();
                range.selectNodeContents(src);

Comment 46

[Manfred Staudinger]

Works fine, Thanks !!!

Comment 47

[Peter Van der Beken [:peterv]]

Attachment: Patch

Comment 48

[Peter Van der Beken [:peterv]]

Attachment: v2.1

Comment 49

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 242484 [details] [diff] [review]
v2.1

>Index: content/base/src/nsDOMDocumentType.cpp
>+    // DocumentType nodes are the only nodes that can have a null ownerDocument
>+    // according to the DOM spec, so we need to give them a new nodeinfo in that
>+    // case.

We also need to reparent the JS wrapper, in this case.  And probably throw if the nodePrincipal of |this| and aDocument is different, or something...  Or just assert, given the checks in appendChild/insertBefore?  Speaking of which, do those checks make it possible to insert a doctype into a document?

The rest of this looks great!

Comment 50

[Peter Van der Beken [:peterv]]

Attachment: v2.2

Turns out I did break inserting docType nodes, fixed that. I also added code to do the rewrapping, but I'm not entirely sure nsContentUtils::GetDocumentFromContext() will always give us the right document. I did see the rewrapping in a simple testcase.

Comment 51

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 243357 [details] [diff] [review]
v2.2

So why do we need the "old document"?  To get the right old scope, basically?

sr=bzbarsky, though it would be nice if we had a better way of getting the old scope here....  I'm just not sure how to do that.

Comment 52

[Jesse Ruderman]

This will break some extensions.  For example, it broke http://www.squarefree.com/extensions/search-keys/, which was creating nodes in chrome documents and using them in content documents, etc.  I just uploaded a new version that creates nodes in the correct documents.

It will also break some bookmarklets.  I'll add calls to importNode to my bookmarklets as needed.  For bookmarklets that move nodes between documents rather than copying nodes, I guess I'll use adoptNode when it is present (e.g. trunk) and not import at all when it is not (e.g. Firefox 2).

Comment 53

[Boris Zbarsky [:bzbarsky]]

> This will break some extensions. 

Yes.

> It will also break some bookmarklets.

Yes.

Too bad.  It'll also break some of our chrome, possibly...  That's also too bad -- we'll fix it when it happens.

See comment 39.  It's too bad that never happened.  :(

Comment 54

[Nickolay_Ponomarev]

Adding dev-doc-needed keyword to make sure we don't forget to document this change when it's done.

Comment 55

[Jesse Ruderman]

To be clear, I'm ok with this change and I'm glad it's happening during alpha.

Should I file a new bug for getting a warning added to Firefox 2.0.0.x?  I'm not sure how many extension developers would actually see it though.  I wish we could search extension source code easily (bug 358591).

Comment 56

[Boris Zbarsky [:bzbarsky]]

> Should I file a new bug for getting a warning added to Firefox 2.0.0.x?

Probably a good idea.

Comment 57

[Jesse Ruderman]

Ok, I filed that as bug 358670.

Comment 58

[Jesse Ruderman]

With this patch, the following code crashes Firefox:

javascript:document.renameNode([1], "foo", "bar");

The crash occurs after aNode fails to QI to nsINode:

4062      nsCOMPtr<nsINode> node = do_QueryInterface(aNode);
4063      if (!HasSameOwnerDoc(node)) {

But I'm not convinced that aNode is even an nsIDOMNode:

(gdb) whats aNode
0x30c2468 <_ZTV14nsXPCWrappedJS+8>:     0x304b78c <_ZN14nsXPCWrappedJS14QueryInterfaceERK4nsIDPPv>

So I'm confused.

Comment 59

[Boris Zbarsky [:bzbarsky]]

Yeah, any DOM-accessible method can have a random JSObject passed to any of its args.  It shouldn't assume the args QI to anything in particular...

Comment 60

[Jesse Ruderman]

I've been running with this patch for a week and haven't noticed any other problems.  I guess now we just need an updated patch for comment 58 and review from Sicking?

Comment 61

[Joao Eiras]

Plesae verify if bug 361241 is related, and if so, please try to fix it simultaneously.
https://bugzilla.mozilla.org/show_bug.cgi?id=361241

Comment 62

[Boris Zbarsky [:bzbarsky]]

> Plesae verify if bug 361241 is related

Very peripherally.

> please try to fix it simultaneously

"One bug per issue".

That patch will be a simple change, unrelated to the code this bug is changing.  We should keep them as two separate patches.

Comment 63

[Peter Van der Beken [:peterv]]

*** Bug 361241 has been marked as a duplicate of this bug. ***

Comment 64

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 243357 [details] [diff] [review]
v2.2

You need to protect against InsertChildAt being called with a node from a different document. With the current patch that can happen when InsertBefore is called and in the middle of moving the node from its old position to its new it is adopted into another document. It's possible that it can happen in other situations as well.

Comment 65

[Peter Van der Beken [:peterv]]

Attachment: v2.3

Comment 66

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 246094 [details] [diff] [review]
v2.3

>Index: content/base/src/nsGenericElement.cpp
>@@ -2314,6 +2269,10 @@ nsGenericElement::doInsertChildAt(nsICon
>   NS_PRECONDITION(!aParent || aParent->GetCurrentDoc() == aDocument,
>                   "Incorrect aDocument");
> 
>+  if (!aParent->HasSameOwnerDoc(aKid)) {
>+    return NS_ERROR_DOM_WRONG_DOCUMENT_ERR;
>+  }
>+

aParent can be null when inserting into a document. With that fixed, r=me

Comment 67

[Boris Zbarsky [:bzbarsky]]

NODE_FROM for the win!

Comment 68

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I want to add a big disclaimer though. I am very worried that this will break a lot of sites. We can certainly try to get them to change their script, but we have to be prepared to back this out if there is too much breakage out there once release rolls around.

Comment 69

[Boris Zbarsky [:bzbarsky]]

I think that if we advertise this well, and get Opera and Safari to undo their hacks, we should be ok....

Comment 70

[Peter Van der Beken [:peterv]]

Attachment: v2.4

Ugh, forgot to add some code after I found out nsDocument calls doInsertChildAt.

Comment 71

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 246103 [details] [diff] [review]
v2.4

r=me with the two things fixed we discussed:

* Add comment in nsDocType::BindToTree stating that we may want to move this code to
  nsDOMImplementation if we want to be able to depend on the wrappers and nodeinfos
  being right earlier on.

* Kill the nsDocument::RenameNode code

Thanks!

Comment 72

[Peter Van der Beken [:peterv]]

Attachment: v2.5

Comment 73

[Nickolay_Ponomarev]

Should an announcement be posted to one of the mailing lists and/or MDC?

Comment 74

[Boris Zbarsky [:bzbarsky]]

So bug 361709 comment 7 points out that IE doesn't enforce this for documents fetched via XMLHttpRequest...  So we may need to create a loophole for that...

Comment 75

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

No, please, lets not intoduce inconsistencies like that. If we can't follow the standard anyway, lets just do what's simplest for users and make all adoptions implicit.

Comment 76

[Peter Van der Beken [:peterv]]

(In reply to comment #75)
> No, please, lets not intoduce inconsistencies like that. If we can't follow the
> standard anyway, lets just do what's simplest for users and make all adoptions
> implicit.

I think we should first try to figure out exactly what it is IE is doing there, and how other browsers are affected. There is no absolute 'simplest' solution (implicit adoption is incompatible with IE at least for the HTML case).

Comment 77

[Brendan Eich [:brendan]]

(In reply to comment #76)
> (In reply to comment #75)
> > No, please, lets not intoduce inconsistencies like that. If we can't follow the
> > standard anyway, lets just do what's simplest for users and make all adoptions
> > implicit.
> 
> I think we should first try to figure out exactly what it is IE is doing there,
> and how other browsers are affected. There is no absolute 'simplest' solution
> (implicit adoption is incompatible with IE at least for the HTML case).

I agree with peterv.  This bug fills me with fear that we'll go invent yet another variation in the field for content authors to hate.

We should be tracking a draft standard from WHATWG or W3C that reflects what the IE6 does.  Is there such a spec?

/be

Comment 78

[Martin Honnen]

> I think we should first try to figure out exactly what it is IE is doing there,
> and how other browsers are affected. There is no absolute 'simplest' solution
> (implicit adoption is incompatible with IE at least for the HTML case).

It is difficult to say exactly what IE is doing as in the case of XML it is not IE doing anything but IE using MSXML doing things and different MSXML versions have different functionality. And script in IE can try to instantiate DOM documents of any existing MSXML version installed, indeed scripts often loop through known program ids e.g. ['Msxml2.DOMDocument.6.0', 'Msxml2.DOMDocument5.0', 'Msxml2.DOMDocument.4.0', 'Msxml2.DOMDocument.3.0'] and try to instantiate them until an object is created.

MSXML 5 and later (so currently MSXML 5 and MSXML 6) do support the importNode method
<http://msdn.microsoft.com/library/default.asp?url=/library/en-us/xmlsdk/html/f279d716-b575-49c1-b034-be79499924d2.asp>
but my current test show that they don't require you to use the method, if you insert a node created from one document into a node from another document then no exception is thrown, the node is simply adopted.

Older versions of MSXML do not support the importNode method at all but also allow you to insert a node (e.g. appendChild, insertBefore) created from one document to a node from another document.

IE 6 and IE 7 install MSXML 3 and IE 7 uses that for the XMLHttpRequest object it has. On the other hand the MS XML team is actively promoting to use MSXML 6 as the most advanced MSXML version that is being part of Vista and that way available in the future without requiring an extra install, see
<http://blogs.msdn.com/xmlteam/archive/2006/10/23/using-the-right-version-of-msxml-in-internet-explorer.aspx>

Moving nodes between DOM documents from different MSXML versions does not work however, it throws an error "wrong parameter" or even more specific "it is an error to mix objects from different versions of MSXML".

Comment 79

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Please mark bugs that got caused by this one as blocking this one, not the other way around.

Comment 80

[Dão Gottwald [:dao]]

(In reply to comment #79)
Bug 366336 and bug 361709 aren't regressions (as in: caused by this one) but Evangelism bugs, and hence depend on WRONG_DOCUMENT_ERR. That's just my impression though and maybe I take "depending on" and "blocking" too literally. Or my English is too bad to get these words right. (I feel the need for a "follow-ups" field.)

Comment 81

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Bug 361709 isn't depending on this one since yahoo could make that change even without this bug being fixed. We needed 361709 because of this bug and so i think we should consider it a "regression"

Comment 82

[Lars Huttar]

(In reply to comment #74)
> So bug 361709 comment 7 points out that IE doesn't enforce this for documents
> fetched via XMLHttpRequest...  So we may need to create a loophole for that...

The referenced comment does not say that IE doesn't enforce this for any documents fetched via XMLHttpRequest; that would be untrue. It says IE 6 and 7 don't enforce this for nodes transferred from an XMLHttpRequest response to a local store. What exactly that means, I don't know. But the former statement I know is not true because that's how I got involved with this bug in the first place... IE 6 threw an error when Firefox did not, on moving a node from an XMLHttpRequest response into an XML document.

Many thanks, developers, for getting this one fixed.

Comment 83

[Nickolay_Ponomarev]

I tweaked the importNode and adoptNode docs to mention this change, and added this to the list of website-affecting changes in Gecko 1.9:

http://developer.mozilla.org/en/docs/DOM:document.importNode
http://developer.mozilla.org/en/docs/DOM:document.adoptNode
http://developer.mozilla.org/en/docs/Gecko_1.9_Changes_affecting_websites

We could still use an article explaining step-by-step what changes need to be done  in order to fix a page in 1.9, while keeping compatibility with other major browsers. We could link to that page from the error message then.

Comment 84

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

...and those docs should mention both .importNode/.adoptNode as well as simply using the right document to create the node in the first place.

Comment 85

[Eric Shepherd [:sheppy]]

Added notes about this to the updating extensions/updating webapps articles:

http://developer.mozilla.org/en/docs/Updating_extensions_for_Firefox_3
http://developer.mozilla.org/en/docs/Updating_web_applications_for_Firefox_3

Comment 86

[Martijn Wargers (dead)]

I sure hope Yahoo and Google fix their 'bad' pages and I wish I could make those 'tech evangelism' bugs blocking1.9?.

Comment 87

[Boris Zbarsky [:bzbarsky]]

I would get a Core:Tracking bug filed to track them and request blocking on that.  I'd also contact schrep and see whether we can get someone to talk to them about this.

Comment 88

[Lars Huttar]

By the way, why does it say this bug (47903) depends on new ones like 401966?
Surely the relationship is reversed?

Comment 89

[Martijn Wargers (dead)]

Because this bug has caused those bugs.
That's how things are going around here, bugs that were caused by a bug that was fixed, you make it blocking that fixed bug.

Comment 90

[Lars Huttar]

(In reply to comment #89)
OK. Seems backwards to me. But nobody's asked me.  :-)

Comment 91

[Jesse Ruderman]

Web sites broken by this change are now tracked as dependencies of bug 407636 instead of dependencies of this bug, I think.

Comment 92

[Martijn Wargers (dead)]

(In reply to comment #87)
> I would get a Core:Tracking bug filed to track them and request blocking on
> that.  I'd also contact schrep and see whether we can get someone to talk to
> them about this.

Ok, bug 407636 has been filed about that.

Comment 93

[Jesse Ruderman]

Sicking started a thread about whether we should revert this change for Gecko 1.9 due to web compatibility concerns.

http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/efbb0861ca5d0e67

Comment 94

[Jeff Walden [:Waldo]]

In case anyone missed it, this bugfix was reverted in bug 418755.

I made a comment there, bug 418755 comment 18, which I think people should read and think about.  I do not think it will (nor do I strongly want it to) change any minds, but I do want people to read it, think about it, and consider it the next time we make difficult fixes for standards compliance.