Bug 263968 - Remove SwitchThumbElement and SwitchTrackElement
Summary: Remove SwitchThumbElement and SwitchTrackElement
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Anne van Kesteren
URL:
Keywords: InRadar
Depends on:
Blocks: 259380
  Show dependency treegraph
 
Reported: 2023-10-31 07:57 PDT by Anne van Kesteren
Modified: 2024-01-25 01:09 PST (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Anne van Kesteren 2023-10-31 07:57:14 PDT
Antti tells me internal element subclasses are an anti-pattern. And given how we have been slowly shifting the approach we might not need them after all.
Comment 1 Radar WebKit Bug Importer 2023-11-07 06:58:13 PST
<rdar://problem/118056124>
Comment 2 Tim Nguyen (:ntim) 2023-11-19 12:28:30 PST
They aren't necessary if you're not using any of the resolveCustomStyle stuff.

See: https://searchfox.org/wubkat/rev/114aa2c8594807cbc34dd5dff48a9e2addfd1e00/Source/WebCore/html/ColorInputType.cpp#148,155 on how you can create a <div> with a PseudoId associated to it.
Comment 3 Anne van Kesteren 2023-12-09 07:43:17 PST
Note that I'm not entirely sure it's possible to remove these. We probably need resolveCustomStyle() to set

    grid-area: 1/1;

conditionally.
Comment 4 Tim Nguyen (:ntim) 2023-12-09 08:31:13 PST
(In reply to Anne van Kesteren from comment #3)
> Note that I'm not entirely sure it's possible to remove these. We probably
> need resolveCustomStyle() to set
> 
>     grid-area: 1/1;
> 
> conditionally.

The problem here is that WebKit is using display: inline-grid for a form control in the first place. Changing the display value should not break the form control layout. (Fwiw, WebKit has a similar issue with date inputs and display: inline-flex)
Comment 5 Anne van Kesteren 2023-12-10 23:16:37 PST
Could you elaborate on that? It's not clear to me how that works for appearance:auto. I can see someone being able to change the outer display value, but the inner display value has to be grid, no? Perhaps for appearance:auto that part has to be !important.
Comment 6 Tim Nguyen (:ntim) 2023-12-11 19:52:32 PST
(In reply to Anne van Kesteren from comment #5)
> Could you elaborate on that? It's not clear to me how that works for
> appearance:auto. I can see someone being able to change the outer display
> value, but the inner display value has to be grid, no? Perhaps for
> appearance:auto that part has to be !important.

This seems to be set on the web exposed element:

https://searchfox.org/wubkat/rev/3943142d2ef1777fc8977e6158eb10e053c3f8ca/Source/WebCore/css/htmlSwitchControl.css#27-30

We shouldn't be able to break the layout of the switch by setting a different display type on the input itself.
Comment 7 Anne van Kesteren 2024-01-12 10:04:26 PST
Given the approach taken in bug 267334 I've become more convinced we can remove these elements. For a future appearance:base we could use StyleAppearance::BaseThumb or some such and some corresponding adjustBaseThumbStyle() method. Or reconcile it with SwitchThumb and branch on the host. Various options.
Comment 8 Anne van Kesteren 2024-01-20 02:46:10 PST
Pull request: https://github.com/WebKit/WebKit/pull/23013
Comment 9 EWS 2024-01-20 04:53:40 PST
Committed 273262@main (75760f40e328): <https://commits.webkit.org/273262@main>

Reviewed commits have been landed. Closing PR #23013 and removing active labels.
Comment 10 EWS 2024-01-25 01:09:31 PST
Committed 272448.356@safari-7618-branch (2e9a891e4ccc): <https://commits.webkit.org/272448.356@safari-7618-branch>

Reviewed commits have been landed. Closing PR #1038 and removing active labels.