Closed Bug 802560 Opened 10 years ago Closed 10 years ago
.create Document() should have [Treat Null As=Empty String] for second arg
The DOM spec defines DOMImplementation.createDocument as: XMLDocument createDocument(DOMString? namespace, [TreatNullAs=EmptyString] DOMString qualifiedName, DocumentType? doctype); <http://dom.spec.whatwg.org/#domimplementation> We have it as nsIDOMDocument createDocument(in DOMString namespaceURI, in DOMString qualifiedName, in nsIDOMDocumentType doctype) raises(DOMException); "in DOMString" for us seems to be the same as "DOMString?" in WebIDL. Our code handles a null qualifiedName differently from an empty string, namely that it throws DOM_NAMESPACE_ERR if qualifiedName is null but namespaceURI is not; and if both are null, it just passes qualifiedName to nsContentUtils::CreateDocument(), which I *think* ultimately treats it as empty. (It calls .IsEmpty() -- which is true for null strings, right?) So I think the only difference here is our behavior of throwing. This causes us to fail one of Ms2ger's test. WebKit and Opera seem to agree with the spec; IE seems to throw like us. The spec's behavior seems more useful, so we may as well align on it.
I just realized this conflicts with bug 796903, so I'll wait for that to land before poking DOMImplementation stuff.
Depends on: 796903
What's the status of this ticket ? Starting with the new stable Firefox 19, calling DOMImplementation.createDocument with an undefined second argument throws an error : [Exception... "An attempt was made to create or change an object in a way which is incorrect with regard to namespaces" code: "14" nsresult: "0x8053000e (NamespaceError)"] This worked just fine before. I've updated my code to pass an empty string instead of undefined, but this may break some website now that Firefox 19 is out.
Ms2ger, could you take a look at this?
We don't know of any major website breakage from this change yet (nor over the past 18 weeks), so we'll leave this on the FF19 nom list for now.
If we don't see any more fallout in the next couple of weeks, we may decide to untrack for 20 and up.
Attachment #720011 - Flags: review?(bzbarsky)
Comment on attachment 720011 [details] [diff] [review] Patch v1 r=me Is there a bug on those remaining failures?
Attachment #720011 - Flags: review?(bzbarsky) → review+
(In reply to Boris Zbarsky (:bz) from comment #7) > Comment on attachment 720011 [details] [diff] [review] > Patch v1 > > r=me > > Is there a bug on those remaining failures? Thanks for reminding me to look at them... These tests look like they're wrong. Fixed upstream: https://dvcs.w3.org/hg/webapps/rev/80701dd9ac79
(In reply to Alex Keybl [:akeybl] from comment #5) > If we don't see any more fallout in the next couple of weeks, we may decide > to untrack for 20 and up. Untracking for release, but we'll consider a low risk uplift if found.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Comment on attachment 720011 [details] [diff] [review] Patch v1 [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 796903 User impact if declined: some pages could be broken (exception thrown) Testing completed (on m-c, etc.): on m-c for a bit Risk to taking this patch (and alternatives if risky): pretty low. Alternative is a backout of bug 796903. String or UUID changes made by this patch: none
Comment on attachment 720011 [details] [diff] [review] Patch v1 low risk uplift for a Fx19 regression.Approving for aurora
Attachment #720011 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 720011 [details] [diff] [review] Patch v1 This has been in product for a while and it's now too late for the fourth week beta which already went to build so let's leave it at Aurora and ship this fix first in FF21.
Attachment #720011 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
You need to log in before you can comment on or make changes to this bug.