Bug 223598

Summary: Use Element for checking Settings in CSSComputedStyleDeclaration
Product: WebKit Reporter: Rob Buis <rbuis>
Component: CSSAssignee: Rob Buis <rbuis>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, macpherson, menard, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Rob Buis
Reported 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.
Attachments
Patch (9.53 KB, patch)
2021-03-26 02:44 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (8.98 KB, patch)
2021-03-26 06:00 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (17.95 KB, patch)
2021-03-31 11:53 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (18.61 KB, patch)
2021-04-01 00:16 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Rob Buis
Comment 1 2021-03-26 02:44:49 PDT
Created attachment 424338 [details] Patch
Rob Buis
Comment 2 2021-03-26 06:00:34 PDT
Created attachment 424343 [details] Patch
Radar WebKit Bug Importer
Comment 3 2021-03-29 13:29:11 PDT
<rdar://problem/75970166>
Darin Adler
Comment 4 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?
Rob Buis
Comment 5 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?
Darin Adler
Comment 6 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.
Rob Buis
Comment 7 2021-03-31 11:53:56 PDT
Created attachment 424795 [details] Patch
Darin Adler
Comment 8 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?
Rob Buis
Comment 9 2021-04-01 00:16:55 PDT
Created attachment 424876 [details] Patch
EWS
Comment 10 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].
Rob Buis
Comment 11 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.
Note You need to log in before you can comment on or make changes to this bug.