Relax <select> parser

[josepharhar]

This patch makes the parser allow additional tags in <select> besides <option>, <optgroup>, and <hr>, mostly by removing the "in select" and "in select in table" parser modes.

In order to replicate the behavior where opening a <select> tag within another open <select> tag inserts a </select> close tag, a traversal through the stack of open elements was added which I borrowed from the <button> part of the parser.

This patch also changes the processing model to make <select> look through all its descendants in the DOM tree for <option> elements, rather than just children and optgroup children which conform to the content model. This is needed for compat reasons because there are websites which put other tags in between their <select> and <option>s which would break with this parser change unless we also update this processing model. More context here and here.

Fixes #10310

• At least two implementers are interested (and none opposed):
  • Chromium
  • Relax <select> parser mozilla/standards-positions#1086
  • Relax <select> parser WebKit/standards-positions#414
• Tests are written and can be reviewed and commented upon at:
  • Update tests for relaxed <select> parser html5lib/html5lib-tests#178
  • https://wpt.fyi/results/html/semantics/forms/the-select-element/customizable-select/select-parsing.tentative.html
• Implementation bugs are filed:
  • Chromium: Already implemented
  • Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1933591
  • WebKit: https://bugs.webkit.org/show_bug.cgi?id=283732
• MDN issue is filed: I read the select, option, and optgroup element pages, and I didn't see any content that needs to be updated.
• The top of this comment includes a clear commit message to use.

(See WHATWG Working Mode: Changes for more details.)

---

/form-elements.html ( diff )
/index.html ( diff )
/infrastructure.html ( diff )
/parsing.html ( diff )

[annevk]

Why does this also make a bunch of changes to the processing model of select elements? If that needs to be part of this change that should really be better motivated in the commit message.

[josepharhar]

Why does this also make a bunch of changes to the processing model of select elements? If that needs to be part of this change that should really be better motivated in the commit message.

The changes in the processing model do support the new proposed content model for select, but they also mitigate compat issues for websites which are already putting tags in between <select> and <option> in their HTML.

For example, without these changes to the processing model, the following <select> would not register as having any options at all:

<select>
  <div>
    <option>...</option>
    ...
  </div>
</select>

In my compat analysis, I found a lot of websites which are doing this, so in order to ship the parser changes separately from customizable select in chrome, I plan to ship the parser changes and these processing model changes together, because otherwise there would be too much breakage.

I'm happy to put them in a separate PR if you want, or keep them here and update the commit message (sorry for not putting it in there). Which would you prefer?

[annevk]

Thanks! Given that rationale I think it's good to couple the changes, but that should be in the commit message as well.

[zcorpan]

This doesn't define optional tags for </option> and </optgroup> correctly.

The definition for "have a particular element in select scope" may be needed for that, but should be changed to be similar to "have a particular element in button scope" (but for select).

In particular, allow these without a parse error:

<select>
<optgroup>
<option>
<optgroup>
</select>

The second <optgroup> should pop the option and optgroup

<select>
<option><p>
<option>
</select>

This should generate implied end tags and pop the option.

<select>
<optgroup>
<hr>
<option>
<hr>
</select>

The hrs should pop the optgroup and option in a select.

See how the parser deals with <ruby><rtc><rt>, I think that can be used as a model for select.

[josepharhar]

Thanks! Given that rationale I think it's good to couple the changes, but that should be in the commit message as well.

Done.

This doesn't define optional tags for </option> and </optgroup> correctly.

@zcorpan thanks for the feedback! I did some experimenting and added some logic to the start tags for option, optgroup, and hr. What do you think?

[zcorpan]

• Need to check that a select element is in scope so that parsing of option/optgroup tags outside of select doesn't change. Example <option><p>1</option>2<option>3
• Need to add select to the "in scope" list so that <div><select></div><option> doesn't close the select.
• Should report parse errors when unexpected elements are popped. Compare with what the spec does for rt/rtc in ruby. Example <select><option><span>1</option>2<option>3

[josepharhar]

• Need to check that a select element is in scope so that parsing of option/optgroup tags outside of select doesn't change. Example <option><p>1</option>2<option>3

I thought that this is covered by "While the stack of open elements has an option element in select scope". What exactly should I change?

• Need to add select to the "in scope" list so that <div><select></div><option> doesn't close the select.

Done

• Should report parse errors when unexpected elements are popped. Compare with what the spec does for rt/rtc in ruby. Example <select><option><span>1</option>2<option>3

I'm guessing this is from "If the current node is not now a rtc element or a ruby element, this is a parse error," right?

Should I add "If the current node is not now an option element, this is a parse error" after "While the stack of open elements has an option element in select scope, pop an element from the stack of open elements"?

[zcorpan]

The "in select scope" I think should be removed altogether since it assumes the stack will not have other elements when in a select, which is no longer the case. Use the normal "in scope" instead.

High-level of what I think should happen: when parsing option or optgroup start tag: check that select is in scope, check that option or optgroup is in scope, generate implied end tags (except for optgroup when handling <option>), check the current node or the stack of open elements again, report a parse error if appropriate, then pop elements off the stack until the option or optgroup has been popped, then insert the new element.

I can look into this more next week and suggest more specific changes.

[josepharhar]

High-level of what I think should happen: when parsing option or optgroup start tag: check that select is in scope, check that option or optgroup is in scope, generate implied end tags (except for optgroup when handling <option>), check the current node or the stack of open elements again, report a parse error if appropriate, then pop elements off the stack until the option or optgroup has been popped, then insert the new element.

Thanks! I gave this a try

[josepharhar]

@zcorpan how does the latest text look?

[zcorpan]

I think the option/optgroup cases are right after these changes.

[josepharhar]

This doesn't define optional tags for </option> and </optgroup> correctly.

I was talking to @mfreed7 about the changes we've made in the PR so far, and I feel like I couldn't provide a good explanation for why we are making the parser not support cases like these:

• <hr> inside an <option>
• nested <optgroup>s

Is it just compat reasons? Is there a good justification?

[zcorpan]

It would be a breaking change from what is conforming HTML today, and break compat for sites that omit </option> and </optgroup> tags. It's the same as why we can't allow certain elements in p.

[zcorpan]

Per discussion in Matrix (1, 2), the web compat situation is not yet clear, see e.g. <select><input> issue: https://issues.chromium.org/issues/379612186

@josepharhar what is the rollout plan in Chromium? I think it makes sense to wait with merging this PR until it has been shipping in Stable for a bit.

[josepharhar]

I disabled this new parser behavior in chrome 131 due to a bug with one of multiple code paths which collects options to render in the select's popup which doesn't exist in the spec. I am going to re-enable in chrome 133, and yes <input> in <select> is still an open question. I'd like to support this if possible and I will come back here to report on that after the next shipping attempt.

I think it makes sense to wait with merging this PR until it has been shipping in Stable for a bit.

That sounds reasonable to me especially since shipping this change in chrome is not blocked by merging this PR.

[lukewarlow]

I think this is missing a required spec change: https://html.spec.whatwg.org/multipage/parsing.html#:~:text=An%20end%20tag%20whose%20tag%20name%20is%20one%20of%3A%20%22address%22 - select needs adding to this list of elements?

[lukewarlow]

Btw I think this test needs updating as it's currently failing in Chromium with this change

[zcorpan]

@lukewarlow good catch! Right now the end tag is handled by "Any other end tag", which will ignore the tag if the current node is not a select element. (Chromium seems to implement something else, testing with <select><div></select>x.)

[josepharhar]

I think this is missing a required spec change: https://html.spec.whatwg.org/multipage/parsing.html#:~:text=An%20end%20tag%20whose%20tag%20name%20is%20one%20of%3A%20%22address%22 - select needs adding to this list of elements?

@lukewarlow good catch! Right now the end tag is handled by "Any other end tag", which will ignore the tag if the current node is not a select element. (Chromium seems to implement something else, testing with <select><div></select>x.)

Thanks, I added select there in this PR. It's already included there in chromium.

Btw I think this test needs updating as it's currently failing in Chromium with this change

Thanks, fixing that here.

[flavorjones]

@josepharhar Are the test changes in html5lib/html5lib-tests#178 up to date with these proposed spec changes now? It's been hard to follow along across the multiple repositories.

[josepharhar]

@josepharhar Are the test changes in html5lib/html5lib-tests#178 up to date with these proposed spec changes now? It's been hard to follow along across the multiple repositories.

I just updated all tests in the PR

[flavorjones]

It doesn't look like the pr-preview bot updated the "diff" links after f8d1434 was added to this PR (the links still point to 08bdb25). Would it be possible for someone to nudge the bot to recreate those links? Thank you!

[flavorjones]

@josepharhar Following up on #10557 (comment) I believe you also need to specify here that the token should be ignored. As written, the token would not be consumed.