Bug 236012 - [Forms] change the default appearance of button, checkbox etc. to 'auto'
Summary: [Forms] change the default appearance of button, checkbox etc. to 'auto'
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zsun
URL:
Keywords: InRadar
Depends on: 238739
Blocks: 240764
  Show dependency treegraph
 
Reported: 2022-02-02 05:06 PST by zsun
Modified: 2022-05-21 14:29 PDT (History)
13 users (show)

See Also:


Attachments
Patch (11.89 KB, patch)
2022-04-01 07:21 PDT, zsun
no flags Details | Formatted Diff | Diff
Patch (20.46 KB, patch)
2022-04-04 01:34 PDT, zsun
no flags Details | Formatted Diff | Diff
Patch (20.67 KB, patch)
2022-04-04 01:36 PDT, zsun
no flags Details | Formatted Diff | Diff
Patch (21.11 KB, patch)
2022-04-04 05:20 PDT, zsun
no flags Details | Formatted Diff | Diff
Patch (24.07 KB, patch)
2022-04-04 05:46 PDT, zsun
no flags Details | Formatted Diff | Diff
Patch (28.25 KB, patch)
2022-04-04 11:41 PDT, zsun
no flags Details | Formatted Diff | Diff
Patch (28.23 KB, patch)
2022-04-04 11:49 PDT, zsun
no flags Details | Formatted Diff | Diff
Patch (24.43 KB, patch)
2022-04-04 12:09 PDT, zsun
no flags Details | Formatted Diff | Diff
[fast-cq] Patch (24.43 KB, patch)
2022-04-04 12:10 PDT, zsun
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zsun 2022-02-02 05:06:23 PST
Affected WPT test 

imported/w3c/web-platform-tests/html/rendering/widgets/appearance/default-styles.html
Comment 1 Radar WebKit Bug Importer 2022-02-09 05:07:16 PST
<rdar://problem/88684058>
Comment 2 zsun 2022-04-01 07:21:41 PDT
Created attachment 456357 [details]
Patch
Comment 3 Tim Nguyen (:ntim) 2022-04-02 05:54:46 PDT
Comment on attachment 456357 [details]
Patch

According to EWS you need to delete: LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/html/rendering/widgets/appearance/default-styles-expected.txt


and rebaseline: LayoutTests/platform/ios-wk2/imported/w3c/web-platform-tests/html/rendering/widgets/appearance/default-styles-expected.txt (though why can't we use auto on iOS as well in the failing cases?)
Comment 4 zsun 2022-04-04 01:34:57 PDT
Created attachment 456548 [details]
Patch
Comment 5 zsun 2022-04-04 01:36:50 PDT
Created attachment 456549 [details]
Patch
Comment 6 Tim Nguyen (:ntim) 2022-04-04 03:55:20 PDT
Comment on attachment 456549 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=456549&action=review

Can https://webkit-search.igalia.com/webkit/rev/81b1b504044f756417974ca63b3992ee817bb619/Source/WebCore/css/html.css#759 be changed to `appearance: initial`?

Also, https://webkit-search.igalia.com/webkit/rev/81b1b504044f756417974ca63b3992ee817bb619/Source/WebCore/rendering/RenderTheme.cpp#333-440 suggests there's more in that UA stylesheet that can be switched over to `appearance: auto` (input::-webkit-caps-lock-indicator for instance, among many examples). Can you do that either here or in a new bug?

> Source/WebCore/css/html.css:500
> -    -webkit-appearance: textfield;
> +    appearance: auto;
>      align-items: center;
>      display: -webkit-inline-flex;
>      overflow: hidden;
>  #if defined(WTF_PLATFORM_IOS_FAMILY) && WTF_PLATFORM_IOS_FAMILY
> -    -webkit-appearance: menulist-button;
> +    appearance: auto;
>      border: 1px solid -webkit-control-background;
>      background-color: -apple-system-opaque-secondary-fill;
>      font-family: system-ui;
>      color: -apple-system-blue;
>      padding: 0.2em 0.5em;
>  #else
> -    -webkit-appearance: textfield;
> +    appearance: auto;

The last 2 are redundant, can you remove them?

> Source/WebCore/css/html.css:1048
> +    appearance: auto;

You can move this out of the #if / #else and remove this property and the one above.

> LayoutTests/platform/ios-wk2/imported/w3c/web-platform-tests/html/rendering/widgets/appearance/default-styles-expected.txt:17
>  FAIL <input type="color"> assert_equals: -webkit-appearance expected "auto" but got "color-well"

You should be able to get rid of this iOS test results file by changing: https://webkit-search.igalia.com/webkit/rev/81b1b504044f756417974ca63b3992ee817bb619/Source/WebCore/rendering/RenderThemeIOS.mm#2602 directly
Comment 7 zsun 2022-04-04 05:20:14 PDT
Created attachment 456560 [details]
Patch
Comment 8 zsun 2022-04-04 05:46:34 PDT
Created attachment 456561 [details]
Patch
Comment 9 Tim Nguyen (:ntim) 2022-04-04 06:31:04 PDT
Comment on attachment 456561 [details]
Patch

r=me if EWS is happy
Comment 10 Tim Nguyen (:ntim) 2022-04-04 08:34:38 PDT
The issue with fast/attachment/attachment-disabled-rendering.html is that `appearance: attachment` is unconditionally added (even when the <attachment> element is disabled), causing the `display` to become `inline-block` here: https://webkit-search.igalia.com/webkit/rev/81b1b504044f756417974ca63b3992ee817bb619/Source/WebCore/rendering/RenderTheme.cpp#128-136

The new behavior checks for HTMLAttachmentElement (which actually takes in consideration whether the <attachment> element is enabled or not), since it is disabled in the test, appearance: auto computes to none, and the codepath above is never hit, meaning the element stays `display: inline`.

The new behavior is more correct, so I suggest to simply rebaseline the results for that test.
Comment 11 Tim Nguyen (:ntim) 2022-04-04 08:49:20 PDT
(In reply to Tim Nguyen (:ntim) from comment #10)
> The issue with fast/attachment/attachment-disabled-rendering.html is that
> `appearance: attachment` is unconditionally added (even when the
> <attachment> element is disabled)

Filed https://bugs.webkit.org/show_bug.cgi?id=238739 for this.
Comment 12 Tim Nguyen (:ntim) 2022-04-04 11:17:01 PDT
Rebasing on top of bug 238739 should fix the remaining failures.
Comment 13 zsun 2022-04-04 11:41:29 PDT
Created attachment 456597 [details]
Patch
Comment 14 zsun 2022-04-04 11:49:21 PDT
Created attachment 456599 [details]
Patch
Comment 15 zsun 2022-04-04 12:09:48 PDT
Created attachment 456602 [details]
Patch
Comment 16 zsun 2022-04-04 12:10:48 PDT
Created attachment 456603 [details]
[fast-cq] Patch
Comment 17 EWS 2022-04-04 22:40:09 PDT
Committed r292376 (249241@main): <https://commits.webkit.org/249241@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 456603 [details].