-
Notifications
You must be signed in to change notification settings - Fork 304
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Allow more characters in element/attribute names and prefixes #1079
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to call out the =
difference in a note? At least a comment would be good I think.
I'm not a big fan of adding "DOM API" to the naming. That makes more sense if this was defined outside of the DOM Standard itself. I think dropping it would still make everything work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this looks good to me, modulo a small oversight. Anyone else that should review this? @mfreed7 perhaps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Discussed equals sign at HTML triage meeting. Conclusion: disallow it in attributes everywhere. (Even though the parser allows it in the first-character position.) |
I think this is ready for re-review. Potential issue: XML's definition of Char seems nonsensical (it excludes various Unicode characters below U+0020). And, its definition of the |
Refined to no longer use EBNF. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not seem equivalent to the sorta-EBNF from before. In particular if the first code point is from BeyondHTMLParserName the second code point was more limited.
I'm not sure exactly what you mean. Recall that it's a union of both. The second+ code point is from HTMLParserCompatibleName, which had |
I don't think the EBNF allows for the second code point to be U+0001 when the first is (I didn't see "An equivalent EBNF is the following" initially and I don't think what it states is correct.) |
I see, I did not capture that this was a branching scenario depending on the behavior of the first code point. And you addressed what harms names like that might hypothetically cause in #849 (comment) . I'll revise. |
I think that is done. The other way I could write this is by looping over the characters individually, which is what a performant implementation would do (instead of using lots of O(n) "contains" operations). But I think this is relatively clear. (Edit: well, a performant implementation would be looping over code units, since that's JS's native string format... which feels ickier to spec.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this looks accurate to me.
OK, this (and whatwg/html#7991) is just waiting on someone to write web platform tests. Then we can close a ~5 year old recurring pain point on the web platform! For fun, these are all the references to this I can find:
I suspect there are more GitHub issues from earlier, because why would I have posted #449 if not because of some other issue someone filed? But I couldn't find them. |
@josepharhar would you be interested in finishing this? |
Yes, I have started a WPT here: web-platform-tests/wpt#38503 |
\o/ I suspect that once you implement this and do a try run you'll find a lot of existing WPT tests that can be adjusted. There's probably no need for a new file, but maybe. |
Any progress on this? |
Not recently, I have unfortunately been focused on other stuff. |
Closes #849. Closes #769.
(See WHATWG Working Mode: Changes for more details.)
Original points for discussion, discussed and concluded on in following comments
=
inside attribute local names. Both the parser and DOM APIs currently disallow them, except the parser allows it for the first character. I'm happy to change this if people prefer; I started with the simpler version.createProcessingInstruction()
orcreateDocumentType()
. We could try to simplify those too, perhaps after investigating parser behavior. But they didn't seem to be causing any real web developer pain, unlike elements and local names, so I thought it'd be better to just leave them as-is.Preview | Diff