Bug 240087

Summary:

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

Product:

WebKit

Reporter:

Devin Rousso <hi>

Component:

New Bugs

Assignee:

Devin Rousso <hi>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

akeerthi, changseok, esprehn+autocc, ews-watchlist, glenn, hi, katherine_cheney, kondapallykalyan, pdr, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

238035

Bug Blocks:

Attachments:

Description	Flags
Patch	none
Patch	none

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