whatwg / html

HTML Standard
https://html.spec.whatwg.org/multipage/
Other
8.12k stars 2.67k forks source link

Is it ok for `appearance:base` `<select>` not to require user activation before `showPicker()`? #10604

Open mfreed7 opened 1 month ago

mfreed7 commented 1 month ago

What is the issue with the HTML Standard?

As we're working on the new appearance:base-select mode for <select>, we came across a question. The "old" (appearance:auto/none) select requires user activation before calls to showPicker(), mostly (I believe) for security reasons. Since the native picker can extend beyond the bounds of the viewport, it'd dangerous to allow it to be invoked directly by JS. However, in appearance:base-select mode, the picker is always just a popover, which can't extend beyond the viewport bounds. So it would seem that showing the <select> picker is no more dangerous than allowing popover.showPopover(), since they're essentially the same thing. So our plan was to remove that restriction when the select's ::picker(select) is set to appearance:base-select. But we wanted to see if there were any other reasons not to allow that?

pshaughn commented 1 month ago

What happens if appearance changes while the picker's open?

mfreed7 commented 1 month ago

What happens if appearance changes while the picker's open?

That’s something that came up in discussion last week. To avoid possible oscillation or hysteresis, if the appearance property changes while the picker (auto or base-select) is open, the picker will be closed.

annevk commented 1 month ago

This would also require showPicker() to perform some kind of style resolution, which seems suboptimal.

mfreed7 commented 1 month ago

This would also require showPicker() to perform some kind of style resolution, which seems suboptimal.

That'll be required anyway, for <select style="appearance:base-select">, to decide which picker to show.

annevk commented 1 month ago

No, that doesn't require it to be done synchronously as far as I can tell.

mfreed7 commented 1 month ago

No, that doesn't require it to be done synchronously as far as I can tell.

Can you elaborate? It seems like the picker should be opened synchronously or nearly-sync, right? I.e.

select.showPicker();
select.matches(‘:open’); // true
annevk commented 1 month ago

You could make reading style block as well in theory, no?

But taking a step back I don't really like the idea that CSS can be used to toggle API behavior in such a fundamental way. That seems like a pretty severe layering violation.

mfreed7 commented 1 month ago

But taking a step back I don't really like the idea that CSS can be used to toggle API behavior in such a fundamental way. That seems like a pretty severe layering violation.

Help me understand this comment a bit better, please. This is essentially exactly what appearance:base does for <select>, right? When appearance is base, we get a popover picker. When it isn't, we get a native picker.

annevk commented 1 month ago

That's about rendering, not behavior.

smaug---- commented 1 month ago

Do we expect that appearance:base-select selects might be still useable in UAs which don't support it (fall back to the old style select)? If so, having different model for showPicker() would be a breaking change. And I don't like inconsistencies. They tend to bite us always later.

mfreed7 commented 1 month ago

It'll be really unfortunate if the whole point of the new customizable-<select> work was to improve the status quo, and then the pushback to improving things is always "well, we added this as part of the existing <select> so we're stuck with the old behavior". I'm hoping we can see appearance:base-select as basically a new element, that just happens to re-use the old element's clothing (which as you point out is great for progressive enhancement). But this is precisely why our original proposal(s) were about adding a new element like <selectlist> to get away from all of the bad precedents. I'd love it if we consider the new customizable-<select> only, and try to decide what is the best behavior for that new element?

Also, I'm not sure it's a "breaking" change, since the site has to opt itself in by adding appearance:base-select to the <select>. It is a limitation on the progressive enhance-ability from old <select> to new <select>, but there are bound to be more of those. E.g. pickers that contain only images will not progressively enhance well either, and will need additional developer work to keep them usable. Perhaps showPicker() is another of these cases?

josepharhar commented 1 week ago

Not allowing the popover picker to be programatically opened is going to make it harder to test and I imagine that sites would want to do this. Back when the proposal allowed the author to provide their own <datalist> element it didn't matter as much because you could call showPopover on the datalist, but now that I removed it, the picker can't be programatically opened anymore.

Since WPTs don't allow user activation in reftests, I'm going to have to work around this limitation to write tests by setting display:block on the popover, which hopefully works.

annevk commented 5 hours ago

You cannot use testdriver with reftests? That seems like something that ought to be fixed if true.