There is no FIXME comment for this property, but it should be animatable too. Oh interesting. There is an implementation of Interpolate trait for border-spacing, but animatable flag is set to False.

Jeremy, would you like to fix this?

Sure, take it from here.

Thank you for taking this. Please add some web-platform test case like other making property animatable bugs.

(In reply to Hiroyuki Ikezoe (:hiro) from comment #4) > Thank you for taking this. Please add some web-platform test case like > other making property animatable bugs. Oops, I thought we had tests already... :( I'll write some in a separate patch then. Thank you for the pointers.

Comment on attachment 8861334 [details] Bug 1354437 - stylo: Make border-spacing animatable. https://reviewboard.mozilla.org/r/133316/#review136138 LGTM.

try with this wip: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7fb624e3991e, looks like we could reduce couple mochitest failures here (from 145 to 129). :)

Comment on attachment 8861795 [details] Bug 1354437 - enable border-spacing interpolation test. Hi Hiro, I did referred to other animation bugs and wrote this WIP. However, I've no idea how to verify locally if the test is indeed enabled. Could you enlighten me a bit about how to get this done? Thanks.

You can run testing/web-platform/tests/web-animations/animation-model/animation-types/interpolation-per-property.html locally.

Comment on attachment 8861795 [details] Bug 1354437 - enable border-spacing interpolation test. Clear request for now, since I know how to verify the patch and it doesn't pass the test... :(

Yeah, actually it's not 'discrete' type animation. We should add lengthPairType or something.

(In reply to Hiroyuki Ikezoe (:hiro) from comment #13) > Yeah, actually it's not 'discrete' type animation. We should add > lengthPairType or something. Yeah, that's the reason why I need a way to do the local verification, to check if I'm not going to the wrong direction. I do add a type with exactly same name as you said (high five). Will ask for review very soon!!!! Patch on the way~

Comment on attachment 8861795 [details] Bug 1354437 - enable border-spacing interpolation test. https://reviewboard.mozilla.org/r/133798/#review136754 Thanks for quick fix. Note that positionType is similar to this lengthPair. We can reuse this lengthPair for positionType. (See lengthPercentageOrCals for example). Please feel free open a bug or fix it in later patch. Thank you! ::: testing/web-platform/tests/web-animations/animation-model/animation-types/property-types.js:135 (Diff revision 2) > + [{ time: 0, expected: '10px 10px' }, > + { time: 500, expected: '30px 30px' }, > + { time: 1000, expected: '50px 50px' }]); In case of interpolation we just need to check at 500ms. ::: testing/web-platform/tests/web-animations/animation-model/animation-types/property-types.js:146 (Diff revision 2) > + [{ time: 0, expected: '10px 10px' }, > + { time: 500, expected: '30px 30px' }, > + { time: 1000, expected: '50px 50px' }]); Likewise here. ::: testing/web-platform/tests/web-animations/animation-model/animation-types/property-types.js:156 (Diff revision 2) > + > + testAddition: function(property, setup) { > + test(function(t) { > + var idlName = propertyToIDL(property); > + var target = createTestElement(t, setup); > + target.style[idlName] = '10px'; This should be '10px 10px'. ::: testing/web-platform/tests/web-animations/animation-model/animation-types/property-types.js:165 (Diff revision 2) > + }, property + ': length pair'); > + > + test(function(t) { > + var idlName = propertyToIDL(property); > + var target = createTestElement(t, setup); > + target.style[idlName] = '1rem'; Likewise here.

These new test should be passed on normal gecko, please run it locally (or try) before landing.

Comment on attachment 8861795 [details] Bug 1354437 - enable border-spacing interpolation test. https://reviewboard.mozilla.org/r/133798/#review136754 Ok, filed Bug 1359786 for this, so we won't block the stylo work here. > In case of interpolation we just need to check at 500ms. Ok, good to know. > This should be '10px 10px'. Hmmm, should've got this. Will fix this in the next version.

(In reply to Hiroyuki Ikezoe (:hiro) from comment #18) > These new test should be passed on normal gecko, please run it locally (or > try) before landing. Okay, I'll run a more thorough try before landing.

Comment on attachment 8861928 [details] Bug 1354437 - update mochitest expectations for border-spacing animation support. https://reviewboard.mozilla.org/r/133940/#review136818 This patch is mainly based on a previous try https://treeherder.mozilla.org/#/jobs?repo=try&revision=7fb624e3991e&selectedJob=93987427.

try looks fine except for few known intermittents: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a4695ec92ece create servo PR: https://github.com/servo/servo/pull/16624

Note that MANIFEST.json will be conflicted on autoland since bug 1356162 landed.

(In reply to Hiroyuki Ikezoe (:hiro) from comment #26) > Note that MANIFEST.json will be conflicted on autoland since bug 1356162 > landed. Oops, then I might need to rebase on top of that change... thanks for the notice.

Comment on attachment 8861928 [details] Bug 1354437 - update mochitest expectations for border-spacing animation support. https://reviewboard.mozilla.org/r/133940/#review137122 ::: layout/style/test/stylo-failures.md:78 (Diff revision 1) > * test_bug413958.html `monitorConsole` [3] > * test_parser_diagnostics_unprintables.html [550] > * Transition support: > * test_compute_data_with_start_struct.html `transition` [2] > * test_transitions.html: pseudo elements [10] > - * test_transitions_computed_value_combinations.html [145] > + * test_transitions_computed_value_combinations.html [129] Thanks for updating this. I just landed a patch to remove this, so I think you can drop this commit after rebasing.

(In reply to Boris Chiou [:boris] from comment #28) > Comment on attachment 8861928 [details] > Bug 1354437 - update mochitest expectations for border-spacing animation > support. > > https://reviewboard.mozilla.org/r/133940/#review137122 > > ::: layout/style/test/stylo-failures.md:78 > (Diff revision 1) > > * test_bug413958.html `monitorConsole` [3] > > * test_parser_diagnostics_unprintables.html [550] > > * Transition support: > > * test_compute_data_with_start_struct.html `transition` [2] > > * test_transitions.html: pseudo elements [10] > > - * test_transitions_computed_value_combinations.html [145] > > + * test_transitions_computed_value_combinations.html [129] > > Thanks for updating this. I just landed a patch to remove this, so I think > you can drop this commit after rebasing. Nice!!! \o/ Will drop this then.

Servo PR is merged to autoland: https://hg.mozilla.org/integration/autoland/rev/3a18fa64eff9

Pushed by jichen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c1d8fb6bbb76 enable border-spacing interpolation test. r=hiro