Validating a qualified name does not match browsers in a corner case

<!DOCTYPE html>
<script>
try { document.createElementNS("a", "a:0") }
catch(e) { document.documentElement.textContent = e.name }
</script>

Outputs InvalidCharacterError in Chrome, Firefox, and Edge. Per spec, it should throw NamespaceError, because "a:0" matches the Name production. I think browsers should probably change here instead of the spec, because browsers' behavior doesn't make sense to me, but I'm filing a spec bug anyway because we seem to have interop against the spec.

(Also if the character after the colon is a combining character, Chrome and Firefox throw InvalidCharacterError for the same reason. Edge doesn't throw in this case, but that's because it doesn't throw in general for anything starting with a combining character, which is just a bug on their part.)

Happy to change the spec if browsers already align. Prolly simplifies a bit too.

We could remove the Name check, right?

Then how would we know whether to throw InvalidCharacterError or NamespaceError?

Oh, I thought you suggested that distinction is not made here.

No, it's just in this one specific case that UAs throw a different exception. If we want to change the spec to match them, we'd have to research exactly what the behavior is here, e.g., a:0:a and a:a:0 and things like that.

Correction: the following test with combining char

<!DOCTYPE html>
<script>
try { document.createElementNS("a", "a:ெ") }
catch(e) { document.documentElement.textContent = e.name }
</script>

gives InvalidCharacterError per Chrome, NamespaceError per Firefox and spec, and no exception in Edge. So if we change the spec, it may have to be only for ASCII characters, which seems even less reasonable.

Do we have tests for this already?

I have tests in dom/nodes/Document-createElementNS.js that I checked in to Gecko but got backed out for a syntax error elsewhere in the push. I'll reland them shortly.

Tests are now in Gecko and should wind up upstream soon: https://hg.mozilla.org/mozilla-central/rev/31177ceb556c

@bzbarsky

@bzbarsky What do you think? Browsers are somewhat interoperable, but it seems like it's just a bug and the spec is right.

So... at least in terms of what Gecko is actually doing, it's validating the qualifiedname in "namespace aware" mode. In that mode, we treat the char right after the ':' (if there is a ':') as a "name start" character, and '0' is not valid "name start" character. In particular, we effectively treat the parts before and after the ':' symmetrically, in terms of treating a digit starting either one as an "invalid character" error. This allows validation of the name in a single scan of it, just resetting some state if a ':' is found.

OK, so back to the spec... The spec requires scanning the name twice, right? This is somewhat easier to spec, but less efficient, of course.

We could try to rewrite our validation algorithm to produce the output the spec expects while staying to a single pass, but that code is a bit fiddly, so this would have nontrivial regression risk in practice.

As far as whether the spec is "right"... it's not obvious to me why you consider it "right". It could be rewritten in terms of a slightly more complicated (in spec terms) but simpler to implement as a single pass validation algorithm, and such an algorithm may naturally treat a localName following a ':' but not matching the NCName production because it doesn't start with NameStartChar as being an invalid char err, not a namespace err, because it's not at all a namespace error, afaict.

I think ideally we just throw InvalidCharacterError here and forget about the distinction. We don't have DOM without namespaces after all.

We could try to rewrite our validation algorithm to produce the output the spec expects while staying to a single pass, but that code is a bit fiddly, so this would have nontrivial regression risk in practice.

I think we have pretty good tests for this.

As far as whether the spec is "right"... it's not obvious to me why you consider it "right". It could be rewritten in terms of a slightly more complicated (in spec terms) but simpler to implement as a single pass validation algorithm, and such an algorithm may naturally treat a localName following a ':' but not matching the NCName production because it doesn't start with NameStartChar as being an invalid char err, not a namespace err, because it's not at all a namespace error, afaict.

It's a namespace error insofar as it is valid in XML without namespaces but invalid in XML with namespaces, so the error is only in namespace-land. I don't know what other meaning you would give to InvalidCharacterError vs. NamespaceError here.

I think ideally we just throw InvalidCharacterError here and forget about the distinction. We don't have DOM without namespaces after all.

That's very logical, but completely doesn't match browsers. Am I too optimistic in hoping that maybe changing the exception type is not going to cause compat issues? If it won't, maybe we can just get everyone to change their exception types. (Did we succeed in getting rid of DOMError?)

We have successfully changed exception types a number of times. It's a risk and some churn each time of course. I believe we did rid ourselves of the DOMError interface successfully.

Because he isn't around, or because he personally disagrees with the change, or what? If the test isn't actively maintained or otherwise doesn't match what the specs say, is there any reason to not start ignoring it? I'd think people aren't judging browsers based on Acid3 conformance anymore.

Because he isn't around, or because he personally disagrees with the change,

Some of both, as far as I can tell.

I'd think people aren't judging browsers based on Acid3 conformance anymore

"people" is a broad generalization. We certainly got bug reports on the CSS thing...

Or put another way, a coordinated statement from all browsers that we plan to ignore Acid3, and why, would be a good idea if we plan to do that.

createElementNS() should now throw only InvalidCharacterError, not Na…

Bug 1358104 - Only throw InvalidCharacterError for bad XML id's, not …

Update Acid3 for DOM exception simplification

Spec-correctness for `Document#createElement()` and `Document#createE…

Bug 1358104 - Only throw InvalidCharacterError for bad XML id's, not …

Navigation Menu

Repository navigation

Validating a qualified name does not match browsers in a corner case #319

25 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Validating a qualified name does not match browsers in a corner case #319

Description

Activity

ayg commented on Aug 30, 2016

annevk commented on Aug 30, 2016

annevk commented on Aug 30, 2016

ayg commented on Aug 30, 2016

annevk commented on Aug 30, 2016

ayg commented on Aug 30, 2016

ayg commented on Aug 30, 2016

annevk commented on Oct 11, 2016

ayg commented on Oct 25, 2016

ayg commented on Oct 27, 2016

ayg commented on Oct 30, 2016

bzbarsky commented on Oct 31, 2016

annevk commented on Oct 31, 2016

ayg commented on Oct 31, 2016

annevk commented on Oct 31, 2016

25 remaining items

ayg commented on Apr 23, 2017

bzbarsky commented on Apr 23, 2017

bzbarsky commented on Apr 23, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions