Web Animations: Always use svg prefix when animating SVG attributes

Web Animations: Always use svg prefix when animating SVG attributes

Some SVG attribute names collide with CSS property names, and the
'offset' attribute collides with the use of 'offset' in Web Animations.

We avoid conflicts by using 'svgOffset', etc. when animating SVG
attributes using Web Animations.

BUG=416735
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=196744

[Eric Willigers, 2015-06-05 08:55:30]

ericwilligers@chromium.org changed reviewers:
+ dstockwell@chromium.org, shans@chromium.org

[shans, 2015-06-07 06:42:06]

lgtm

https://codereview.chromium.org/1151573017/diff/20001/Source/core/animation/E...
File Source/core/animation/EffectInput.cpp (right):

https://codereview.chromium.org/1151573017/diff/20001/Source/core/animation/E...
Source/core/animation/EffectInput.cpp:66: property.insert(&first, 1, 0);
This differs from the previous behaviour, but I assume this is more correct
(single char toLower rather than whole string)?

[Eric Willigers, 2015-06-09 04:02:12]

ericwilligers@chromium.org changed reviewers:
+ alancutter@chromium.org

[Eric Willigers, 2015-06-09 04:02:12]

https://codereview.chromium.org/1151573017/diff/20001/Source/core/animation/E...
File Source/core/animation/EffectInput.cpp (right):

https://codereview.chromium.org/1151573017/diff/20001/Source/core/animation/E...
Source/core/animation/EffectInput.cpp:66: property.insert(&first, 1, 0);
On 2015/06/07 06:42:06, shans wrote:
> This differs from the previous behaviour, but I assume this is more correct
> (single char toLower rather than whole string)?

Yes, I was sloppy before, it is now correct.

[alancutter (OOO until 2018), 2015-06-09 06:25:06]

lgtm with nits.

https://codereview.chromium.org/1151573017/diff/20001/LayoutTests/animations/...
File
LayoutTests/animations/svg-attribute-interpolation/resources/interpolation-test.js
(right):

https://codereview.chromium.org/1151573017/diff/20001/LayoutTests/animations/...
LayoutTests/animations/svg-attribute-interpolation/resources/interpolation-test.js:474:
attributeName = 'svg' + attributeName[0].toUpperCase() + attributeName.slice(1);
Nit: Store this in a different variable called prefixedAttributeName (and maybe
drop the Name part from both).

https://codereview.chromium.org/1151573017/diff/20001/Source/core/animation/E...
File Source/core/animation/EffectInput.cpp (right):

https://codereview.chromium.org/1151573017/diff/20001/Source/core/animation/E...
Source/core/animation/EffectInput.cpp:66: property.insert(&first, 1, 0);
On 2015/06/09 at 04:02:12, Eric Willigers wrote:
> On 2015/06/07 06:42:06, shans wrote:
> > This differs from the previous behaviour, but I assume this is more correct
> > (single char toLower rather than whole string)?
> 
> Yes, I was sloppy before, it is now correct.

So we no longer support animating things like svgcx or svgCX? You should mention
this in the description if it is the case.

[Eric Willigers, 2015-06-09 07:05:53]

The CQ bit was checked by ericwilligers@chromium.org

[commit-bot: I haz the power, 2015-06-09 07:06:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1151573017/20001

[commit-bot: I haz the power, 2015-06-09 08:23:40]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196744

[Eric Willigers, 2015-06-09 09:35:51]

On 2015/06/09 06:25:06, alancutter wrote:
> lgtm with nits.
> 
>
https://codereview.chromium.org/1151573017/diff/20001/LayoutTests/animations/...
> File
>
LayoutTests/animations/svg-attribute-interpolation/resources/interpolation-test.js
> (right):
> 
>
https://codereview.chromium.org/1151573017/diff/20001/LayoutTests/animations/...
>
LayoutTests/animations/svg-attribute-interpolation/resources/interpolation-test.js:474:
> attributeName = 'svg' + attributeName[0].toUpperCase() +
attributeName.slice(1);
> Nit: Store this in a different variable called prefixedAttributeName (and
maybe
> drop the Name part from both).
> 
>
https://codereview.chromium.org/1151573017/diff/20001/Source/core/animation/E...
> File Source/core/animation/EffectInput.cpp (right):
> 
>
https://codereview.chromium.org/1151573017/diff/20001/Source/core/animation/E...
> Source/core/animation/EffectInput.cpp:66: property.insert(&first, 1, 0);
> On 2015/06/09 at 04:02:12, Eric Willigers wrote:
> > On 2015/06/07 06:42:06, shans wrote:
> > > This differs from the previous behaviour, but I assume this is more
correct
> > > (single char toLower rather than whole string)?
> > 
> > Yes, I was sloppy before, it is now correct.
> 
> So we no longer support animating things like svgcx or svgCX? You should
mention
> this in the description if it is the case.

Sorry Alan, I noticed the nits after it committed. The feature it still behind a
flag, this change does prevent 'svgcx' and 'svgCX' from working, which is
important as we hope to soon move the feature out from behind a flag.