Bug 224028 - aspect-ratio not recomputed on hover
Summary: aspect-ratio not recomputed on hover
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rob Buis
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-31 17:19 PDT by Simon Fraser (smfr)
Modified: 2021-04-02 12:03 PDT (History)
13 users (show)

See Also:


Attachments
Patch (1.29 KB, patch)
2021-04-01 00:55 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (4.21 KB, patch)
2021-04-01 08:58 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (4.44 KB, patch)
2021-04-01 11:27 PDT, Rob Buis
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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