Bug 216978 - REGRESSION (r260276): Transform `scale()` removes round corners declared with `border-radius` on a hover state in Safari
Summary: REGRESSION (r260276): Transform `scale()` removes round corners declared with...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Compositing (show other bugs)
Version: Safari 14
Hardware: Mac macOS 10.15
: P2 Normal
Assignee: Cameron McCormack (:heycam)
URL:
Keywords: InRadar
: 223507 (view as bug list)
Depends on:
Blocks:
 
Reported: 2020-09-25 09:28 PDT by alehm
Modified: 2021-05-13 16:19 PDT (History)
14 users (show)

See Also:


Attachments
Testcase (1.46 KB, text/html)
2020-09-29 14:25 PDT, Simon Fraser (smfr)
no flags Details
Testcase without :hover (581 bytes, text/html)
2021-04-23 00:02 PDT, Cameron McCormack (:heycam)
no flags Details
Patch (20.84 KB, patch)
2021-05-11 00:15 PDT, Cameron McCormack (:heycam)
no flags Details | Formatted Diff | Diff
Patch (6.73 KB, patch)
2021-05-11 23:49 PDT, Cameron McCormack (:heycam)
no flags Details | Formatted Diff | Diff
Patch (6.60 KB, patch)
2021-05-13 15:36 PDT, Cameron McCormack (:heycam)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description alehm 2020-09-25 09:28:26 PDT
Overview:
Please take a look at the demo: https://jsfiddle.net/0fpho21q/2/

Steps to Reproduce:
1. Open the link above.
2. Hover over the first button.
3. Hover over the second button.

Actual Results: 
Round corners are removed from the first button.

Expected Results:
Round corners should be kept on a hover state, i.e. observe behavior of the second button.

Build Date & Hardware: Date and hardware of the build in which you first encountered the bug.
Safari Version 14.0 (15610.1.28.1.9, 15610), macOS Catalina Version 10.15.6

Additional Builds and Platforms:
Works as intended in Chrome.
Comment 1 Radar WebKit Bug Importer 2020-09-27 10:13:11 PDT
<rdar://problem/69660229>
Comment 2 Simon Fraser (smfr) 2020-09-29 14:25:36 PDT
Created attachment 410052 [details]
Testcase
Comment 3 Simon Fraser (smfr) 2020-09-29 14:26:36 PDT
This is about clip rects and compositing.
Comment 4 Simon Fraser (smfr) 2020-10-20 16:24:16 PDT
This is because PaintLayerFlag::PaintingOverflowContents is set so we incorrectly ignore ore the clip.
Comment 5 Simon Fraser (smfr) 2020-10-20 16:52:22 PDT
... and the transform makes a layer a clipRectsContext.rootLayer
Comment 6 Brendan Donovan 2021-03-20 03:12:53 PDT
*** Bug 223507 has been marked as a duplicate of this bug. ***
Comment 7 Cameron McCormack (:heycam) 2021-04-23 00:02:20 PDT
Created attachment 426891 [details]
Testcase without :hover

Just the bad sub-test.
Comment 8 Cameron McCormack (:heycam) 2021-05-11 00:15:13 PDT
Created attachment 428248 [details]
Patch
Comment 9 Cameron McCormack (:heycam) 2021-05-11 23:49:41 PDT
Created attachment 428348 [details]
Patch
Comment 10 Darin Adler 2021-05-12 20:47:06 PDT
Comment on attachment 428348 [details]
Patch

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

It’s possible that one of the more layout-savvy people wants to look this over too, but this looks obviously right to me.

> Source/WebCore/rendering/RenderLayer.cpp:4495
> +        // FIXME: Do we need to set the posClipRect to be affected by border-radius too?

Seems like our job is to come up with a test case for this so we don’t have to leave a question behind in the code.

> LayoutTests/fast/layers/overflow-scroll-transform-border-radius.html:5
> +    <div id="e" style="width: 50px; height: 50px; background-color: blue;"></div>

Not sure we need the id property here.
Comment 11 Cameron McCormack (:heycam) 2021-05-13 14:56:44 PDT
(In reply to Darin Adler from comment #10)
> > Source/WebCore/rendering/RenderLayer.cpp:4495
> > +        // FIXME: Do we need to set the posClipRect to be affected by border-radius too?
> 
> Seems like our job is to come up with a test case for this so we don’t have
> to leave a question behind in the code.

I tried and failed to make a test case that needed this the other day.  Now trying the "assert if we look at this state and run the whole test suite" technique.  Will remove the comment if nothing comes up.
Comment 12 Cameron McCormack (:heycam) 2021-05-13 15:36:54 PDT
Created attachment 428569 [details]
Patch
Comment 13 EWS 2021-05-13 16:19:56 PDT
Committed r277462 (237705@main): <https://commits.webkit.org/237705@main>

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