Skip to content

Make element and attribute names behave like the HTML parser #449

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

Closed
wants to merge 2 commits into from

Conversation

domenic
Copy link
Member

@domenic domenic commented Apr 26, 2017

@dominiccooney has started gathering data on this in Blink, so I wanted to help the process along by preparing a spec PR. We should not merge this yet.

The biggest open question, which we could use some help on, is what to do with the -NS variants, createElementNS and createAttributeNS/setAttributeNS.

One idea I thought of is that "validate and extract" should delegate to the HTML validate-and-canonicalize algorithms if the namespace passed in is the HTML namespace.

Another idea is that we should just leave them alone. But I think the same frameworks that are asking for this change for HTML elements probably also want it for SVG elements. (I'm surprised SVG elements care what namespace their attributes are in...)


Note to self: we need to remember to also update the restrictions on valid custom element names, since they were designed to be the union of restrictions from the HTML parser + the DOM APIs.


💥 Error: 500 Internal Server Error 💥

PR Preview failed to build. (Last tried on Jan 15, 2021, 7:33 AM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 CSS Spec Preprocessor - CSS Spec Preprocessor is the web service used to build Bikeshed specs.

🔗 Related URL

<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>500 Internal Server Error</title>
</head><body>
<h1>Internal Server Error</h1>
<p>The server encountered an internal error or
misconfiguration and was unable to complete
your request.</p>
<p>Please contact the server administrator at 
 [no address given] to inform them of the time this error occurred,
 and the actions you performed just before this error.</p>
<p>More information about this error may be available
in the server error log.</p>
<hr>
<address>Apache/2.4.10 (Debian) Server at api.csswg.org Port 443</address>
</body></html>

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

@annevk
Copy link
Member

annevk commented Apr 26, 2017

The biggest open question, which we could use some help on, is what to do with the -NS variants, createElementNS and createAttributeNS/setAttributeNS.

Those are the variants you would actually use internally, as you don't want to use setAttribute() which is rather odd when it comes to namespaces.

(I'm surprised SVG elements care what namespace their attributes are in...)

HTML elements care too. Say more.

@domenic
Copy link
Member Author

domenic commented Apr 26, 2017

Those are the variants you would actually use internally, as you don't want to use setAttribute() which is rather odd when it comes to namespaces.

What do you mean "internally"? Looking at e.g. Ember or React it seems like everyone branches on whether they're supplied a namespace by their caller (presumably only for SVG cases) and uses setAttribute instead. Anecdotally, I've never used setAttributeNS myself.

HTML elements care too. Say more.

I would have thought that after all the trouble we went through to try to simplify Attrs, at least one of the changes that might have stuck was making their namespace APIs have no effect on the processing models that consult those attributes. I guess not.

@annevk
Copy link
Member

annevk commented Apr 26, 2017

I mean browsers. Browsers definitely care about the namespace of an attribute. And the problem with setAttribute() is that it doesn't care, so you might be accidentally overwriting a value of a namespaced attribute, while you were just trying to set a (new) non-namespaced attribute. (This is only a problem for frameworks of course, and only for those that care about usually theoretical scenarios.)

@domenic
Copy link
Member Author

domenic commented Apr 26, 2017

I'm still a little unclear on what you mean by "care". They store it, so it can be reflected in other DOM APIs, sure. But e.g. if I set the src="" attribute on a script element using setAttribute("src", "...") (which sets a null namespace, instead of the HTML namespace) it seems like browsers still load the script file.

Basically my confusion is about whether anyone ever cares about setting the namespace of an attribute "correctly", such as when trying to set two attributes with the same name but different namespace. It seems like the fact frameworks provide methods that take a namespace means some people care. I am not sure why, given my above example of browsers not caring about the namespace for <script src>, although I suspect it has something to do with SVG.

@annevk
Copy link
Member

annevk commented Apr 26, 2017

Why do you think it should be the HTML namespace? (Hint: if you use the HTML namespace for the "src" attribute, it won't load.)

@domenic
Copy link
Member Author

domenic commented Apr 26, 2017

Ah, very interesting. Browsing https://developer.mozilla.org/en-US/docs/Web/SVG/Namespaces_Crash_Course, it appears that people use namespaces for xlink:href mostly.

This makes me lean toward not caring about the -NS methods for attributes at all as people should mostly only have to use them for xlink:href, which is going away. They can just use the null namespace for all attributes most of the time, which means only using createAttribute/setAttribute.

I guess that still leaves createElementNS though, and we maybe should make the attribute-NS methods consistent with that. So, what should our answer be for createElementNS?

@annevk
Copy link
Member

annevk commented Apr 26, 2017

Given that createElement() depends on context for creating HTML elements (and cannot create SVG elements) it seems unsuitable as well. So I think we definitely want to make createElementNS() work, if anything.

@dominiccooney
Copy link

My $0.02, I think you do want createElementNS to use the HTML rules for local names in the HTML namespace. Why? It would be good if there's one line of code authors can reliably use to duplicate an element. I think that line has to be:

document.createElementNS(
    element.namespaceURI,
    `${element.prefix}:${element.localName}`,
    magic(element));

(OK three lines. And a function, "magic", which somehow detects autonomous built-in elements and produces a dictionary with 'is' for them.)

Maybe validate and extract step 5 should specify that it splits on the first colon. Since :foo would not be a valid local name any more, validate and extract may want to map the empty string prefix to null so that the checks in step 6 work.

If you make the local name checks consistent with createElement's then that line of code should be sufficient for duplicating an element, whether it is HTML, SVG or vanilla.

@domenic domenic force-pushed the relax-name-validation branch from d607808 to d91bdac Compare May 11, 2017 00:01
@domenic
Copy link
Member Author

domenic commented May 11, 2017

I've updated this with a revision that takes into account the -NS methods too.

@dominiccooney
Copy link

I have posted some data from Chromium here: https://bugs.chromium.org/p/chromium/issues/detail?id=648179&desc=2#c19

@domenic
Copy link
Member Author

domenic commented Aug 15, 2017

OK, so we finally have data!

  • DOM valid, HTML parser invalid: 0.047 % page loads
  • DOM invalid, HTML parser valid: 0.000003 % page loads

Unfortunately, it looks like simply aligning with the HTML parser is not going to be possible. @dominiccooney mentioned it might be possible to change the parser to allow more, but in general that's pretty high-cost and risky. Unless there's some unexpected enthusiasm for that, I'd rather set it aside.

What do people think of just expanding the valid-in-DOM category? Then we will still have two categories, but at least valid-in-DOM will be a superset of valid-in-HTML. I could work on updating this PR to illustrate that concretely, if people would like.

@annevk
Copy link
Member

annevk commented Aug 16, 2017

I always thought that was the plan. With the syntax being less expressive. Matching the HTML exactly would never have worked due to casing depending on context and such.

@strongholdmedia
Copy link

What do people think of just expanding the valid-in-DOM category?
I personally think that you should say WHICH DOM.
There is almost as much cowboyism recently with custom rules and "solutions" lately (as in angular, etc), that makes me think the "creators" either forgot quirks mode and browser wars, or just entered the game after it was over (is it even, btw?)


<li><p>If <var>qualifiedName</var> contains a "<code>:</code>" (U+003E), then split
<var>qualifiedName</var> on that first occurrence of "<code>:</code>" and set <var>prefix</var> to
the part before and <var>localName</var> to the part after.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@domenic before and after seem a bit vague. Perhaps suffix with ... _prefix to the part before and localName to the part after the : respectively.

Just a thought. Sounded incomplete when reading out loud.

Base automatically changed from master to main January 15, 2021 07:32
@domenic domenic closed this Oct 25, 2021
@annevk annevk deleted the relax-name-validation branch October 25, 2021 16:00
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
do not merge yet Pull request must not be merged per rationale in comment needs tests Moving the issue forward requires someone to write tests
Development

Successfully merging this pull request may close these issues.

5 participants