Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
ayg opened this issue Aug 30, 2016 · 37 comments
Closed

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

ayg opened this issue Aug 30, 2016 · 37 comments
Assignees
@annevk

Comments

@ayg
Copy link
Contributor

ayg commented Aug 30, 2016 

<!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.
@ayg
Copy link
Contributor Author

ayg commented Aug 30, 2016

(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.)

@annevk
Copy link
Member

annevk commented Aug 30, 2016

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

@annevk
Copy link
Member

annevk commented Aug 30, 2016

We could remove the Name check, right?

@ayg
Copy link
Contributor Author

ayg commented Aug 30, 2016

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

@annevk
Copy link
Member

annevk commented Aug 30, 2016

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

@ayg
Copy link
Contributor Author

ayg commented Aug 30, 2016 •
edited
Loading

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.

@ayg
Copy link
Contributor Author

ayg commented Aug 30, 2016

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.

@annevk
Copy link
Member

annevk commented Oct 11, 2016

Do we have tests for this already?

@ayg
Copy link
Contributor Author

ayg commented Oct 25, 2016

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.

@ayg
Copy link
Contributor Author

ayg commented Oct 27, 2016

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

@ayg
Copy link
Contributor Author

ayg commented Oct 30, 2016

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

@bzbarsky
Copy link

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.

@annevk
Copy link
Member

annevk commented Oct 31, 2016

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

@ayg
Copy link
Contributor Author

ayg commented Oct 31, 2016

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?)

@annevk
Copy link
Member

annevk commented Oct 31, 2016

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.

@bzbarsky
Copy link

I don't know what other meaning you would give to InvalidCharacterError vs. NamespaceError here

I think in practice the distinction is completely irrelevant here.

@ayg
Copy link
Contributor Author

ayg commented Oct 31, 2016

I completely agree, so should we just go with InvalidCharacterError consistently?

@annevk
Copy link
Member

annevk commented Nov 22, 2016

Just to be clear, that is my preference.

@annevk
Copy link
Member

annevk commented Mar 16, 2017

@domenic are you still planning on trying to make name validation match the HTML parser? If so, we should maybe hold off on trying to clean this up until that either succeeds or another year has passed.

@domenic
Copy link
Member

domenic commented Mar 16, 2017

@dominiccooney and I are planning on doing so, but I still think converging on InvalidCharacterError and fixing the tests would be a good intermediate step. Otherwise Blink at least is likely to end up first changing to NamespaceError then later changing to no error. Just this morning @tkent-google filed https://bugs.chromium.org/p/chromium/issues/detail?id=702077 .

@annevk annevk self-assigned this Mar 16, 2017
annevk added a commit to web-platform-tests/wpt that referenced this issue Mar 16, 2017
@annevk
DOM: use a single exception for name validation
94dd371 
See whatwg/dom#319.
@annevk annevk mentioned this issue Mar 16, 2017
Merged
annevk added a commit that referenced this issue Mar 16, 2017
@annevk
Use a single exception for name validation
24569d8 
Tests: web-platform-tests/wpt#5161.

Fixes #319.
@annevk annevk mentioned this issue Mar 16, 2017
Merged
@annevk
Copy link
Member

annevk commented Mar 16, 2017

Posted a web-platform-tests PR and a standard PR.

@tkent-google
Copy link
Collaborator

Chrome's implementation validates input string in a single scan, and it seems an error of the first character validation for LocalPart is handled as InvalidCharacterError incorrectly. It's easy to fix.

I'm fine with either of simplifying the specification or keeping the current one.

@annevk
Copy link
Member

annevk commented Mar 23, 2017

@cdumez opinions?

@ayg
Copy link
Contributor Author

ayg commented Apr 19, 2017

@annevk I think people seem to be okay with InvalidCharacterError here. Do you think it's time to change the spec?

@annevk
Copy link
Member

annevk commented Apr 19, 2017

I wish we had heard from @cdumez. Did you review the patches to the specification and tests @ayg? If @tkent-google is still on board I guess I'm okay with simplifying this given that @ayg changes Firefox.

@cdumez
Copy link

cdumez commented Apr 19, 2017

Sorry, missed this. Same opinion as @tkent-google here.

@ayg
Copy link
Contributor Author

ayg commented Apr 20, 2017

LGTM. I'm happy to write the patch for Firefox to use only InvalidCharacterError if that's what we want.

annevk added a commit that referenced this issue Apr 20, 2017
@annevk
Use a single exception for name validation
61f40b0 
Tests: web-platform-tests/wpt#5161.

Fixes #319.
annevk added a commit to web-platform-tests/wpt that referenced this issue Apr 20, 2017
@annevk
DOM: use a single exception for name validation
85470b4 
See whatwg/dom#319.
@annevk
Copy link
Member

annevk commented Apr 20, 2017

Alright, let's run with it.

@ayg
Copy link
Contributor Author

ayg commented Apr 20, 2017

https://bugzilla.mozilla.org/show_bug.cgi?id=1358104

Someone want to make sure there are bugs filed on other UAs?

@annevk
Copy link
Member

annevk commented Apr 20, 2017

Thanks for the reminder.

https://bugs.webkit.org/show_bug.cgi?id=171052
https://bugs.chromium.org/p/chromium/issues/detail?id=713681
https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/11730496/

@cdumez
Copy link

cdumez commented Apr 22, 2017 •
edited
Loading

FYI, making this change seems to cause 2 acid3 test failures :/

Failed 2 tests.
Test 23 failed: expected '14' but got '5' - wrong exception for createElementNS('null', ':div')
Test 25 failed: wrong exception

@annevk
Copy link
Member

annevk commented Apr 23, 2017

Sigh. Maybe we should just give up on Acid3?

@ayg
Copy link
Contributor Author

ayg commented Apr 23, 2017

Can't we update Acid3?

@bzbarsky
Copy link

@ayg "we" == "hixie". If you want to try to convince him, go ahead. So far we haven't managed for the CSS spec change that likewise caused an Acid3 test failure.

@ayg
Copy link
Contributor Author

ayg commented Apr 23, 2017

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.

@bzbarsky
Copy link

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...

@bzbarsky
Copy link

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.

hubot pushed a commit to WebKit/WebKit-http that referenced this issue Apr 25, 2017
createElementNS() should now throw only InvalidCharacterError, not Na…
371f979 
…mespaceError

https://bugs.webkit.org/show_bug.cgi?id=171052

Reviewed by Sam Weinig.

LayoutTests/imported/w3c:

Re-sync web-platform-tests/dom from upstream to gain test coverage.

* web-platform-tests/dom/collections/HTMLCollection-as-proto-length-get-throws.html:
* web-platform-tests/dom/collections/HTMLCollection-empty-name.html:
* web-platform-tests/dom/collections/HTMLCollection-supported-property-indices.html:
* web-platform-tests/dom/collections/HTMLCollection-supported-property-names.html:
* web-platform-tests/dom/collections/domstringmap-supported-property-names.html:
* web-platform-tests/dom/collections/namednodemap-supported-property-names.html:
* web-platform-tests/dom/events/AddEventListenerOptions-once.html:
* web-platform-tests/dom/events/AddEventListenerOptions-passive.html:
* web-platform-tests/dom/events/CustomEvent.html:
* web-platform-tests/dom/events/Event-cancelBubble.html:
* web-platform-tests/dom/events/Event-constants.html:
* web-platform-tests/dom/events/Event-constructors.html:
* web-platform-tests/dom/events/Event-defaultPrevented-after-dispatch.html:
* web-platform-tests/dom/events/Event-defaultPrevented.html:
* web-platform-tests/dom/events/Event-dispatch-bubble-canceled.html:
* web-platform-tests/dom/events/Event-dispatch-bubbles-false.html:
* web-platform-tests/dom/events/Event-dispatch-bubbles-true.html:
* web-platform-tests/dom/events/Event-dispatch-click.html:
* web-platform-tests/dom/events/Event-dispatch-detached-click.html:
* web-platform-tests/dom/events/Event-dispatch-handlers-changed.html:
* web-platform-tests/dom/events/Event-dispatch-multiple-cancelBubble.html:
* web-platform-tests/dom/events/Event-dispatch-multiple-stopPropagation.html:
* web-platform-tests/dom/events/Event-dispatch-omitted-capture.html:
* web-platform-tests/dom/events/Event-dispatch-order.html:
* web-platform-tests/dom/events/Event-dispatch-other-document.html:
* web-platform-tests/dom/events/Event-dispatch-propagation-stopped.html:
* web-platform-tests/dom/events/Event-dispatch-redispatch.html:
* web-platform-tests/dom/events/Event-dispatch-reenter.html:
* web-platform-tests/dom/events/Event-dispatch-target-moved.html:
* web-platform-tests/dom/events/Event-dispatch-target-removed.html:
* web-platform-tests/dom/events/Event-dispatch-throwing.html:
* web-platform-tests/dom/events/Event-init-while-dispatching.html:
* web-platform-tests/dom/events/Event-initEvent.html:
* web-platform-tests/dom/events/Event-propagation.html:
* web-platform-tests/dom/events/Event-subclasses-constructors.html:
* web-platform-tests/dom/events/Event-type-empty.html:
* web-platform-tests/dom/events/Event-type.html:
* web-platform-tests/dom/events/EventListener-handleEvent.html:
* web-platform-tests/dom/events/EventListener-incumbent-global-1.sub.html:
* web-platform-tests/dom/events/EventListener-incumbent-global-2.sub.html:
* web-platform-tests/dom/events/EventListener-invoke-legacy.html:
* web-platform-tests/dom/events/EventListenerOptions-capture.html:
* web-platform-tests/dom/events/EventTarget-addEventListener.html:
* web-platform-tests/dom/events/EventTarget-dispatchEvent-returnvalue.html:
* web-platform-tests/dom/events/EventTarget-dispatchEvent.html:
* web-platform-tests/dom/events/EventTarget-removeEventListener.html:
* web-platform-tests/dom/historical.html:
* web-platform-tests/dom/interface-objects.html:
* web-platform-tests/dom/interfaces.html:
* web-platform-tests/dom/lists/DOMTokenList-Iterable.html:
* web-platform-tests/dom/lists/DOMTokenList-coverage-for-attributes.html:
* web-platform-tests/dom/lists/DOMTokenList-iteration.html:
* web-platform-tests/dom/lists/DOMTokenList-stringifier.html:
* web-platform-tests/dom/lists/DOMTokenList-value.html:
* web-platform-tests/dom/nodes/CharacterData-appendChild.html:
* web-platform-tests/dom/nodes/CharacterData-appendData.html:
* web-platform-tests/dom/nodes/CharacterData-data.html:
* web-platform-tests/dom/nodes/CharacterData-deleteData.html:
* web-platform-tests/dom/nodes/CharacterData-insertData.html:
* web-platform-tests/dom/nodes/CharacterData-remove.html:
* web-platform-tests/dom/nodes/CharacterData-replaceData.html:
* web-platform-tests/dom/nodes/CharacterData-substringData.html:
* web-platform-tests/dom/nodes/CharacterData-surrogates.html:
* web-platform-tests/dom/nodes/ChildNode-after.html:
* web-platform-tests/dom/nodes/ChildNode-before.html:
* web-platform-tests/dom/nodes/ChildNode-replaceWith.html:
* web-platform-tests/dom/nodes/Comment-constructor.html:
* web-platform-tests/dom/nodes/DOMImplementation-createDocument-expected.txt:
* web-platform-tests/dom/nodes/DOMImplementation-createDocument.html:
* web-platform-tests/dom/nodes/DOMImplementation-createDocumentType-expected.txt:
* web-platform-tests/dom/nodes/DOMImplementation-createDocumentType.html:
* web-platform-tests/dom/nodes/DOMImplementation-createHTMLDocument.html:
* web-platform-tests/dom/nodes/DOMImplementation-hasFeature.html:
* web-platform-tests/dom/nodes/Document-URL.sub.html:
* web-platform-tests/dom/nodes/Document-adoptNode.html:
* web-platform-tests/dom/nodes/Document-characterSet-normalization.html:
* web-platform-tests/dom/nodes/Document-constructor.html:
* web-platform-tests/dom/nodes/Document-contentType/contentType/contenttype_bmp.html:
* web-platform-tests/dom/nodes/Document-contentType/contentType/contenttype_css.html:
* web-platform-tests/dom/nodes/Document-contentType/contentType/contenttype_datauri_01.html:
* web-platform-tests/dom/nodes/Document-contentType/contentType/contenttype_datauri_02.html:
* web-platform-tests/dom/nodes/Document-contentType/contentType/contenttype_gif.html:
* web-platform-tests/dom/nodes/Document-contentType/contentType/contenttype_html.html:
* web-platform-tests/dom/nodes/Document-contentType/contentType/contenttype_javascripturi.html:
* web-platform-tests/dom/nodes/Document-contentType/contentType/contenttype_jpg.html:
* web-platform-tests/dom/nodes/Document-contentType/contentType/contenttype_mimeheader_01.html:
* web-platform-tests/dom/nodes/Document-contentType/contentType/contenttype_mimeheader_02.html:
* web-platform-tests/dom/nodes/Document-contentType/contentType/contenttype_png.html:
* web-platform-tests/dom/nodes/Document-contentType/contentType/contenttype_txt.html:
* web-platform-tests/dom/nodes/Document-contentType/contentType/contenttype_xml.html:
* web-platform-tests/dom/nodes/Document-contentType/contentType/createDocument.html:
* web-platform-tests/dom/nodes/Document-contentType/contentType/createHTMLDocument.html:
* web-platform-tests/dom/nodes/Document-contentType/contentType/xhr_responseType_document.html:
* web-platform-tests/dom/nodes/Document-createAttribute.html:
* web-platform-tests/dom/nodes/Document-createComment.html:
* web-platform-tests/dom/nodes/Document-createElement-namespace-tests/w3c-import.log:
* web-platform-tests/dom/nodes/Document-createElement-namespace.html:
* web-platform-tests/dom/nodes/Document-createElement.html:
* web-platform-tests/dom/nodes/Document-createElementNS-expected.txt:
* web-platform-tests/dom/nodes/Document-createElementNS.html:
* web-platform-tests/dom/nodes/Document-createElementNS.js:
* web-platform-tests/dom/nodes/Document-createEvent.html:
* web-platform-tests/dom/nodes/Document-createProcessingInstruction-xhtml.xhtml:
* web-platform-tests/dom/nodes/Document-createProcessingInstruction.html:
* web-platform-tests/dom/nodes/Document-createTextNode.html:
* web-platform-tests/dom/nodes/Document-createTreeWalker.html:
* web-platform-tests/dom/nodes/Document-doctype.html:
* web-platform-tests/dom/nodes/Document-getElementById.html:
* web-platform-tests/dom/nodes/Document-getElementsByTagName-xhtml.xhtml:
* web-platform-tests/dom/nodes/Document-getElementsByTagName.html:
* web-platform-tests/dom/nodes/Document-getElementsByTagNameNS.html:
* web-platform-tests/dom/nodes/Document-implementation.html:
* web-platform-tests/dom/nodes/Document-importNode.html:
* web-platform-tests/dom/nodes/DocumentType-literal-xhtml.xhtml:
* web-platform-tests/dom/nodes/DocumentType-literal.html:
* web-platform-tests/dom/nodes/DocumentType-remove.html:
* web-platform-tests/dom/nodes/Element-childElement-null-xhtml.xhtml:
* web-platform-tests/dom/nodes/Element-childElement-null.html:
* web-platform-tests/dom/nodes/Element-childElementCount-dynamic-add-xhtml.xhtml:
* web-platform-tests/dom/nodes/Element-childElementCount-dynamic-add.html:
* web-platform-tests/dom/nodes/Element-childElementCount-dynamic-remove-xhtml.xhtml:
* web-platform-tests/dom/nodes/Element-childElementCount-dynamic-remove.html:
* web-platform-tests/dom/nodes/Element-childElementCount-nochild-xhtml.xhtml:
* web-platform-tests/dom/nodes/Element-childElementCount-nochild.html:
* web-platform-tests/dom/nodes/Element-childElementCount-xhtml.xhtml:
* web-platform-tests/dom/nodes/Element-childElementCount.html:
* web-platform-tests/dom/nodes/Element-children.html:
* web-platform-tests/dom/nodes/Element-classlist.html:
* web-platform-tests/dom/nodes/Element-closest.html:
* web-platform-tests/dom/nodes/Element-firstElementChild-entity-xhtml.xhtml:
* web-platform-tests/dom/nodes/Element-firstElementChild-namespace-xhtml.xhtml:
* web-platform-tests/dom/nodes/Element-firstElementChild-namespace.html:
* web-platform-tests/dom/nodes/Element-firstElementChild-xhtml.xhtml:
* web-platform-tests/dom/nodes/Element-firstElementChild.html:
* web-platform-tests/dom/nodes/Element-getElementsByClassName.html:
* web-platform-tests/dom/nodes/Element-getElementsByTagName-change-document-HTMLNess.html:
* web-platform-tests/dom/nodes/Element-getElementsByTagName.html:
* web-platform-tests/dom/nodes/Element-getElementsByTagNameNS.html:
* web-platform-tests/dom/nodes/Element-hasAttributes.html:
* web-platform-tests/dom/nodes/Element-insertAdjacentElement.html:
* web-platform-tests/dom/nodes/Element-insertAdjacentText.html:
* web-platform-tests/dom/nodes/Element-lastElementChild-xhtml.xhtml:
* web-platform-tests/dom/nodes/Element-lastElementChild.html:
* web-platform-tests/dom/nodes/Element-matches.html:
* web-platform-tests/dom/nodes/Element-nextElementSibling-xhtml.xhtml:
* web-platform-tests/dom/nodes/Element-nextElementSibling.html:
* web-platform-tests/dom/nodes/Element-previousElementSibling-xhtml.xhtml:
* web-platform-tests/dom/nodes/Element-previousElementSibling.html:
* web-platform-tests/dom/nodes/Element-remove.html:
* web-platform-tests/dom/nodes/Element-removeAttributeNS.html:
* web-platform-tests/dom/nodes/Element-siblingElement-null-xhtml.xhtml:
* web-platform-tests/dom/nodes/Element-siblingElement-null.html:
* web-platform-tests/dom/nodes/Element-tagName.html:
* web-platform-tests/dom/nodes/Element-webkitMatchesSelector.html:
* web-platform-tests/dom/nodes/MutationObserver-attributes.html:
* web-platform-tests/dom/nodes/MutationObserver-characterData.html:
* web-platform-tests/dom/nodes/MutationObserver-childList.html:
* web-platform-tests/dom/nodes/MutationObserver-disconnect.html:
* web-platform-tests/dom/nodes/MutationObserver-document.html:
* web-platform-tests/dom/nodes/MutationObserver-inner-outer.html:
* web-platform-tests/dom/nodes/MutationObserver-takeRecords.html:
* web-platform-tests/dom/nodes/Node-appendChild.html:
* web-platform-tests/dom/nodes/Node-baseURI.html:
* web-platform-tests/dom/nodes/Node-childNodes.html:
* web-platform-tests/dom/nodes/Node-cloneNode.html:
* web-platform-tests/dom/nodes/Node-compareDocumentPosition.html:
* web-platform-tests/dom/nodes/Node-constants.html:
* web-platform-tests/dom/nodes/Node-contains-xml.xml:
* web-platform-tests/dom/nodes/Node-contains.html:
* web-platform-tests/dom/nodes/Node-insertBefore.html:
* web-platform-tests/dom/nodes/Node-isConnected.html:
* web-platform-tests/dom/nodes/Node-isEqualNode-xhtml.xhtml:
* web-platform-tests/dom/nodes/Node-isEqualNode.html:
* web-platform-tests/dom/nodes/Node-isSameNode.html:
* web-platform-tests/dom/nodes/Node-lookupNamespaceURI.html:
* web-platform-tests/dom/nodes/Node-lookupPrefix.xhtml:
* web-platform-tests/dom/nodes/Node-nodeName-xhtml.xhtml:
* web-platform-tests/dom/nodes/Node-nodeName.html:
* web-platform-tests/dom/nodes/Node-nodeValue.html:
* web-platform-tests/dom/nodes/Node-normalize.html:
* web-platform-tests/dom/nodes/Node-parentElement.html:
* web-platform-tests/dom/nodes/Node-parentNode.html:
* web-platform-tests/dom/nodes/Node-properties.html:
* web-platform-tests/dom/nodes/Node-removeChild.html:
* web-platform-tests/dom/nodes/Node-replaceChild.html:
* web-platform-tests/dom/nodes/Node-textContent.html:
* web-platform-tests/dom/nodes/NodeList-Iterable.html:
* web-platform-tests/dom/nodes/ParentNode-append.html:
* web-platform-tests/dom/nodes/ParentNode-prepend.html:
* web-platform-tests/dom/nodes/ParentNode-querySelector-All-xht.xht:
* web-platform-tests/dom/nodes/ParentNode-querySelector-All.html:
* web-platform-tests/dom/nodes/ProcessingInstruction-escapes-1.xhtml:
* web-platform-tests/dom/nodes/ProcessingInstruction-literal-1.xhtml:
* web-platform-tests/dom/nodes/ProcessingInstruction-literal-2.xhtml:
* web-platform-tests/dom/nodes/Text-constructor.html:
* web-platform-tests/dom/nodes/Text-splitText.html:
* web-platform-tests/dom/nodes/append-on-Document.html:
* web-platform-tests/dom/nodes/attributes-expected.txt:
* web-platform-tests/dom/nodes/attributes.html:
* web-platform-tests/dom/nodes/case.html:
* web-platform-tests/dom/nodes/getElementsByClassName-01.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-02.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-03.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-04.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-05.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-06.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-07.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-08.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-09.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-10.xml:
* web-platform-tests/dom/nodes/getElementsByClassName-11.xml:
* web-platform-tests/dom/nodes/getElementsByClassName-12.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-13.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-14.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-15.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-16.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-17.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-18.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-19.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-20.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-21.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-22.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-23.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-24.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-25.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-26.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-27.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-28.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-29.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-30.htm:
* web-platform-tests/dom/nodes/getElementsByClassName-31.htm:
* web-platform-tests/dom/nodes/insert-adjacent.html:
* web-platform-tests/dom/nodes/prepend-on-Document.html:
* web-platform-tests/dom/nodes/remove-unscopable.html:
* web-platform-tests/dom/nodes/rootNode.html:
* web-platform-tests/dom/nodes/w3c-import.log:
* web-platform-tests/dom/ranges/Range-attributes.html:
* web-platform-tests/dom/ranges/Range-cloneContents.html:
* web-platform-tests/dom/ranges/Range-cloneRange.html:
* web-platform-tests/dom/ranges/Range-collapse.html:
* web-platform-tests/dom/ranges/Range-commonAncestorContainer-2.html:
* web-platform-tests/dom/ranges/Range-commonAncestorContainer.html:
* web-platform-tests/dom/ranges/Range-compareBoundaryPoints.html:
* web-platform-tests/dom/ranges/Range-comparePoint-2.html:
* web-platform-tests/dom/ranges/Range-comparePoint.html:
* web-platform-tests/dom/ranges/Range-constructor.html:
* web-platform-tests/dom/ranges/Range-deleteContents.html:
* web-platform-tests/dom/ranges/Range-detach.html:
* web-platform-tests/dom/ranges/Range-extractContents.html:
* web-platform-tests/dom/ranges/Range-insertNode.html:
* web-platform-tests/dom/ranges/Range-intersectsNode-binding.html:
* web-platform-tests/dom/ranges/Range-intersectsNode.html:
* web-platform-tests/dom/ranges/Range-isPointInRange.html:
* web-platform-tests/dom/ranges/Range-mutations-appendChild.html:
* web-platform-tests/dom/ranges/Range-mutations-appendData.html:
* web-platform-tests/dom/ranges/Range-mutations-dataChange.html:
* web-platform-tests/dom/ranges/Range-mutations-deleteData.html:
* web-platform-tests/dom/ranges/Range-mutations-insertBefore.html:
* web-platform-tests/dom/ranges/Range-mutations-insertData.html:
* web-platform-tests/dom/ranges/Range-mutations-removeChild.html:
* web-platform-tests/dom/ranges/Range-mutations-replaceChild.html:
* web-platform-tests/dom/ranges/Range-mutations-replaceData.html:
* web-platform-tests/dom/ranges/Range-mutations-splitText.html:
* web-platform-tests/dom/ranges/Range-selectNode.html:
* web-platform-tests/dom/ranges/Range-set.html:
* web-platform-tests/dom/ranges/Range-stringifier.html:
* web-platform-tests/dom/ranges/Range-surroundContents.html:
* web-platform-tests/dom/traversal/NodeFilter-constants.html:
* web-platform-tests/dom/traversal/NodeIterator-removal.html:
* web-platform-tests/dom/traversal/NodeIterator.html:
* web-platform-tests/dom/traversal/TreeWalker-acceptNode-filter.html:
* web-platform-tests/dom/traversal/TreeWalker-basic.html:
* web-platform-tests/dom/traversal/TreeWalker-currentNode.html:
* web-platform-tests/dom/traversal/TreeWalker-previousNodeLastChildReject.html:
* web-platform-tests/dom/traversal/TreeWalker-previousSiblingLastChildSkip.html:
* web-platform-tests/dom/traversal/TreeWalker-traversal-reject.html:
* web-platform-tests/dom/traversal/TreeWalker-traversal-skip-most.html:
* web-platform-tests/dom/traversal/TreeWalker-traversal-skip.html:
* web-platform-tests/dom/traversal/TreeWalker-walking-outside-a-tree.html:
* web-platform-tests/dom/traversal/TreeWalker.html:

Source/WebCore:

Validating a qualified name should only throw InvalidCharacterError, not NamespaceError,
after:
- whatwg/dom#319
- web-platform-tests/wpt#5161
- whatwg/dom#423

Latest spec:
- https://dom.spec.whatwg.org/#validate

No new tests, updated web-platform-tests.

* dom/Document.cpp:
(WebCore::Document::parseQualifiedName):

LayoutTests:

Updated / rebaselined existing tests to reflect behavior change.

* fast/dom/DOMImplementation/createDocument-namespace-err-expected.txt:
* fast/dom/DOMImplementation/createDocumentType-err-expected.txt:
* fast/dom/DOMImplementation/script-tests/createDocument-namespace-err.js:
* fast/dom/DOMImplementation/script-tests/createDocumentType-err.js:
* fast/dom/Document/createAttributeNS-namespace-err-expected.txt:
* fast/dom/Document/createElementNS-namespace-err-expected.txt:
* fast/dom/Document/script-tests/createAttributeNS-namespace-err.js:
* fast/dom/Document/script-tests/createElementNS-namespace-err.js:
* fast/dom/Element/script-tests/setAttributeNS-namespace-err.js:
* fast/dom/Element/setAttributeNS-namespace-err-expected.txt:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@215701 268f45cc-cd09-0410-ab3c-d52691b4dbfc
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Apr 30, 2017
@ayg
Bug 1358104 - Only throw InvalidCharacterError for bad XML id's, not …
527fb3f 
…NamespaceError r=baku

Discussion at <whatwg/dom#319>.  In short, the
specification used to say to throw sometimes InvalidCharacterError and
sometimes NamespaceError, but browsers disagreed on which to throw in
corner cases, and everyone agreed it wasn't worth the effort to spec the
distinction, so we just changed it to InvalidCharacterError across the
board.

The test changes are already upstream.

MozReview-Commit-ID: AWSZBznQprG

--HG--
extra : rebase_source : 2f0051f48124380f17300a38ceb8c2ab23015ca1
JerryShih pushed a commit to JerryShih/gecko-dev that referenced this issue May 2, 2017
@ayg
Bug 1358104 - Only throw InvalidCharacterError for bad XML id's, not …
f803e1d 
…NamespaceError r=baku

Discussion at <whatwg/dom#319>.  In short, the
specification used to say to throw sometimes InvalidCharacterError and
sometimes NamespaceError, but browsers disagreed on which to throw in
corner cases, and everyone agreed it wasn't worth the effort to spec the
distinction, so we just changed it to InvalidCharacterError across the
board.

The test changes are already upstream.

MozReview-Commit-ID: AWSZBznQprG
jryans pushed a commit to jryans/gecko-dev that referenced this issue May 3, 2017
@ayg
Bug 1358104 - Only throw InvalidCharacterError for bad XML id's, not …
07881fa 
…NamespaceError r=baku

Discussion at <whatwg/dom#319>.  In short, the
specification used to say to throw sometimes InvalidCharacterError and
sometimes NamespaceError, but browsers disagreed on which to throw in
corner cases, and everyone agreed it wasn't worth the effort to spec the
distinction, so we just changed it to InvalidCharacterError across the
board.

The test changes are already upstream.

MozReview-Commit-ID: AWSZBznQprG
gsnedders added a commit to gsnedders/web-platform-tests that referenced this issue Nov 24, 2017
@gsnedders
Update Acid3 for DOM exception simplification
e43a66d 
See http://www.w3.org/mid/CADnb78js2FtmdMBGtOEo=i+YMNa7OzWYiJc96qTM2YpQvG99MQ@mail.gmail.com

Change comes from whatwg/dom#319
gsnedders added a commit to web-platform-tests/wpt that referenced this issue Nov 24, 2017
@gsnedders
Update Acid3 for DOM exception simplification
3cbdbaa 
See http://www.w3.org/mid/CADnb78js2FtmdMBGtOEo=i+YMNa7OzWYiJc96qTM2YpQvG99MQ@mail.gmail.com

Change comes from whatwg/dom#319
@cscott cscott mentioned this issue Jul 27, 2018
Closed
cscott added a commit to fgnass/domino that referenced this issue Jul 27, 2018
@cscott
Spec-correctness for Document#createElement() and `Document#createE…
88419de 
…lementNS()`

The spec (and its test suite) is itself a bit confused; see
web-platform-tests/wpt#12202 and
whatwg/dom#671 for more details.

Used proper exception type (InvalidCharacterError in more cases, after
whatwg/dom#319) and ensured that we did the
proper WebIDL type coercion thing if given null/undefined as arguments.
cscott added a commit to fgnass/domino that referenced this issue Jul 27, 2018
@cscott
Spec-correctness for Document#createElement() and `Document#createE…
9d3d86a 
…lementNS()`

The spec (and its test suite) is itself a bit confused; see
web-platform-tests/wpt#12202 and
whatwg/dom#671 for more details.

Used proper exception type (InvalidCharacterError in more cases, after
whatwg/dom#319) and ensured that we did the
proper WebIDL type coercion thing if given null/undefined as arguments.
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 1, 2019
@marco-c
Bug 1358104 - Only throw InvalidCharacterError for bad XML id's, not …
fb0a680 
…NamespaceError r=baku

Discussion at <whatwg/dom#319>.  In short, the
specification used to say to throw sometimes InvalidCharacterError and
sometimes NamespaceError, but browsers disagreed on which to throw in
corner cases, and everyone agreed it wasn't worth the effort to spec the
distinction, so we just changed it to InvalidCharacterError across the
board.

The test changes are already upstream.

MozReview-Commit-ID: AWSZBznQprG

UltraBlame original commit: b57cea7362656856b373828a932ceb3d8a84400f
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 1, 2019
@marco-c
Bug 1358104 - Only throw InvalidCharacterError for bad XML id's, not …
da44202 
…NamespaceError r=baku

Discussion at <whatwg/dom#319>.  In short, the
specification used to say to throw sometimes InvalidCharacterError and
sometimes NamespaceError, but browsers disagreed on which to throw in
corner cases, and everyone agreed it wasn't worth the effort to spec the
distinction, so we just changed it to InvalidCharacterError across the
board.

The test changes are already upstream.

MozReview-Commit-ID: AWSZBznQprG

UltraBlame original commit: b57cea7362656856b373828a932ceb3d8a84400f
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 1, 2019
@marco-c
Bug 1358104 - Only throw InvalidCharacterError for bad XML id's, not …
dde41d3 
…NamespaceError r=baku

Discussion at <whatwg/dom#319>.  In short, the
specification used to say to throw sometimes InvalidCharacterError and
sometimes NamespaceError, but browsers disagreed on which to throw in
corner cases, and everyone agreed it wasn't worth the effort to spec the
distinction, so we just changed it to InvalidCharacterError across the
board.

The test changes are already upstream.

MozReview-Commit-ID: AWSZBznQprG

UltraBlame original commit: b57cea7362656856b373828a932ceb3d8a84400f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@annevk annevk

Labels
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@domenic @ayg @cdumez @bzbarsky @annevk @tkent-google