WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
238062
hasExplicitlySetBorderRadius logic is broken
https://bugs.webkit.org/show_bug.cgi?id=238062
Summary
hasExplicitlySetBorderRadius logic is broken
Oriol Brufau
Reported
2022-03-17 18:08:24 PDT
Bug 212476
added a hasExplicitlySetBorderRadius flag that tracks whether the radius is a non initial value. It doesn't work well, because border-radius is a shorthand with multiple longhands. <style> .apple-pay-button { -webkit-appearance: -apple-pay-button; } .square-corners { border-radius: 0px; } </style> <div class=container> <button class='apple-pay-button square-corners' style='border-bottom-left-radius: inherit'></button> <button class='apple-pay-button square-corners' style='border-bottom-right-radius: inherit'></button> <button class='apple-pay-button square-corners' style='border-top-left-radius: inherit'></button> <button class='apple-pay-button square-corners' style='border-top-right-radius: inherit'></button> </div> Result: all buttons have square corners, except the last one. Why? Because all the border-*-radius share the same flag, and the properties are applied in alphabetical order. So e.g. for the first button: 1. Applying border-bottom-left-radius:inherit sets the flag to the parent value, which is false. 2. Applying border-bottom-right-radius:0px sets the flag to true. 3. Applying border-top-left-radius:0px sets the flag to true. 4. Applying border-top-right-radius:0px sets the flag to true. The flag ends up being true. So e.g. for the first button: 1. Applying border-bottom-left-radius:0px sets the flag to true. 2. Applying border-bottom-right-radius:0px sets the flag to true. 3. Applying border-top-left-radius:0px sets the flag to true. 4. Applying border-top-right-radius:inherit sets the flag to the parent value, which is false. The flag ends up being false! Also, only BuilderCustom::applyInheritBorder*Radius and BuilderCustom::applyValueBorder*Radius set the flag. But BuilderFunctions::applyInitialBorder*Radius doesn't set it. This makes the test appearance-apple-pay-button-border-radius.html fail with my patch for
bug 236199
. IMO this flag doesn't make much sense, as mentioned in
bug 212476
it should probably be about cascade origins. Or rather than being a separate flag, it would probably be better to store it as an auto radius Length? But given the current state, and since I don't want to refactor the entire thing, as a band-aid I think it makes more sense to: - Have one flag for each longhand. - Let 'inherit' set it to true.
Attachments
Patch
(22.83 KB, patch)
2022-03-17 18:36 PDT
,
Oriol Brufau
no flags
Details
Formatted Diff
Diff
Patch
(25.31 KB, patch)
2022-03-17 19:30 PDT
,
Oriol Brufau
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Oriol Brufau
Comment 1
2022-03-17 18:36:33 PDT
Created
attachment 455055
[details]
Patch
Oriol Brufau
Comment 2
2022-03-17 19:30:17 PDT
Created
attachment 455059
[details]
Patch
Oriol Brufau
Comment 3
2022-03-18 08:01:13 PDT
PTAL.
EWS
Comment 4
2022-03-19 19:19:13 PDT
Committed
r291536
(
248642@main
): <
https://commits.webkit.org/248642@main
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 455059
[details]
.
Radar WebKit Bug Importer
Comment 5
2022-03-19 19:20:17 PDT
<
rdar://problem/90531307
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug