Bug 240087 - [Apple Pay] REGRESSION(r291588): `appearance: -apple-pay-button` doesn't work with `border-width: 0`
Summary: [Apple Pay] REGRESSION(r291588): `appearance: -apple-pay-button` doesn't work...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on: 238035
Blocks:
  Show dependency treegraph
 
Reported: 2022-05-04 15:31 PDT by Devin Rousso
Modified: 2022-05-04 21:58 PDT (History)
10 users (show)

See Also:


Attachments
Patch (6.51 KB, patch)
2022-05-04 15:34 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (6.53 KB, patch)
2022-05-04 17:21 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2022-05-04 15:31:24 PDT
.
Comment 1 Devin Rousso 2022-05-04 15:31:55 PDT
<rdar://problem/92559095>
Comment 2 Devin Rousso 2022-05-04 15:34:35 PDT
Created attachment 458830 [details]
Patch
Comment 3 Kate Cheney 2022-05-04 15:44:11 PDT
Comment on attachment 458830 [details]
Patch

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

> Source/WebCore/rendering/RenderTheme.cpp:123
> +static bool isApperanceAllowedForAllElements(ControlPart part)

nit: Appearance

> Source/WebCore/rendering/RenderTheme.cpp:127
> +        return true;

can this be: return part == ApplePayButtonPart?
Comment 4 Devin Rousso 2022-05-04 15:46:49 PDT
Comment on attachment 458830 [details]
Patch

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

>> Source/WebCore/rendering/RenderTheme.cpp:123
>> +static bool isApperanceAllowedForAllElements(ControlPart part)
> 
> nit: Appearance

blargh i make this mistake so often T.T

>> Source/WebCore/rendering/RenderTheme.cpp:127
>> +        return true;
> 
> can this be: return part == ApplePayButtonPart?

yes it could be that, but I personally prefer how it's written as it means we can avoid having an `#else`, which I find more annoying to have to read than this (which would look identical if the `#if` was removed)

I can change it if you feel very strongly tho :)
Comment 5 Kate Cheney 2022-05-04 15:50:27 PDT
Comment on attachment 458830 [details]
Patch

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

>>> Source/WebCore/rendering/RenderTheme.cpp:127
>>> +        return true;
>> 
>> can this be: return part == ApplePayButtonPart?
> 
> yes it could be that, but I personally prefer how it's written as it means we can avoid having an `#else`, which I find more annoying to have to read than this (which would look identical if the `#if` was removed)
> 
> I can change it if you feel very strongly tho :)

I don't feel strongly :P
Comment 6 Aditya Keerthi 2022-05-04 15:56:59 PDT
Comment on attachment 458830 [details]
Patch

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

> LayoutTests/fast/css/appearance-apple-pay-button-border-width.html:5
> +    apperance: apple-pay-button;

Spelling here too :)
Comment 7 Aditya Keerthi 2022-05-04 15:58:21 PDT
(In reply to Aditya Keerthi from comment #6)
> Comment on attachment 458830 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=458830&action=review
> 
> > LayoutTests/fast/css/appearance-apple-pay-button-border-width.html:5
> > +    apperance: apple-pay-button;
> 
> Spelling here too :)

And `apple-pay-button` -> `-apple-pay-button`?
Comment 8 Devin Rousso 2022-05-04 17:21:44 PDT
Created attachment 458837 [details]
Patch
Comment 9 EWS 2022-05-04 21:58:09 PDT
Committed r293820 (250293@main): <https://commits.webkit.org/250293@main>

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