Closed Bug 418755 Opened 13 years ago Closed 13 years ago

Stop throwing WRONG_DOCUMENT_ERR since the web can't deal.

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla1.9beta4

People

(Reporter: jst, Assigned: jst)

References

Details

(Keywords: dev-doc-complete)

Attachments

(1 file)

Attached patch Fix.Splinter Review
In bug 47903 we started throwing WRONG_DOCUMENT_ERR when nodes were inserted from one document into another, as the spec says it should, and IE does. Turns out it's too late, lots of sites already depend on it (see bug 407636), so we unfortunately need to revert our decision to throw here. Patch coming up that removes the exception throwing code, and uses AdoptNode() to properly move nodes from one document into the other.
Flags: blocking1.9+
Attachment #304646 - Flags: superreview?(jonas)
Attachment #304646 - Flags: review?(jonas)
Status: NEW → ASSIGNED
Priority: -- → P1
Is there anything we can do to get the spec to at least allow this behavior?

It almost starts to feel like maybe IE's "really standards mode, we mean it" is not such a bad idea after all.  :(
I should note that bug 407636 doesn't seem to have that many unfixed issues blocking it...
Isn't part of the problem that Gecko 1.8 can't deal as it doesn't have adoptNode implemented?
There was a plan to implement it (as basically a no-op).
Yes, and there was also a plan to show a WRONG_DOCUMENT_ERR warning in the error console on branch. But neither happened, and it's probably too late now, right?
The warning couldn't happen after l10n freeze of branch.  But the other can happen any time.
New sites are constantly found that break over this. And it's not small sites that don't get a lot of testing either, sites like google checkout and sj.se are used by hundreds of thousands of users.

Ultimately I just don't think this is a fight worth fighting. So what if we manage to get the web to deal with WRONG_DOCUMENT_ERROR being thrown? The only advantage that brings is that you'll have one less issue when porting your site to IE. Unlike something like IEs broken box model this doesn't make it harder to write code that works cross browser, or that forces us to have lots of quirks code that needs maintaining.
(In reply to comment #7)
> The only
> advantage that brings is that you'll have one less issue when porting your site
> to IE.

It also makes it possible to create a browser implementation where moving nodes between documents is hard or impossible due to architectural reasons (there is such an implementation already btw). AFAIK that's why the specification was written the way it is.

(In reply to comment #6)
> The warning couldn't happen after l10n freeze of branch.

For a warning only used by web developers, I'm not convinced an exception couldn't be made where an English string was created, inserted in every existing localization, and notice was made to localizers that they had the option of localizing it but wouldn't be required to do so.  You'd want to use keywords such that a non-localized version could be interpreted by searching for those keywords even if you didn't understand English, but that seems doable.
The warning alone doesn't help much, because 1) there's only little time left for developers to pick up and react on it and 2) there's no good way to react, due to the lacking adoptNode support. It might be better to add a (localized) warning for 1.9 and throw on 2.0, when developers don't have to deal anymore with 1.8.
> 2) there's no good way to react, due to the lacking adoptNode support.

This will be fixed in the next branch release.

> It might be better to add a (localized) warning for 1.9 and throw on 2.0

If we can't change this now, I doubt we'd be able to ever change it.  Worse yet, Safari and Opera (who changed to follow our behavior pretty recently due to compat issues) are less and less likely to be able to change back the longer the behavior stays that way.
Attachment #304646 - Flags: superreview?(jonas)
Attachment #304646 - Flags: superreview+
Attachment #304646 - Flags: review?(jonas)
Attachment #304646 - Flags: review+
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Since this makes us give up on ever enforcing correct ownerDocument it should be documented clearly. Also the documentation about the exception and the need for importNode/adoptNode in Firefox 3 should be removed.

Bah!
I think we should continue recommending that people adopt/import...
I'm all for making the spec auto-adopt nodes when it would have thrown WRONG_DOC. Shouldn't the ownerDocument change, though?
Yes, the code literally calls adoptNode if the node has the "wrong" ownerDocument.
I've updated the note that gets imported in places where WRONG_DOCUMENT_ERR was an issue to say that although we don't currently enforce this, we urge developers to update their code for future compatibility.  Fortunately, every reference to WRONG_DOCUMENT_ERR is done by importing one note.

http://developer.mozilla.org/en/docs/DOM:WRONG_DOCUMENT_ERR_note
This ship's sailed, and I don't really disagree with it.  There isn't a really good reason for behavior one way or the other, given that adoptNode requires some way to transfer nodes between documents anyway.  I'm less concerned about our specific behavior here than about what our decision to revert means for our ability to make breaking changes in the future.

There was a recent WASP-Microsoft round table discussion held concerning IE8's controversial opt-in for greater standards compliance[0]; one paragraph in particular stuck out for me:

> Chris Wilson: When people hit issues [with IE7b], they didn't really know
> whether it was just a bug in the beta or not, and they tended to presumed
> that it was. In fact, a lot of people, a lot of the major sites — you know,
> like, the, sort-of banking, high-end delivery sites — they didn't even want
> to touch it until we were done, until we were shipping RTM. Then they would
> pick it up and come up with a test path, and test it and everything else.
> Some of them were extremely ... I'm not sure what the word I'm looking for
> here is ... but they were really certain that that was the right thing for
> them to do. And I can kind of understand that, because they don't want to
> have to keep fixing it as they go.

It's hard enough getting sites fixed when they break in betas or nightlies.  While I'm not aware of sites which go so far as to wait for Firefox releases to update their sites, I cannot believe they do not exist.  The bug at issue here was fixed for over a year in nightlies and betas.  We repeatedly sent word to numerous sites known to have broken because of our change, telling them exactly what they needed to do to fix their sites.  Yahoo! in particular put up with a ton of grief in bug 361709 and in external fora until they finally fixed their site to conform to standards-compliant behavior.  Many sites updated in response to our change; a few high-profile sites did not or were slow to do so.

What message do we send to reticent sites by reverting this long-standing fix?

What message do we send to the sites who responded quickly and fixed their sites' bugs, only to see us revert our bugfix at the end of the release cycle and render their effort pointless?

Again, I don't intend this comment to change minds or the decision here, although if it did I wouldn't mind.  I just want people to think about the ramifications of our decision here with respect to successfully encouraging sites to fix nightly- or beta-only bugs because we implemented better support for standards.

0. http://www.glendathegood.com/wasp/transcript.html
Unfortunately the testing goes both way when we release a beta. We, and our community, get to test how the latest version of the browser works with sites, and sites get to test how they work with us.

So while I agree that ideally we should have known much earlier what path we wanted to take, I'm not really sure that we could have known.

I agree we should try to come up with some better plan though. Maybe we should have some sort of "web compat freeze" just like we have feature freeze and string freeze. We could declare a given beta as target for all changes to web compatibility changes and let all developers aim bugs that they think will affect web compat for that beta. We could then announce to the world that we think we are done and possibly include a list of things that we're still working on.

Just keeping track of which issues are still up in the air and which we have made up our minds on would help developers I suspect.

Something to talk about in a post mortem for sure.
I knew we were gonna back off; to borrow from Einstein, my little finger told me. I said so to at least mrbkap and jst if not sicking; prolly to shaver too.

But this is not an I-told-you-so (or not just one ;-). Consider how little was at stake: Waldo himself just wrote "There isn't a really good reason for behavior one way or the other, given that adoptNode requires some way to transfer nodes between documents anyway".

We need to thrash developers over more crucial issues, or simply evolve compatibly -- usually this means adding new alongside old. It's the way of the web and eventually, especially if the new really wins and sells itself, the old goes away.

/be
I largely agree. And I think that if we had had a 'web compat freeze date' we could have gotten to a conclusion and patch much sooner.
I have 2 suggestions.

1.  Add a do-nothing adoptnode to 1.8

2.  Propose altering the standard to not require this.  Or at least have the standard clarified to specify which cases throw this error and which do not.  This is kind of a way to perhaps get Microsoft to reveal this information.  If we could have actually figured out how to mimic the IE behavior and only throw the exception in the same cases they did, and not in the cases they don't, I doubt there would have been a really big issue here.
(In reply to comment #20)
> adoptNode requires
> some way to transfer nodes between documents anyway.

Note that implementing adoptNode is optional, I believe specifically to allow implementations that can't transfer nodes between documents.

I'm not interested in rehashing all the arguments, but I think it's good to be explicit about what arguments have been considered before deciding, namely that some people feel that in this case backwards compatibility trumps:
- making it easier to port code written for Firefox to other browsers (interoperability),
- removing another deviation from the spec, benefiting both web developers and other implementors (standards compliance),
- allowing a specific implementation of the DOM, with separation of documents and their contents in memory regions.

I personally don't consider all that "little at stake", and I wouldn't have argued for keeping the fix for bug 47903 in if I did.

A 'web compat freeze date' would help avoid forcing sites to fix code that we unbreak before a release, but I think most sites will always only test with final versions of browsers. What I think would also help is 1. fix our known standards compliance bugs quicker (bug 47903 was filed in 2000) and 2. get those fixes released more quickly (bug 47903 was fixed in 2006). I think this will be especially important for new standards that we start to implement, where backwards compatibility should be less of an issue.
(In reply to comment #22)
> This is kind of a way to perhaps get Microsoft to reveal this information.  If
> we could have actually figured out how to mimic the IE behavior and only throw
> the exception in the same cases they did, and not in the cases they don't, I
> doubt there would have been a really big issue here.

We can't mimic IE behaviour given that for XML it depends on whether documents are from the same version of MSXML (and they always throw for HTML). Having web authors use the right document to create their node in, or use adoptNode/importNode will lead to the most compatible code.
Bill: we have done both.

2.0.0.13 will have a functioning importNode (which lack very much impeded site
fixes, IMO, because they couldn't just use importNode everywhere, which is why
we've been trying to get it into the 2.x branch for many many moons).  Too late
to help, IMO.

We also proposed the change to the W3C, and it was rejected, because the
specification in question is not just for browsers, or something.

Your doubt was not validated by our testing.  A lot of sites had code paths for
"IE" and "everything else", and in the latter case (for us, Safari, Opera) they
didn't use adoptNode.

(I don't agree that the warning couldn't happen after l10n freeze on branch; JS
errors aren't localized, and never have been, so I don't think that putting
this _warning_ in as English only would have harmed anyone's experience. 
Certainly strange economics to say "we prefer silence to a warning that the
very few developers who don't understand basic English might have to puzzle
through", given that adoptNode itself is basically English!)

This was a case in which standards-compliant code would function correctly on
us (once we fixed the adoptNode disaster), excepting ACID-like explorations of
error pickiness, not one where we had to choose between compatibility and
letting standards-oriented content get the author's intent.  The IE7b
transcript excerpt to me is very much about the latter, where you have to break
existing content to let developers program to the standard.  I don't think
"this code is supposed to generate an error" is a useful enough bit of spec
compatibility to add to warrant the disruption.  Maybe in 2006 it would have
been, or if we'd got our branch story straighter so that we could have added
FX3 spec-compliance warnings, but we lost the fight to change the web in this
one, extremely low-value way.

I don't think it sets a dangerous precedent, and in fact I think that the more
dangerous precedent would be that we are willing to break lots of compatibility
for minimal practical gain: no new capabilities given to authors, no performance or security gain, still incompatibility with browsers who _followed_our_path_ to the current behaviour. (As though we reasoned by such things; see "can't turn off content XBL because it's used on some intranets" contra "yeah, it's OK, only Yahoo mail and a bunch of other sites and hardware that we know about had problems!")
Peter: for the interoperability gain, why not put in a patch that warns, on both trunk and branch?  It doesn't get you all the way to "forced to debug", but it seems like a minimal investment that would have some reasonable pay-off for well-meaning developers.  I think there's a patch in bug 358670 that's basically ready to go.
Peter: sorry, I didn't mean to make light of anything -- I agree with you that this would have been better done sooner. That it was not, though, again suggests to me some lack of urgency (at the time), and it seemed clear for a while that the "window" was closing, if it hadn't already closed.

/be
FWIW, I expect adoptNode() to be made optional in future DOM versions (much like the behaviour here).
Component: DOM: Core → DOM: Core & HTML
Depends on: 626262
You need to log in before you can comment on or make changes to this bug.