DOMImplementation.createDocument() should have [TreatNullAs=EmptyString] for second arg

RESOLVED FIXED in Firefox 21

Status

()

defect
--
minor
RESOLVED FIXED
7 years ago
3 months ago

People

(Reporter: ayg, Assigned: Ms2ger)

Tracking

(Blocks 1 bug, {regression})

Trunk
mozilla22
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox19- wontfix, firefox20- wontfix, firefox21- fixed, firefox22- fixed)

Details

Attachments

(1 attachment)

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

Comment 2

6 years ago
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.

Updated

6 years ago
Ms2ger, could you take a look at this?
Assignee

Updated

6 years ago
Assignee: ayg → Ms2ger
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.
Flags: needinfo?(Ms2ger)
Assignee

Comment 6

6 years ago
Posted patch Patch v1Splinter Review
Attachment #720011 - Flags: review?(bzbarsky)
Flags: needinfo?(Ms2ger)
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+
Assignee

Comment 8

6 years ago
(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.
https://hg.mozilla.org/mozilla-central/rev/50da21920e0c
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
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
Attachment #720011 - Flags: approval-mozilla-beta?
Attachment #720011 - Flags: approval-mozilla-aurora?
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-
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.