-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Update content model for customizable select #10586
base: main
Are you sure you want to change the base?
Conversation
This PR updated the content model for the <select>, <option>, and <optgroup> elements in support of customizable <select>. Fixes whatwg#10317
20549ca
to
088b919
Compare
Is the idea to allow If the former, you need to change the definition of phrasing content to include " If the latter, you need to change the content model if |
source
Outdated
<dd>Zero or more <span>select element inner content elements</span>.</dd> | ||
<dd>Zero or one child <code>button</code>.</dd> |
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.
<dd>Zero or more <span>select element inner content elements</span>.</dd> | |
<dd>Zero or one child <code>button</code>.</dd> | |
<dd>Zero or more <span>select element inner content elements</span>, followed by zero or one | |
<code>button</code> element.</dd> |
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.
The button should probably go first, and I wanted to keep the "zero or one" concept with the button, so I wrote it like this:
<dd>Zero or one <code>button</code> element followed by zero or more <span>select element inner content elements</span>.</dd>
Look good?
source
Outdated
<dd>If the element has no <code data-x="attr-option-label">label</code> attribute and is a child | ||
of a <code>datalist</code> element: <span data-x="text content">Text</span>.</dd> | ||
<dd>If the element has no <code data-x="attr-option-label">label</code> attribute: <span>Text content</span> and zero or more <code>div</code>, <code>span</code>, <code>noscript</code>, <code>img</code>, <span>SVG <code>svg</code></span>, and <span data-x="script-supporting elements">script-supporting</span> elements.</dd> | ||
<dd><div class="note">If an <code>img</code> is used within an <code>option</code> and there is no other text within the <code>option</code>, then the <code>option</code> won't have an accessible name unless the <code>img</code> also has a non-empty <code data-x="attr-img-alt">alt</code> attribute.</div></dd> |
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.
Don't have notes in the "element" box, put it further down along with the prose about the content model. The note is also not correct, so maybe it's better to remove it?
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.
I thought this was what @scottaohara suggested, but as per this comment, it sounds like we can add notes and examples as a follow up. I removed the note for now.
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.
besides the placement could you elaborate on what was wrong with the note @zcorpan? i might have phrased it a bit differently, but it seemed to be saying <option><img src=...></option>
would have no accessible name, and that is correct.
h1 elements are not supposed to be part of the content model, so I implemented the div and span switching by pulling out the content models for option and optgroup and using them in div and span. |
source
Outdated
@@ -53203,7 +53280,7 @@ interface <dfn interface>HTMLButtonElement</dfn> : <span>HTMLElement</span> { | |||
<dt><span data-x="concept-element-contexts">Contexts in which this element can be used</span>:</dt> | |||
<dd>Where <span>phrasing content</span> is expected.</dd> | |||
<dt><span data-x="concept-element-content-model">Content model</span>:</dt> | |||
<dd>Zero or more <code>option</code>, <code>optgroup</code>, <code>hr</code>, and <span data-x="script-supporting elements">script-supporting</span> elements.</dd> | |||
<dd>Zero or one <code>button</code> element followed by zero or more <span>select element inner content elements</span>.</dd> |
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.
Description of "As first child of select element" should be added to "Contexts in which this element can be used:" of section of "4.10.6 The button element".
In this case, this button element is neither of "Submit Button" state, "Reset Button" state or "Button" state.
Therefore, a fourth state of button element sholud be defined.
In addition, Shouldn't The following sentent be added to section of "4.10.6 The button element"?
If the button element is a fourth state, "popovertarget", "popovertargetaction", "type", "name" and "value" attribute are must not be spedified.
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.
Description of "As first child of select element" should be added to "Contexts in which this element can be used:" of section of "4.10.6 The button element".
Done
In this case, this button element is neither of "Submit Button" state, "Reset Button" state or "Button" state. Therefore, a fourth state of button element sholud be defined.
In addition, Shouldn't The following sentent be added to section of "4.10.6 The button element"?
If the button element is a fourth state, "popovertarget", "popovertargetaction", "type", "name" and "value" attribute are must not be spedified.
Those states are based on the type attribute. Since we didn't add any states when adding popovertarget, I don't think we should add a state here.
I suppose that we could say that the type attribute should not be set when the button is the first child of a select or when it has the popovertarget attribute though? Perhaps as a follow-up?
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.
In "type" attribute of button element, the attribute's missing value default and invalid value default are both the Submit Button state.
Therefore, if "type" attribute would be simply prohibited, button element which is child of select element would be Submit Button state.
Because this is not appropriate, shouldn't the fourth state of button element be defined?
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.
defining a fourth button type sounds like it could be potential scope creep, but @mtrootyy does make a good point that it is probably worth noting that this button is not a submit button, and that it also probably shouldn't even allow many of the attributes that would be valid for a button in other contexts.
source
Outdated
<span>inter-element whitespace</span>.</dd> | ||
<dd>If the element has no <code data-x="attr-option-label">label</code> attribute and is a child | ||
of a <code>datalist</code> element: <span data-x="text content">Text</span>.</dd> | ||
<dd>If the element has no <code data-x="attr-option-label">label</code> attribute: Zero or more <span>option element inner content elements</span>.</dd> | ||
<dt><span data-x="concept-element-attributes">Content attributes</span>:</dt> |
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.
"Content model:" should become the following.
If the element has a label attribute and a value attribute: Nothing.
If the element has a label: Text.
If the element has no value attribute: Text.
If the element has no label attribute and has a value attribute: Zero or more option element inner content elements.
Because the submitted value must be simply text, and "option element inner content elements" is only used to decorate label text.
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 could use some examples. Or are we adding those as part of another PR?
source
Outdated
<h6>Select element inner content elements</h6> | ||
|
||
<p><dfn>Select element inner content elements</dfn> are <code>option</code> elements and other | ||
elements which are used to group and decorate them with a <code>select</code> element.</p> |
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 reads weird.
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.
I re-wrote it like this: ... are the elements which are allowed as descendants of select elements.
source
Outdated
@@ -12785,6 +12785,68 @@ https://software.hixie.ch/utilities/js/live-dom-viewer/?%3C%21DOCTYPE%20HTML%3E% | |||
</ul> | |||
|
|||
|
|||
<h6>Select element inner content elements</h6> |
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.
I think it's rather confusing to have both select
element and "Select element".
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.
I'm not sure if I should reference <code>select</code>
in the <dfn>
for this concept, and I didn't make all of the references to it do that either.
Should I do that instead? Or are there other places where I'm using <code>select</code>
that should just be "select"?
In regards to the updated content model for the select element and its allowed children, an `optgroup` can have a `legend` element as its first child, and this `legend` needs to be able to name the `optgroup` similarly to how a `legend` names a `fieldset`. see: whatwg/html#10586
@scottaohara |
@mtrootyy then even more reason the spec needs to be updated to mention this, and that work be done to not render both the text from the label attribute AND the legend - which is what's happening now and seems rather rubbish - especially if someone has to write duplicate / close to duplicate content just so they can put an image into their optgroup label Edit: under the assumption that if both are used, both will continue to be rendered, i am trying to account for that in this html aam PR |
source
Outdated
@@ -55773,8 +55842,9 @@ interface <dfn interface>HTMLFieldSetElement</dfn> : <span>HTMLElement</span> { | |||
<dd>None.</dd> | |||
<dt><span data-x="concept-element-contexts">Contexts in which this element can be used</span>:</dt> | |||
<dd>As the <span>first child</span> of a <code>fieldset</code> element.</dd> | |||
<dd>As the <span>first child</span> of an <code>optgroup</code> element.</dd> |
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.
Some sentence should be added to prose of section of "4.10.16 The legend element".
For example;
The legend element represents a caption for the rest of the contents of the legend element's parent fieldset element, if any.
↓
The legend element represents a caption for the rest of the contents of the legend element's parent fieldset element, if any.
Or, the legend element represents label of it's parent optgroup element.
I tried adding an example initially, but I put it in the wrong spot and/or otherwise messed it up: #10586 (comment) Scott suggested doing a follow up, so that's my plan: #10317 (comment) |
This PR removes some important(?) context about option use within / outside of a datalist.
per the above, if i have but again, that only works within a datalist, which would be good to point out (unless we want to do similar for options within a select - which could be useful.....) Additionally, testing with the current chromium canary build, following the first content model condition results in no visible label for an option
so if the label attribute is not being used in the customizable select, i think this needs to be clearly stated. (checking/rechecking the definition for option and the label attribute, i don't see this called out?) but just as importantly, it should also be clear whether new content model allowances are only allowed within options of a styleable select, or if they are allowed for options in a datalist. the current PR reads as if this is a change for everywhere, and i'm not sure if that's really the intent at this point? Edit: we spoke about some of this in OpenUI yesterday, so just wanted to acknowledge what I wrote here is at least going to be partially addressed. |
I re-added the datalist cases and added the new content model under the case where there is no label attribute and there is no parent datalist.
This has been fixed |
@@ -24529,10 +24585,14 @@ wormhole connection.</mark></p></code></pre> | |||
<dd><span>Flow content</span>.</dd> | |||
<dd><span>Phrasing content</span>.</dd> | |||
<dd><span>Palpable content</span>.</dd> | |||
<dd><span>Select element inner content elements</span>.</dd> | |||
<dd><span>Optgroup element inner content elements</span>.</dd> |
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.
span element is not "Select element inner content elements" and "Optgroup element inner content elements"
<span>phrasing content</span>, <span>flow content</span>, and <span>palpable content</span> | ||
categories for the purposes of the content models in this specification.</p> | ||
<span>phrasing content</span>, <span>flow content</span>, <span>palpable content</span>, and | ||
<span>option element inner content elements</span> categories for the purposes of the content |
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.
As long as it is described that it is "phrasing content", it is not necessary to described that it is "Option element inner content elements".
The same is true for "span", "img", "noscript".
<dt><span data-x="concept-element-contexts">Contexts in which this element can be used</span>:</dt> | ||
<dd>Where <span>phrasing content</span> is expected.</dd> | ||
<dt><span data-x="concept-element-content-model">Content model</span>:</dt> | ||
<dd><span>Phrasing content</span>.</dd> | ||
<dd>If the element is a descendant of an <code>option</code> element: Zero or more <span>option element inner content elements</span>.</dd> |
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.
Is this sentence necessary?
If it is necessary here, then it seems necessary for "Content model:" of all of the other element of "Phrasing content" as well as .
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.
OP says this resolves #10317 (comment) (among other things) but I don't see that aspect defined here. Where is that defined?
child of a <code>datalist</code> element: <span data-x="text content">Text</span> that is not | ||
<span>inter-element whitespace</span>.</dd> | ||
child of a <code>datalist</code> element: Zero or more <span>option element inner content | ||
elements</span>.</dd> | ||
<dd>If the element has no <code data-x="attr-option-label">label</code> attribute and is a child |
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.
To deal with the following cases, the "child" should be changed to "descendant".
<label>
Animal:
<input name=animal list=animals>
</label>
<datalist id=animals>
<label>
or select from the list:
<select name=animal>
<option value="">
<option>Cat
<option>Dog
</select>
</label>
</datalist>
The text in the legend element replaces the text in the label attribute for both rendering and what's exposed to the accessibility tree. The fact that both the label attribute gets rendered when you have a legend is a bug, I'll fix it here: https://issues.chromium.org/issues/378601807 |
good to know. i need to revise the HTML AAM PR for this now, since i was attempting to account for both being rendered until your comment. |
This PR updated the content model for the
<select>
,<option>
, and<optgroup>
elements in support of customizable<select>
.Fixes #10317
(See WHATWG Working Mode: Changes for more details.)
/dom.html ( diff )
/embedded-content-other.html ( diff )
/embedded-content.html ( diff )
/form-elements.html ( diff )
/grouping-content.html ( diff )
/index.html ( diff )
/scripting.html ( diff )
/text-level-semantics.html ( diff )