Bug 216978

Summary:

REGRESSION (r260276): Transform `scale()` removes round corners declared with `border-radius` on a hover state in Safari

Product:

WebKit

Reporter:

alehm

Component:

Compositing

Assignee:

Cameron McCormack (:heycam) <heycam>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

alehm, brendan, changseok, darin, esprehn+autocc, ews-watchlist, fred.wang, glenn, heycam, kondapallykalyan, pdr, simon.fraser, webkit-bug-importer, zalan

Priority:

Keywords:

InRadar

Version:

Safari 14

Hardware:

Mac

OS:

macOS 10.15

See Also:

https://bugs.webkit.org/show_bug.cgi?id=140535

Attachments:

Description	Flags
Testcase	none
Testcase without :hover	none
Patch	none
Patch	none
Patch	none

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