I can own this.

Hi,

I am interested in this as a first bug.

alpercakan98@ - awesome! :)

Please take a look at the following docs to help get yourself setup:
  - Get the Code: Checkout, Build, & Run Chromium: https://www.chromium.org/developers/how-tos/get-the-code
  - Contribution Code to Chromium: http://dev.chromium.org/developers/contributing-code

After that there are some instructions in #1 for what needs doing to fix this. Let me know if I can provide additional help.

smcgruer@

I have sent my attempt, but I am not sure if you got a notification because the page says change is WIP and there won't be email notifications. So, I am pinging from here.

Strangely, I could not make the tests work for CSS animations and Web Animations. However, the tests pass for the transitions. I looked around a little bit in the code, but I could not find a reason. Aren't all 3 of them using the same interpolation code?

Your attempt https://chromium-review.googlesource.com/c/chromium/src/+/956462
seems reasonable. I'll need to check more closely.

Thanks alphercakan90@, I'm taking a look now.

Regarding the WIP status, do you see a 'Start Review' button when you visit https://chromium-review.googlesource.com/c/chromium/src/+/956462 ? If so, please click it.

Alternatively, I've been told that sometimes there's an option in More -> WIP that can make a change no longer WIP.

Deleted: wMMKfrQvQO9.png 116 KB

	wMMKfrQvQO9.png 116 KB View Download

smcgruer@
Clicked "start review". Thanks.

I have tried some debugging, and found this but I am not if it is relevant since I am not familiar with the code base:

- CSSAnimations::CalculateTransitionUpdateForProperty calls CSSInterpolationType::MaybeConvertUnderlyingValue and if it returns null, it skips the transpolation

- CSSInterpolationType::MaybeConvertUnderlyingValue returns the return value of a call to CSSInterpolationType::MaybeConvertValue

- CSSNumberInterpolationType::MaybeConvertValue returns null if ToCSSPrimitiveValue(value).IsNumber() is false. I could not find the definition of ToCSSPrimitiveValue, but I am guessing it returns false for TextSizeAdjust. I guess ToCSSPrimitiveValue(value).IsPercentage() returns true.

- Since we are using CSSNumberInterpolationType for TextSizeAdjust, the problem might be that. I guess TextSizeAdjust is the first property to be interpolable just as percentage (https://drafts.csswg.org/css-transitions/#animatable-properties), instead of length-percentage-calc, so other percentage things were being interpolated using the length interpolator, and the actual number properties were being interpolated using number interpolater (without a problem, bcs ToCSSPrimitiveValue(value).IsNumber() is true). 

But https://drafts.csswg.org/css-transitions/#animtype-percentage says the properties only interpolable as percentages should interpolated as real numbers, just as it is for the number properties. So, I think I was not wrong putting it on that case in the type map. So, that condition on the number interpolater might need a change. 

However, long story short, I still could not make it work by trying somethings related to what I described above.

Hi alpercakan98@;

Took some time looking into this today, and I think I've solved it. All it was ('all' - took some debugging!) was that your CL doesn't convert the percentages where necessary.

The following changes should suffice to make your patch work properly:

1. CSSNumberInterpolationType::MaybeConvertValue - as you noted, this needs to allow ToCSSPrimitiveValue(value).IsPercentage() as well as ToCSSPrimitiveValue(value).IsNumber().

2. NumberPropertyFunctions::GetNumber - the return value for CSSPropertyTextSizeAdjust should be multiplied by 100, e.g. return textSizeAdjust.Multiplier() * 100;

3. NumberPropertyFunctions::SetNumber - the value for CSSPropertyTextSizeAdjust should be divided by 100, e.g. style.SetTextSizeAdjust(value / 100.);


The reason for the multiplication/division is that the keyframe stores the percentage value as a whole number, but the internal representation of TextSizeAdjust is fractional. So in your CL as it is, we would take 0.6 and try to interpolate towards that from 50, for example, rather than taking 60 and interpolating towards it from 50.

smcgruer@

They did work! I had also tried this scaling by 100 thing, but it had not work, because then I probably had not included IsPercentage() thing.

Thanks for the great help. I have sent the fixed patch.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/261cb0ef9a5a0d7603f28a516a1aaff82b5dd7da

commit 261cb0ef9a5a0d7603f28a516a1aaff82b5dd7da
Author: Alper Cakan <alpercakan98@gmail.com>
Date: Wed Mar 14 11:30:19 2018

make text-size-adjust animatable

Make text-size-adjust animatable, as it is supposed be so according to:
drafts.csswg.org/css-size-adjust/#adjustment-control

R=nainar@chromium.org, shend@chromium.org, smcgruer@chromium.org

Bug:  818526 
Change-Id: I0c3b98c4726f6898599aa5626ad2fec876057ec0
Reviewed-on: https://chromium-review.googlesource.com/956462
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Reviewed-by: Stephen McGruer <smcgruer@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543050}
[modify] https://crrev.com/261cb0ef9a5a0d7603f28a516a1aaff82b5dd7da/AUTHORS
[add] https://crrev.com/261cb0ef9a5a0d7603f28a516a1aaff82b5dd7da/third_party/WebKit/LayoutTests/animations/interpolation/text-size-adjust-interpolation.html
[add] https://crrev.com/261cb0ef9a5a0d7603f28a516a1aaff82b5dd7da/third_party/WebKit/LayoutTests/animations/responsive/animations-responsive-text-size-adjust.html
[modify] https://crrev.com/261cb0ef9a5a0d7603f28a516a1aaff82b5dd7da/third_party/WebKit/Source/core/animation/CSSInterpolationTypesMap.cpp
[modify] https://crrev.com/261cb0ef9a5a0d7603f28a516a1aaff82b5dd7da/third_party/WebKit/Source/core/animation/CSSNumberInterpolationType.cpp
[modify] https://crrev.com/261cb0ef9a5a0d7603f28a516a1aaff82b5dd7da/third_party/WebKit/Source/core/animation/NumberPropertyFunctions.cpp
[modify] https://crrev.com/261cb0ef9a5a0d7603f28a516a1aaff82b5dd7da/third_party/WebKit/Source/core/css/CSSProperties.json5
[modify] https://crrev.com/261cb0ef9a5a0d7603f28a516a1aaff82b5dd7da/third_party/WebKit/Source/core/css/CSSPropertyEquality.cpp

Woo! Congrats to alpercakan98@ on your first fix, and thanks very much for the contribution to Chrome and animations :)

smcgruer@

Thanks for the help.