There are a few properties left on keyframes (composite and easing) that we don't support yet as well as the getKeyframes() method to expose the parsed keyframes.
<rdar://problem/36772586>
Created attachment 332019 [details] Patch
Attachment 332019 [details] did not pass style-queue: ERROR: Source/WebCore/animation/KeyframeEffect.cpp:456: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] WARNING: This machine could support 4 simulators, but is only configured for 3. WARNING: Please see <https://trac.webkit.org/wiki/IncreasingKernelLimits>. Total errors found: 1 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 332034 [details] Patch
Attachment 332034 [details] did not pass style-queue: ERROR: Source/WebCore/animation/KeyframeEffect.cpp:456: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] WARNING: This machine could support 4 simulators, but is only configured for 3. WARNING: Please see <https://trac.webkit.org/wiki/IncreasingKernelLimits>. Total errors found: 1 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 332034 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=332034&action=review > Source/WebCore/ChangeLog:10 > + parsig the remaining properties that can be exposed on keyframes: "easing" and "composite". And since we parse parsig > Source/WebCore/animation/KeyframeEffect.cpp:82 > + for (size_t i = 0; i < keyframes.size(); ++i) { Use for : loop > Source/WebCore/platform/animation/TimingFunction.cpp:106 > +ExceptionOr<Ref<TimingFunction>> TimingFunction::createFromCSSText(const String& cssText) If this returned a RefPtr<TimingFunction> you wouldn't need the &foo.get() at the calling sites. > Source/WebCore/platform/animation/TimingFunction.cpp:172 > + if (function.x1() == 0.25 && function.y1() == 0.1 && function.x2() == 0.25 && function.y2() == 1.0) > + return "ease"; > + if (function.x1() == 0.42 && !function.y1() && function.x2() == 1.0 && function.y2() == 1.0) > + return "ease-in"; > + if (!function.x1() && !function.y1() && function.x2() == 0.58 && function.y2() == 1.0) > + return "ease-out"; > + if (function.x1() == 0.42 && !function.y1() && function.x2() == 0.58 && function.y2() == 1.0) > + return "ease-in-out"; Are these accurate enough? (no rounding errors?) > Source/WebCore/platform/animation/TimingFunction.cpp:180 > + if (m_type == TimingFunction::StepsFunction) { > + auto& function = downcast<StepsTimingFunction>(*this); > + if (!function.stepAtStart()) > + return String::format("steps(%d)", function.numberOfSteps()); > + } We don't do step-start or step-end?
(In reply to Dean Jackson from comment #6) > Comment on attachment 332034 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=332034&action=review > > > Source/WebCore/ChangeLog:10 > > + parsig the remaining properties that can be exposed on keyframes: "easing" and "composite". And since we parse > > parsig Will fix in commit. > > Source/WebCore/animation/KeyframeEffect.cpp:82 > > + for (size_t i = 0; i < keyframes.size(); ++i) { > > Use for : loop Will fix in commit. > > Source/WebCore/platform/animation/TimingFunction.cpp:106 > > +ExceptionOr<Ref<TimingFunction>> TimingFunction::createFromCSSText(const String& cssText) > > If this returned a RefPtr<TimingFunction> you wouldn't need the &foo.get() > at the calling sites. Will look into that. > > Source/WebCore/platform/animation/TimingFunction.cpp:172 > > + if (function.x1() == 0.25 && function.y1() == 0.1 && function.x2() == 0.25 && function.y2() == 1.0) > > + return "ease"; > > + if (function.x1() == 0.42 && !function.y1() && function.x2() == 1.0 && function.y2() == 1.0) > > + return "ease-in"; > > + if (!function.x1() && !function.y1() && function.x2() == 0.58 && function.y2() == 1.0) > > + return "ease-out"; > > + if (function.x1() == 0.42 && !function.y1() && function.x2() == 0.58 && function.y2() == 1.0) > > + return "ease-in-out"; > > Are these accurate enough? (no rounding errors?) Not that I have found in my testing. > > Source/WebCore/platform/animation/TimingFunction.cpp:180 > > + if (m_type == TimingFunction::StepsFunction) { > > + auto& function = downcast<StepsTimingFunction>(*this); > > + if (!function.stepAtStart()) > > + return String::format("steps(%d)", function.numberOfSteps()); > > + } > > We don't do step-start or step-end? We should! Will fix in commit.
Created attachment 332040 [details] Patch for landing
Created attachment 332042 [details] Patch for landing
Comment on attachment 332042 [details] Patch for landing Rejecting attachment 332042 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 332042, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in LayoutTests/ChangeLog contains OOPS!. Full output: http://webkit-queues.webkit.org/results/6184537
Committed r227428: <https://trac.webkit.org/changeset/227428>
On macOS WK1 the test http/wpt/web-animations/interfaces/AnimationEffectTiming/easing.html is failing consistently. It looks like the expectations changed for the test were not correct for this. Build: https://build.webkit.org/results/Apple%20High%20Sierra%20Release%20WK1%20(Tests)/r227428%20(2865)/results.html Diff: --- /Volumes/Data/slave/highsierra-release-tests-wk1/build/layout-test-results/http/wpt/web-animations/interfaces/AnimationEffectTiming/easing-expected.txt +++ /Volumes/Data/slave/highsierra-release-tests-wk1/build/layout-test-results/http/wpt/web-animations/interfaces/AnimationEffectTiming/easing-actual.txt @@ -1,11 +1,11 @@ PASS Test default value -FAIL step-start function assert_approx_equals: The progress of the animation should be approximately 1 at 0ms expected a number but got a "object" -FAIL steps(1, start) function assert_approx_equals: The progress of the animation should be approximately 1 at 0ms expected a number but got a "object" +FAIL step-start function assert_equals: expected "steps(1, start)" but got "step-start" +FAIL steps(1, start) function assert_equals: expected "steps(1, start)" but got "step-start" FAIL steps(2, start) function assert_approx_equals: The progress of the animation should be approximately 0.5 at 0ms expected a number but got a "object" -FAIL step-end function assert_approx_equals: The progress of the animation should be approximately 0 at 0ms expected a number but got a "object" -FAIL steps(1) function assert_approx_equals: The progress of the animation should be approximately 0 at 0ms expected a number but got a "object" -FAIL steps(1, end) function assert_approx_equals: The progress of the animation should be approximately 0 at 0ms expected a number but got a "object" +FAIL step-end function assert_equals: expected "steps(1)" but got "step-end" +FAIL steps(1) function assert_equals: expected "steps(1)" but got "step-end" +FAIL steps(1, end) function assert_equals: expected "steps(1)" but got "step-end" FAIL steps(2, end) function assert_approx_equals: The progress of the animation should be approximately 0 at 0ms expected a number but got a "object" FAIL frames function assert_approx_equals: The progress of the animation should be approximately 0 at 0ms expected a number but got a "object" FAIL linear function assert_approx_equals: The progress of the animation should be approximately 0 at 0ms expected a number but got a "object" Would this just need a rebaseline or is this something deeper?
(In reply to Matt Lewis from comment #12) Fixing it now.
Committed r227441: <https://trac.webkit.org/changeset/227441>