Define optgroup base appearance rendering#12201
Define optgroup base appearance rendering#12201josepharhar wants to merge 10 commits intowhatwg:mainfrom
Conversation
The old text didn't explain that there is a shadow tree with an element which renders the label attribute, and that it goes away when the author provides a legend element. The UA styles for rendering the optgroup element with base appearance are still valid. Fixes whatwg#12199
annevk
left a comment
There was a problem hiding this comment.
I think we should also change <optgroup>.label so it actually reflects the legend element when there is one. Seems quite strange the API would not reflect all of this. But that's probably a separate change at this point with its own tests.
| <span>expected</span> to contain a copy of the text in the <code>optgroup</code>'s <code | ||
| data-x="attr-optgroup-label">label</code> attribute. If the <code>optgroup</code> has a child | ||
| <code>legend</code> element, then the <span>optgroup label element</span> is not | ||
| rendered.</p></li> |
There was a problem hiding this comment.
Why not use a slot with fallback for this? Does the legend element intentionally have different requirements than the button element. It can appear anywhere?
There was a problem hiding this comment.
I also found out today that this optgroup label element is expected to have padding?
There was a problem hiding this comment.
And in Chrome min-height is set to 0 but that should probably be auto? cc @nt1m
There was a problem hiding this comment.
Why not use a slot with fallback for this? Does the
legendelement intentionally have different requirements than thebuttonelement. It can appear anywhere?
It is supposed to be first, so yeah we could do slotting instead: https://html.spec.whatwg.org/multipage/form-elements.html#the-optgroup-element
I'll go ahead and make that change here, in chromium, and WPT.
I also found out today that this optgroup label element is expected to have padding?
Yes, I made it have the same style as the legend element when rendered with base appearance. If we made it a web exposed part-like pseudo-element, which would be awesome, then we could define that in the same style rule as the legend element. Since it isn't, at least not yet, I'll add a paragraph there which says what the expected styles are.
And in Chrome
min-heightis set to 0 but that should probably be auto? cc @nt1m
Ah, my bad I thought that 0 was the default value. I'll change it to something better like unset.
There was a problem hiding this comment.
Maybe we can first agree on the details? Since we’d have to change styles as well presumably. I’m not a 100% sold on either approach yet.
There was a problem hiding this comment.
Sure, how would you like to go about agreeing on the details?
| <code>option</code> <span data-x="option-base-appearance">is being rendered with base | ||
| appearance</span> and the <code>option</code>'s <code data-x="attr-option-label">label</code> | ||
| attribute is not set, then the <code>option</code> is <span>expected</span> to render all of its | ||
| children rather than by displaying its <span data-x="concept-option-label">label</span>.</p> |
There was a problem hiding this comment.
Ok, I replaced this with an internal shadow tree definition
Apparently min-height:0 is not the initial/default value that we should be using, so I am changing it to unset. Context: whatwg/html#12201 (comment) Change-Id: Id3af6326646851d31c614b0290f9feca3db5e91e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7630018 Commit-Queue: Joey Arhar <jarhar@chromium.org> Reviewed-by: Joey Arhar <jarhar@chromium.org> Reviewed-by: David Baron <dbaron@chromium.org> Cr-Commit-Position: refs/heads/main@{#1608241}
|
I think we want to make it so only the first legend of an optgroup renders right? Currently chromium renders all legends, and Safari Tech Preview renders only the first. Making the rest display: none seems reasonable (not sure if that is what Safari has done). Though saying that fieldset currently renders all legends you give it, it just only treats the first one specially. Maybe we should still render the legends but just not give them the special styles, specifically I don't htink they should get the optgroups font-weight bolder. |
Considering that we still render everything else that doesn't fit in the content model, I don't think we should add behavior to not render this case in particular - assuming that you're talking about base appearance, not native appearance.
The screenshot in your second comment seems to show this, and yeah it looks to me like firefox just hasn't gotten around to implementing this yet. |
|
It feels weird for the second one to be bold to me given it's nothing special it's just text at that point. But perhaps if it's unbold it'll look too much like an option. Idk the safari option feels better at least for auto appearance. But I'll defer to others. |
|
Listbox base appearance is not yet finalized, right? For listbox auto appearance I would expect "get an optgroup element's label" to determine the label. And only the For dropdown base appearance I think it's fine if multiple |
|
Listbox base appearance is in the spec fwiw. |
|
Should I add any text to this PR which says that in native/primtive appearance only one legend or label should be rendered? Or anything else from #12199 like explicitly saying that when options and optgroups are placed outside of a select element they render their contents like a normal element? |
|
If the aim is to fix that issue just defining the base appearance is insufficient. As customizable select introduced new features, we need to update the rendering text to account for those, including for native appearance. I would be okay with addressing the issue with several PRs, but then OP should not claim it fixes that issue. |
|
Ok, I changed the op to say that the issue is not fixed and filed bugs. |
| <p>An <code>optgroup</code> element is <dfn data-x="optgroup-base-appearance">rendered with base | ||
| appearance</dfn> if it has an <span>ancestor</span> <code>select</code> element, and the nearest | ||
| <span>ancestor</span> <span><code>select</code>'s <code>option</code>s are being rendered with | ||
| base appearance</span>.</p> |
There was a problem hiding this comment.
Why are we talking about option elements here? An optgroup can't have option element ancestors, can it? Can we reference the existing definition for nearest ancestor select element?
There was a problem hiding this comment.
I did this in order to reuse the algorithm which determines when the elements inside a select besides the option are being rendered with base appearance: https://html.spec.whatwg.org/multipage/rendering.html#select's-options-are-being-rendered-with-base-appearance
I removed the word option from the algorithm to make that more clear. How does it look now?
| <p>If an <code>optgroup</code> element is not being <span | ||
| data-x="optgroup-base-appearance">rendered with base appearance</span>, then it is | ||
| <span>expected</span> to be rendered by displaying the element's <span | ||
| data-x="concept-optgroup-label">label</span>.</p> |
There was a problem hiding this comment.
I think if we're not fixing this here we should add an XXX marker as this is clearly broken.
There was a problem hiding this comment.
ok, i added an XXX
| displaying its <span data-x="concept-option-label">label</span>.</p> | ||
| <p>If an <code>option</code> element is not being <span data-x="option-base-appearance">rendered | ||
| with base appearance</span>, then it is <span>expected</span> to be rendered by displaying the | ||
| result of <span>collect option text</span> given the <code>option</code> and true.</p> |
There was a problem hiding this comment.
I think we first have to define what it means for an option element to be rendered with base appearance as base/native appearance isn't a thing for option elements (aiui). So the same order as above for optgroup with probably the same set of comments.
If we also tackle option we need to update the PR to make that clear.
There was a problem hiding this comment.
So should I add a <p class="XXX"> saying that option elements need their native and primitive appearance to be specified?
There was a problem hiding this comment.
That probably has to happen as well then, yes.
My preferred solution would be that we just define it as part of select's native appearance, but it seems that's not possible given how Chromium does things here and maybe Gecko?
There was a problem hiding this comment.
I took another look and I'm not sure what the option element is missing that the optgroup element has with regards to base/native appearance in this PR. Could you elaborate on what is missing?
There was a problem hiding this comment.
For optgroup we start out with defining what it means to be rendered with base appearance. For instance, we require an ancestor select (though we need to make that more precise so it deals with intermediate hr elements and the like I think).
I'm not sure that is sufficient however as these don't really have base appearance through appearance: base. It's a result of nesting, right? So I'm not sure that makes them compatible with the appearance base infra.
There was a problem hiding this comment.
I think what we want is that some things apply universally, such as display: block and some things are conditional upon the nearest ancestor select's computed/used appearance. Because I don't think we can say that option elements themselves have any kind of appearance, right?
There was a problem hiding this comment.
I think what we want is that some things apply universally, such as display: block and some things are conditional upon the nearest ancestor select's computed/used appearance.
Yes, that makes sense. I took a look at chromium, and the only UA style rule that is applied to option elements in both appearance:auto and appearance:base selects is white-space:nowrap, which maybe is another thing we just need to undo for "base appearance".
Because I don't think we can say that option elements themselves have any kind of appearance, right?
Yes, option elements don't support the appearance property. If you think we should rename this definition for options and optgroup elements, I'd be happy to rename it.
There was a problem hiding this comment.
white-space:nowrap is currently non-standard. Does Gecko have that?
WebKit is adding display: block to both option and optgroup elements, and font-weight: bolder for optgroup, but that's it thus far (https://commits.webkit.org/311911@main). We should make the user agent style sheet reflect at least that.
As for the language, maybe something like "option/optgroup elements [with a base appearance select]" and then "with a base appearance select" is defined through the ancestor select element thingy that also handles hr elements and the like and requires the select element to have base appearance?
There was a problem hiding this comment.
white-space:nowrap is currently non-standard. Does Gecko have that?
It is standardized for base appearance: https://html.spec.whatwg.org/multipage/rendering.html#:~:text=0.5em%3B-,white%2Dspace%3A%20nowrap,-%3B%0A%7D%0A%0Aselect
WebKit is adding
display: blockto bothoptionandoptgroupelements, andfont-weight: bolderforoptgroup, but that's it thus far (https://commits.webkit.org/311911@main). We should make the user agent style sheet reflect at least that.
Thanks, I see that this is about UA styles for options and optgroups when they are outside of a select element, not about base appearance. I'd be supportive of standardizing/changing those styles as long as there aren't compat issues. Maybe that's worth a separate spec issue and PR?
I added properties like white-space:nowrap to the base appearance section because I didn't think that we would actually standardize the UA styles for these elements outside of base appearance.
As for the language, maybe something like "option/optgroup elements [with a base appearance select]" and then "with a base appearance select" is defined through the ancestor select element thingy that also handles
hrelements and the like and requires theselectelement to have base appearance?
Ok, I used the shared algorithm to find an ancestor select, and I'm still using the shared algorithm which defines when select's contents are rendered with base appearance. How does it look?
There was a problem hiding this comment.
I think this works overall. I think we should probably keep things restricted to the base appearance style sheet if they are not universal, but if we can agree on making them universal we should move them. I think that's the case for display: block and font-weight at least.
| not have the <code data-x="attr-option-label">label</code> attribute, or the <code | ||
| data-x="attr-option-label">label</code> is set to an empty string, then the <span>option label | ||
| element</span> is <span>expected</span> to have its <span>'display'</span> property set to | ||
| 'none'.</p></li> |
There was a problem hiding this comment.
So why not have this as a child of the slot element and then determine whether we slot things into the slot or not based on the condition of the label attribute? I thought that was the direction we were heading in.
There was a problem hiding this comment.
Ok, I rewrote it to use slot assignment and a fallback instead
|
|
||
| <p>The following styles are <span>expected</span> to apply to the <span>optgroup label | ||
| element</span> when its parent <code>optgroup</code> element is being <span | ||
| data-x="optgroup-base-appearance">rendered with base appearance</span>:</p> |
There was a problem hiding this comment.
Should we just make these unconditional based on select optgroup?
There was a problem hiding this comment.
I'd prefer not to, in chromium we have different styles for this element in appearance:auto: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/resources/html.css;l=2307-2309;drc=fc92cc6dc0a63cab87ad5e4ff9d3b17cdd7df198
There was a problem hiding this comment.
But the shadow tree is conditional upon a select with base appearance so there is no optgroup label element without that.
annevk
left a comment
There was a problem hiding this comment.
This looks better. We also discovered that display: block for option and optgroup is universal and not restricted to having a select ancestor. Should we make those changes as part of this? And optgroup has font-weight: bolder iirc.
|
|
||
| <li><p>An <dfn>optgroup contents slot</dfn>, which is a <code>slot</code> element. It is appended | ||
| to the <code>optgroup</code>'s <span>shadow root</span> after the <span>optgroup label | ||
| element</span>. It is <span>expected</span> to take all remaining child nodes of the |
There was a problem hiding this comment.
| element</span>. It is <span>expected</span> to take all remaining child nodes of the | |
| element</span>. It is <span>expected</span> to take all remaining child nodes of the |
|
|
||
| <p>The following styles are <span>expected</span> to apply to the <span>optgroup label | ||
| element</span> when its parent <code>optgroup</code> element is being <span | ||
| data-x="optgroup-base-appearance">rendered with base appearance</span>:</p> |
There was a problem hiding this comment.
But the shadow tree is conditional upon a select with base appearance so there is no optgroup label element without that.



The old text didn't explain that there is a shadow tree with an element which renders the label attribute, and that it goes away when the author provides a legend element. The UA styles for rendering the optgroup element with base appearance are still valid.
Context: #12199
(See WHATWG Working Mode: Changes for more details.)
/form-elements.html ( diff )
/rendering.html ( diff )