Bug 240087 – [Apple Pay] REGRESSION(r291588): `appearance: -apple-pay-button` doesn't work with `border-width: 0`

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Devin Rousso]

.

[Devin Rousso]

<rdar://problem/92559095>

[Devin Rousso]

Created attachment 458830 [details]
Patch

[Kate Cheney]

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?

[Devin Rousso]

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 :)

[Kate Cheney]

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

[Aditya Keerthi]

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 :)

[Aditya Keerthi]

(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`?

[Devin Rousso]

Created attachment 458837 [details]
Patch

[EWS]

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].