Bug 224028

Summary: aspect-ratio not recomputed on hover
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: Layout and RenderingAssignee: Rob Buis <rbuis>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, changseok, clopez, esprehn+autocc, ews-watchlist, glenn, kondapallykalyan, pdr, rbuis, simon.fraser, webkit-bug-importer, youennf, zalan
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 226673    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Simon Fraser (smfr) 2021-03-31 17:19:50 PDT
https://codepen.io/smfr/pen/abppGyQ

Hover the image. aspect-ratio should change but does not.
Comment 1 Rob Buis 2021-04-01 00:55:07 PDT
Created attachment 424878 [details]
Patch
Comment 2 Rob Buis 2021-04-01 08:58:17 PDT
Created attachment 424899 [details]
Patch
Comment 3 EWS Watchlist 2021-04-01 08:59:16 PDT
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment 4 Simon Fraser (smfr) 2021-04-01 10:55:33 PDT
Comment on attachment 424899 [details]
Patch

Don't use hover in the test. A test with a simple class change should be enough.
Comment 5 Rob Buis 2021-04-01 11:27:30 PDT
Created attachment 424916 [details]
Patch
Comment 6 zalan 2021-04-01 11:53:32 PDT
Comment on attachment 424916 [details]
Patch

It's a pity that this will trigger layout on renderers where the property is not applicable.
Comment 7 Simon Fraser (smfr) 2021-04-01 13:44:08 PDT
Comment on attachment 424916 [details]
Patch

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

> LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/replaced-element-dynamic-aspect-ratio.html:7
> +.applyAspectRatio {

this could be body.changed #aspectRatio {

> LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/replaced-element-dynamic-aspect-ratio.html:12
> +<body onload="aspectRatio.classList.add('applyAspectRatio')">

This could be document.body.classList.add('changed')
Comment 8 EWS 2021-04-01 13:52:45 PDT
Committed r275377: <https://commits.webkit.org/r275377>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424916 [details].
Comment 9 Radar WebKit Bug Importer 2021-04-01 13:53:27 PDT
<rdar://problem/76119937>
Comment 10 Rob Buis 2021-04-01 13:58:01 PDT
Comment on attachment 424916 [details]
Patch

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

>> LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/replaced-element-dynamic-aspect-ratio.html:7
>> +.applyAspectRatio {
> 
> this could be body.changed #aspectRatio {

I was not aware changed existed.

>> LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/replaced-element-dynamic-aspect-ratio.html:12
>> +<body onload="aspectRatio.classList.add('applyAspectRatio')">
> 
> This could be document.body.classList.add('changed')

Thanks, I will fix the test when upstreaming to WPT tomorrow and then we can re-import it.
Comment 11 Simon Fraser (smfr) 2021-04-01 15:03:09 PDT
(In reply to Rob Buis from comment #10)
> Comment on attachment 424916 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=424916&action=review
> 
> >> LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/replaced-element-dynamic-aspect-ratio.html:7
> >> +.applyAspectRatio {
> > 
> > this could be body.changed #aspectRatio {
> 
> I was not aware changed existed.

'changed' is just an arbitrary classname. The simplicity is that you can just set a class on the body and write the appropriate selectors, instead of targeting the element from script.
Comment 12 Rob Buis 2021-04-02 12:03:31 PDT
(In reply to Simon Fraser (smfr) from comment #11)
> > I was not aware changed existed.
> 
> 'changed' is just an arbitrary classname. The simplicity is that you can
> just set a class on the body and write the appropriate selectors, instead of
> targeting the element from script.

Got it. I ended up importing the improved WPT test here:
https://bugs.webkit.org/show_bug.cgi?id=222266