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 BugsAssignee: 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: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 238035    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch none

Devin Rousso
Reported 2022-05-04 15:31:24 PDT
.
Attachments
Patch (6.51 KB, patch)
2022-05-04 15:34 PDT, Devin Rousso 		no flags
DetailsFormatted DiffDiff
Patch (6.53 KB, patch)
2022-05-04 17:21 PDT, Devin Rousso 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Devin Rousso
Comment 1 2022-05-04 15:31:55 PDT
<rdar://problem/92559095>
Devin Rousso
Comment 2 2022-05-04 15:34:35 PDT
Created attachment 458830 [details] Patch
Kate Cheney
Comment 3 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?
Devin Rousso
Comment 4 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 :)
Kate Cheney
Comment 5 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
Aditya Keerthi
Comment 6 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 :)
Aditya Keerthi
Comment 7 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`?
Devin Rousso
Comment 8 2022-05-04 17:21:44 PDT
Created attachment 458837 [details] Patch
EWS
Comment 9 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].
Note You need to log in before you can comment on or make changes to this bug.