-
Notifications
You must be signed in to change notification settings - Fork 300
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.
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
createElementNS() QName/Name confusion #671
Comments
It appears that #319 and cscott/web-platform-tests@85470b4 recently (Mar 16 2017) changed some some of the
That is, remove the reference to |
@cscott You state:
I believe this example is an ambiguous case that could also be taken to mean: it matched the The specs for So, I agree the specs could use some further clarification, or perhaps reevaluation as what should be allowed and what not. |
Although "non-namespaced" of course implies the default namespace. But that basically translates to calling |
Perhaps the documentation could be updated to include something like:
|
…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.
…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.
Well, as long as the "validate the qualified name" step only allows a QName, then we don't have to worry about empty strings being the prefix, since QName requires at least one character before the colon. |
If Name is indeed truly a subset of QName it does seem 61f40b0 should be further simplified to remove the mention of Name. Justification and PRs to that effect welcome. |
But
As an aside: This is the outcome of some tests in Firefox 61:
So, to conclude: |
What I meant is that if you check against QName is there potential for additional failures to be detected if you check against Name afterwards? |
Ah right, of course. In that case: no, I don't believe there is. |
This addresses two cases overlooked by 61f40b0: * The QName production is a strict subset of the Name production. To avoid confusion, drop "or Name" from the definition of "validate a qualifiedName". * The non-normative "for web developers" text for `createElementNS` was not updated by the original commit. Fixes: whatwg#671
PR submitted as #675. |
Issue #671 is marked with the "good first issue" label. I disagree. I am a git beginner and cannot understand why the pull request #675 was not granted. This seems a small fix in the DOM Standard, and hardly important since XML Namespaces will be deprecated soon if they are not already deprecated. I will try to change the labels, if I have permission. |
My reading here is that we should be throwing InvalidCharacterError on a non-QName Name, and the issue remains open only because of a question about rights assignment on the spec change PR. Is that correct? |
Yes, that is my understanding. |
Just my personal opinion, but I see many projects like this on GitHub: they have been discussed and a change is ready to be released, but it just doesn't happen, in this case for a year, but frequently for many years. In some cases it is because the single original developer has abandoned the project. I'm not sure of the entire range of reasons. As a retired software engineer, I would like to participate in Open Software development, but am completely turned off to the idea because, with the exception of currently popular and very active projects, changes proposed for projects don't actually get made. In addition, I've taken several courses in git and GitHub and tried to make changes using pull requests (both locally on cloned repositories and on GitHub), but these have failed with error messages I cannot understand. I find the git process to be arcane, meaning too difficult to use. Open Software is a great idea: letting skilled people contribute their time for free to improve shared products. But the main tool supporting Open Software seems impossible for an experienced retired software engineer to learn. There is a disconnect here somewhere. There isn't even an appropriate forum (that is, meta-concerned about Open Software and its tools) to post this complaint on. |
I have no idea whether that is appropriate in this case. I do hope it moves this stalled issue forward, of course, and I thank annevk. In any case, I stand by my comments of Feb 24, 2020: there is no good place to complain about the broken GitHub model for open software development. |
The spec is not clear about the valid arguments to
Document#createElementNS()
. It states:But this doesn't explicitly clarify what happens when the
qualifiedName
matchesName
but notQName
, for instance the stringfoo:bar:bat
. SinceQName
is defined in term ofName
anyway, I believe this would be more clearly stated as just...if qualifiedName does not match the QName production
(eliminating the reference toName
).However, this would change the errors thrown in some corner cases. According to the test suite, calling
document.createElementNS(null, ":foo")
ought to throw aNAMESPACE_ERR
(line 24), but:foo
doesn't matchQName
(prefix must be at least one character in the production), so it ought to throw anINVALID_CHARACTER_ERROR
. (This assumes thatprefix
will be set to the empty string on line 5 which will trigger the exception on line 6 because the empty string is "non-null".)On the other hand,
document.createElementNS("http://example.com", ":foo")
ought to throw aNAMESPACE_ERR
(test case ine 56) yet there is no spec language to justify this: if it failed the "validate qualifiedName" test it ought to throw a INVALID_CHARACTER_ERROR, and steps 5 and 6 state:At this point the namespace is non-null and the prefix is the empty string. There is no language triggering a NamespaceError.
See also web-platform-tests/wpt#12202 which covers some other issues with the test suite for
createElementNS()
, so fixing the test suite might be reasonable.For best compliance with the existing test suite, the "validate a qualifiedName" test could be written as:
That would be my recommended fix.
The text was updated successfully, but these errors were encountered: