LayoutTests/animations/interpolation/font-size-adjust-interpolation.html - Issue 983073002: Make font-size-adjust animatable.

Side by Side Diff: LayoutTests/animations/interpolation/font-size-adjust-interpolation.html

Issue 983073002: Make font-size-adjust animatable. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master

Patch Set: Do not allow font-size-adjust to animate for none or 0 Created 5 years, 9 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

	2 <style type="text/css">

	3 .container {

	4 font-size: 20px;

	5 line-height: 1;

	6 }

	7

	8 @font-face {

	9 font-family : 'testFont';

	10 src : url('../../resources/opensans/OpenSans-Regular.woff') format("woff");

	11 }

	12

	13 .target {

	14 display: inline-block;

	15 font-family: testFont;

	16 }

	17

	18 .replica {

	19 color: green;

	20 margin-right: 30px;

	21 }

	22 </style>

	23

	24 <template id="target-template">

	25 <span class="container">

	26 <div class="target">x</span>

	27 </div>

	28 </template>

	29 <script src="resources/interpolation-test.js"></script>

	30 <script>

	31 assertInterpolation({

	32 property: 'font-size-adjust',

	33 from: '0',

	34 to: '1.2'

	35 }, [

	36 {at: -2, is: 'none'},
	dstockwell 2015/03/16 21:49:31 Why can't we interpolate smoothly from 0 to 1.2? Why can't we interpolate smoothly from 0 to 1.2? changseok 2015/03/17 07:12:58 Because '0' for font-size-adjust is same meaning w Show quoted text On 2015/03/16 21:49:31, dstockwell wrote: > Why can't we interpolate smoothly from 0 to 1.2? Because '0' for font-size-adjust is same meaning with 'none'. This is following behavior to the gecko. It treats them as a same value and It doesn't animate for 0 nor none if any frame has the values for font-size-adjust. My first patch allowed font-size-adjust for '0' to animate though, I changed it not to animate after https://codereview.chromium.org/983073002/#msg11. dstockwell 2015/03/17 07:42:10 This sounds like a bug in gecko, we should clarify Show quoted text On 2015/03/17 at 07:12:58, changseok wrote: > On 2015/03/16 21:49:31, dstockwell wrote: > > Why can't we interpolate smoothly from 0 to 1.2? > > Because '0' for font-size-adjust is same meaning with 'none'. This is following behavior to the gecko. It treats them as a same value and It doesn't animate for 0 nor none if any frame has the values for font-size-adjust. My first patch allowed font-size-adjust for '0' to animate though, I changed it not to animate after https://codereview.chromium.org/983073002/#msg11. This sounds like a bug in gecko, we should clarify the behavior with the CSS working group before blindly following -- given gecko is the only other implementation. If we must treat 0 the same as none, you can use AnimatableDouble::InterpolationIsNonContinuousWithZero in AnimatedStyleBuilder and forceDefaultStyleInterpolation in StringKeyframe to prevent interpolating other numbers with 0. changseok 2015/03/17 11:35:06 O.K Show quoted text On 2015/03/17 07:42:10, dstockwell wrote: > On 2015/03/17 at 07:12:58, changseok wrote: > > On 2015/03/16 21:49:31, dstockwell wrote: > > > Why can't we interpolate smoothly from 0 to 1.2? > > > > Because '0' for font-size-adjust is same meaning with 'none'. This is > following behavior to the gecko. It treats them as a same value and It doesn't > animate for 0 nor none if any frame has the values for font-size-adjust. My > first patch allowed font-size-adjust for '0' to animate though, I changed it not > to animate after https://codereview.chromium.org/983073002/#msg11. > > This sounds like a bug in gecko, we should clarify the behavior with the CSS > working group before blindly following -- given gecko is the only other > implementation. O.K Show quoted text > If we must treat 0 the same as none, you can use > AnimatableDouble::InterpolationIsNonContinuousWithZero in AnimatedStyleBuilder > and forceDefaultStyleInterpolation in StringKeyframe to prevent interpolating > other numbers with 0. I don't have a strong opinion whether or not we should treat '0' as none. But if it should be, I'd like to cover it in a seperated bug later. For now, let's treat 0 as 'none' as gecko does. dstockwell 2015/03/17 23:36:46 OK, but please file the bug and start the discussi Show quoted text On 2015/03/17 at 11:35:06, changseok wrote: > On 2015/03/17 07:42:10, dstockwell wrote: > > On 2015/03/17 at 07:12:58, changseok wrote: > > > On 2015/03/16 21:49:31, dstockwell wrote: > > > > Why can't we interpolate smoothly from 0 to 1.2? > > > > > > Because '0' for font-size-adjust is same meaning with 'none'. This is > > following behavior to the gecko. It treats them as a same value and It doesn't > > animate for 0 nor none if any frame has the values for font-size-adjust. My > > first patch allowed font-size-adjust for '0' to animate though, I changed it not > > to animate after https://codereview.chromium.org/983073002/#msg11. > > > > This sounds like a bug in gecko, we should clarify the behavior with the CSS > > working group before blindly following -- given gecko is the only other > > implementation. > O.K > > > If we must treat 0 the same as none, you can use > > AnimatableDouble::InterpolationIsNonContinuousWithZero in AnimatedStyleBuilder > > and forceDefaultStyleInterpolation in StringKeyframe to prevent interpolating > > other numbers with 0. > I don't have a strong opinion whether or not we should treat '0' as none. But if it should be, I'd like to cover it in a seperated bug later. > For now, let's treat 0 as 'none' as gecko does. OK, but please file the bug and start the discussion with the CSS Working Group before landing this patch.
	37 {at: -0.3, is: 'none'},

	38 {at: 0, is: 'none'},

	39 {at: 0.3, is: 'none'},

	40 {at: 0.6, is: 'none'},

	41 {at: 1, is: 'none'},

	42 {at: 1.5, is: 'none'},

	43 ]);

	44

	45 assertInterpolation({

	46 property: 'font-size-adjust',

	47 from: 'none',

	48 to: '1.2'

	49 }, [

	50 {at: -2, is: 'none'},

	51 {at: -0.3, is: 'none'},

	52 {at: 0, is: 'none'},

	53 {at: 0.3, is: 'none'},

	54 {at: 0.6, is: 'none'},
	dstockwell 2015/03/16 21:49:31 This is incorrect, the values should be '1.2' at 0 This is incorrect, the values should be '1.2' at 0.6, 1 and 1.5. changseok 2015/03/17 07:12:58 With same reason above. if any single frame has '0 Show quoted text On 2015/03/16 21:49:31, dstockwell wrote: > This is incorrect, the values should be '1.2' at 0.6, 1 and 1.5. With same reason above. if any single frame has '0' or 'none' for font-size-adjust, it is intended not to allow animation for the property. Gecko works with the same way as if it doesn't know such animating property. Please run this test in gecko. dstockwell 2015/03/17 07:42:10 For values which do not smoothly interpolate, eg. Show quoted text On 2015/03/17 at 07:12:58, changseok wrote: > On 2015/03/16 21:49:31, dstockwell wrote: > > This is incorrect, the values should be '1.2' at 0.6, 1 and 1.5. > > With same reason above. if any single frame has '0' or 'none' for font-size-adjust, it is intended not to allow animation for the property. Gecko works with the same way as if it doesn't know such animating property. Please run this test in gecko. For values which do not smoothly interpolate, eg. none and '1.2' the result should step at 0.5. This behavior may not yet be implemented in Gecko but is consistent with all other animation in Blink. See the comments on non-interoperable values in http://dev.w3.org/csswg/css-animations-1/#wg-resolutions-pending changseok 2015/03/17 11:35:06 I see. Fixed as you said. Show quoted text On 2015/03/17 07:42:10, dstockwell wrote: > On 2015/03/17 at 07:12:58, changseok wrote: > > On 2015/03/16 21:49:31, dstockwell wrote: > > > This is incorrect, the values should be '1.2' at 0.6, 1 and 1.5. > > > > With same reason above. if any single frame has '0' or 'none' for > font-size-adjust, it is intended not to allow animation for the property. Gecko > works with the same way as if it doesn't know such animating property. Please > run this test in gecko. > > For values which do not smoothly interpolate, eg. none and '1.2' the result > should step at 0.5. This behavior may not yet be implemented in Gecko but is > consistent with all other animation in Blink. See the comments on > non-interoperable values in > http://dev.w3.org/csswg/css-animations-1/#wg-resolutions-pending I see. Fixed as you said.
	55 {at: 1, is: 'none'},

	56 {at: 1.5, is: 'none'},

	57 ]);

	58

	59 assertInterpolation({

	60 property: 'font-size-adjust',

	61 from: '0.2',

	62 to: '1.2'

	63 }, [

	64 {at: -2, is: 'none'}, // CSS font-size-adjust can't be negative.

	65 {at: -0.3, is: 'none'},

	66 {at: 0, is: '0.200000002980232'},
	dstockwell 2015/03/16 21:49:31 just '0.2' should work here, the helper performs r just '0.2' should work here, the helper performs rounding changseok 2015/03/17 07:12:58 Cool! Show quoted text On 2015/03/16 21:49:31, dstockwell wrote: > just '0.2' should work here, the helper performs rounding Cool!
	67 {at: 0.3, is: '0.5'},

	68 {at: 0.6, is: '0.800000011920929'},

	69 {at: 1, is: '1.20000004768372'},

	70 {at: 1.5, is: '1.70000004768372'},

	71 ]);

	72 </script>

OLD	NEW