-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
getComputedStyle() on transform property should return function list #4591
getComputedStyle() on transform property should return function list #4591
Conversation
EWS run on previous version of this PR (hash cd50a8f) |
The two regressions on @anttijk do you think this is an acceptable change? It really only manifests itself through the computed style. We should add an allow-list for |
cd50a8f
to
84f06ce
Compare
EWS run on current version of this PR (hash 84f06ce) |
FAIL KeyframeEffect.getKeyframes() returns expected values for animations with CSS variables as keyframe values assert_equals: value for 'transform' on Keyframe #1 should match expected "translate(100px)" but got "matrix(1, 0, 0, 1, 100, 0)" | ||
FAIL KeyframeEffect.getKeyframes() returns expected values for animations with CSS variables as keyframe values assert_equals: value for 'transform' on Keyframe #1 should match expected "translate(100px)" but got "translate(100px, 0px)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still not entirely canonical?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, there seems to be some differences between expected serialization that I haven't fully worked out. Fixing this is the next item on my to-do list.
84f06ce
to
914e153
Compare
https://bugs.webkit.org/show_bug.cgi?id=23924 Reviewed by Antti Koivisto. We should serialize the "transform" property to a series of individual functions to match the individual operations found in the TransformOperations value. To determine when we should be doing this, we pass a nullptr value as the renderer when calling valueForPropertyInStyle() on the ComputedStyleExtractor in KeyframeEffect::getKeyframes() and WebAnimation::commitStyles(). Note the slight regression with first-letter and first-line tests where we now expose the "transform" property not being rejected at parse-time, bug 245523 will address this. * LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-pseudo/first-letter-allowed-properties-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-pseudo/first-line-allowed-properties-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-transforms/animation/transform-interpolation-computed-value-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-transforms/animation/transform-interpolation-inline-value-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animation/commitStyles-expected.txt: * Source/WebCore/animation/KeyframeEffect.cpp: (WebCore::KeyframeEffect::getKeyframes): * Source/WebCore/animation/WebAnimation.cpp: (WebCore::WebAnimation::commitStyles): * Source/WebCore/css/ComputedStyleExtractor.cpp: (WebCore::computedTransform): Canonical link: https://commits.webkit.org/254760@main
914e153
to
ae44d0c
Compare
Committed 254760@main (ae44d0c): https://commits.webkit.org/254760@main Reviewed commits have been landed. Closing PR #4591 and removing active labels. |
ae44d0c
84f06ce
π mac-debugπ tvπ§ͺ mac-wk1π tv-simπ watchπ§ͺ mac-AS-debug-wk2π watch-sim