Bug 223598 - Use Element for checking Settings in CSSComputedStyleDeclaration
Summary: Use Element for checking Settings in CSSComputedStyleDeclaration
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rob Buis
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-22 13:28 PDT by Rob Buis
Modified: 2021-04-01 03:36 PDT (History)
8 users (show)

See Also:


Attachments
Patch (9.53 KB, patch)
2021-03-26 02:44 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (8.98 KB, patch)
2021-03-26 06:00 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (17.95 KB, patch)
2021-03-31 11:53 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (18.61 KB, patch)
2021-04-01 00:16 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 Rob Buis 2021-03-22 13:28:05 PDT
Use Element for checking Settings in CSSComputedStyleDeclaration. Right now renderer is used but this will not work for non-rendered elements.
Comment 1 Rob Buis 2021-03-26 02:44:49 PDT
Created attachment 424338 [details]
Patch
Comment 2 Rob Buis 2021-03-26 06:00:34 PDT
Created attachment 424343 [details]
Patch
Comment 3 Radar WebKit Bug Importer 2021-03-29 13:29:11 PDT
<rdar://problem/75970166>
Comment 4 Darin Adler 2021-03-31 09:24:07 PDT
Comment on attachment 424343 [details]
Patch

8 changes, but only 2 tests; I’d expect we’d need 8 different tests to cover them all. Also, did these tests fail before making the change?
Comment 5 Rob Buis 2021-03-31 09:43:26 PDT
(In reply to Darin Adler from comment #4)
> Comment on attachment 424343 [details]
> Patch
> 
> 8 changes, but only 2 tests; I’d expect we’d need 8 different tests to cover
> them all. Also, did these tests fail before making the change?

Yes, I verified for aspect-ratio and contain. I missed that scroll-behaviour has a similar test. Should I adapt that one and add a similar if-disabled test for cssIndividualTransformProperties before landing?
Comment 6 Darin Adler 2021-03-31 10:23:31 PDT
Yes, I suggest we make sure we have a test for every change. I don’t feel strongly about how to structure those tests; one test, many, use existing tests that happen to have the correct set-up.
Comment 7 Rob Buis 2021-03-31 11:53:56 PDT
Created attachment 424795 [details]
Patch
Comment 8 Darin Adler 2021-03-31 18:18:24 PDT
Looks like some of the new tests are failing on Windows. Maybe DumpRenderTree doesn’t have the necessary support for the CSSIndividualTransformPropertiesEnabled=false on Windows?
Comment 9 Rob Buis 2021-04-01 00:16:55 PDT
Created attachment 424876 [details]
Patch
Comment 10 EWS 2021-04-01 03:26:42 PDT
Committed r275351: <https://commits.webkit.org/r275351>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424876 [details].
Comment 11 Rob Buis 2021-04-01 03:36:30 PDT
(In reply to Darin Adler from comment #8)
> Looks like some of the new tests are failing on Windows. Maybe
> DumpRenderTree doesn’t have the necessary support for the
> CSSIndividualTransformPropertiesEnabled=false on Windows?

Yes, this happened before with the aspect-ratio test, it was marked as failure. Looks like the bug on Windows is that experimental features that are enabled by default can't be disabled for testing. I will leave this to the windows experts.