Implement SVGFESpecularLightingElement.kernelUnitLengthX/kernelUnitLengthY.

Implement SVGFESpecularLightingElement.kernelUnitLengthX/kernelUnitLengthY.

Result about the support in other browsers.
- Firefox(36.0.4)     [Support]
- IE(11.0.9600.17801) [Support]

TEST=LayoutTests/svg/dom/SVGFESpecularLightingElement-svgdom-kernelUnitLength.html
BUG=492546
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=196300

[hyunjunekim2, 2015-05-28 14:31:16]

hyunjune.kim@samsung.com changed reviewers:
+ fs@opera.com, pdr@chromium.org, philipj@opera.com

[hyunjunekim2, 2015-05-29 12:41:45]

philipj_UTC2, Could you check this one? firefox support this one. and i'm going
to check IE. thank you.

[philipj_slow, 2015-05-29 12:50:09]

Looks good, but I'll let fs and/or pdr l g t m this. It's a bit odd to add new
things to the SVG1DOM bucket, since presumably we're hopeful of getting rid of
the things protected by that runtime flag.

https://codereview.chromium.org/1155743010/diff/20001/LayoutTests/svg/dynamic...
File
LayoutTests/svg/dynamic-updates/script-tests/SVGFESpecularLightingElement-dom-kernelUnitLength-attr.js
(right):

https://codereview.chromium.org/1155743010/diff/20001/LayoutTests/svg/dynamic...
LayoutTests/svg/dynamic-updates/script-tests/SVGFESpecularLightingElement-dom-kernelUnitLength-attr.js:63:
shouldBeEqualToString("specularElement.getAttribute('kernelUnitLength')", "4
5");
Also test reading the attributes, i.e. check that after
specularElement.setAttribute("kernelUnitLength", "3 4"),
specularElement.kernelUnitLengthX.baseVal == 3 and
specularElement.kernelUnitLengthY.baseVal == 4.

[hyunjunekim2, 2015-05-29 12:51:00]

On 2015/05/29 12:41:45, hyunjunekim2 wrote:
> philipj_UTC2, Could you check this one? firefox support this one. and i'm
going
> to check IE. thank you.

And IE supported kernelUnitLengthX/kernelUnitLengthY also.

[philipj_slow, 2015-05-29 12:52:23]

Please write about the support in other browsers in the description, so that
people don't need to look up the review if they see this in git log.

[hyunjunekim2, 2015-05-29 12:52:42]

> And IE supported kernelUnitLengthX/kernelUnitLengthY also.
IE version is 11.0.9600.17801.

[hyunjunekim2, 2015-05-29 12:53:59]

On 2015/05/29 12:52:23, philipj wrote:
> Please write about the support in other browsers in the description, so that
> people don't need to look up the review if they see this in git log.

Ok, I will re-write the description.

[fs, 2015-05-29 13:18:40]

On 2015/05/29 12:50:09, philipj wrote:
> Looks good, but I'll let fs and/or pdr l g t m this. It's a bit odd to add new
> things to the SVG1DOM bucket, since presumably we're hopeful of getting rid of
> the things protected by that runtime flag.

Yeah, I'm a bit on the fence on this, for this exact reason. Adding a property
we'd rather not have in the first place, and that no one has been complaining
about the lack of... It's still in the Filter Effects draft though, so I guess I
won't throw a spanner in the works. If/when we decide to drop them we'd have the
other two in the boat with us at least...

https://codereview.chromium.org/1155743010/diff/20001/LayoutTests/svg/dynamic...
File
LayoutTests/svg/dynamic-updates/script-tests/SVGFESpecularLightingElement-dom-kernelUnitLength-attr.js
(right):

https://codereview.chromium.org/1155743010/diff/20001/LayoutTests/svg/dynamic...
LayoutTests/svg/dynamic-updates/script-tests/SVGFESpecularLightingElement-dom-kernelUnitLength-attr.js:63:
shouldBeEqualToString("specularElement.getAttribute('kernelUnitLength')", "4
5");
On 2015/05/29 12:50:09, philipj wrote:
> Also test reading the attributes, i.e. check that after
> specularElement.setAttribute("kernelUnitLength", "3 4"),
> specularElement.kernelUnitLengthX.baseVal == 3 and
> specularElement.kernelUnitLengthY.baseVal == 4.

Preferably === even.

Also, a test for this should strictly speaking go in a file named
SVGFESpecularLightingElement-svgdom-*. That said though I think a simple one-off
test in svg/dom/ would be fine (and even preferred to svg/dynamic-updates/ which
is a location I'm not very fond of TBH...)

[hyunjunekim2, 2015-05-31 14:21:47]

fs, philipj,
Could you check ps3?

[fs, 2015-06-01 09:13:46]

https://codereview.chromium.org/1155743010/diff/40001/LayoutTests/svg/dom/SVG...
File
LayoutTests/svg/dom/SVGFESpecularLightingElement-svgdom-kernelUnitLength.html
(right):

https://codereview.chromium.org/1155743010/diff/40001/LayoutTests/svg/dom/SVG...
LayoutTests/svg/dom/SVGFESpecularLightingElement-svgdom-kernelUnitLength.html:1:
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
Use HTML5 doctype:

<!DOCTYPE html>

https://codereview.chromium.org/1155743010/diff/40001/LayoutTests/svg/dom/SVG...
LayoutTests/svg/dom/SVGFESpecularLightingElement-svgdom-kernelUnitLength.html:3:
<head>
Don't need these two. (And corresponding end tags.)

https://codereview.chromium.org/1155743010/diff/40001/LayoutTests/svg/dom/SVG...
LayoutTests/svg/dom/SVGFESpecularLightingElement-svgdom-kernelUnitLength.html:6:
<body>
Don't need this.

https://codereview.chromium.org/1155743010/diff/40001/LayoutTests/svg/dom/SVG...
LayoutTests/svg/dom/SVGFESpecularLightingElement-svgdom-kernelUnitLength.html:10:
<script
src="script-tests/SVGFESpecularLightingElement-svgdom-kernelUnitLength.js"></script>
Use an inline script instead. This also seems like an excellent case for using
testharness.js (instead of js-test.js).

[philipj_slow, 2015-06-01 09:16:10]

pdr@, how do you feel about adding stuff to the SVG1DOM bucket? In this
particular case I don't think SVGFESpecularLightingElement is a candidate for
removal, but if the baseVal/animVal distinction can be eliminated then these
would be double rather than SVGAnimatedNumber.

[hyunjunekim2, 2015-06-01 14:11:40]

fs, philipj,
I modified a test. Could you check PS 4?

[philipj_slow, 2015-06-01 14:16:33]

https://codereview.chromium.org/1155743010/diff/60001/LayoutTests/svg/dom/SVG...
File
LayoutTests/svg/dom/SVGFESpecularLightingElement-svgdom-kernelUnitLength.html
(right):

https://codereview.chromium.org/1155743010/diff/60001/LayoutTests/svg/dom/SVG...
LayoutTests/svg/dom/SVGFESpecularLightingElement-svgdom-kernelUnitLength.html:5:
setup(function() {
You only have one test, so you can just do `var specularElement = ...` in the
test itself.

[hyunjunekim2, 2015-06-01 14:51:40]

On 2015/06/01 14:16:33, philipj wrote:
>
https://codereview.chromium.org/1155743010/diff/60001/LayoutTests/svg/dom/SVG...
> File
> LayoutTests/svg/dom/SVGFESpecularLightingElement-svgdom-kernelUnitLength.html
> (right):
> 
>
https://codereview.chromium.org/1155743010/diff/60001/LayoutTests/svg/dom/SVG...
>
LayoutTests/svg/dom/SVGFESpecularLightingElement-svgdom-kernelUnitLength.html:5:
> setup(function() {
> You only have one test, so you can just do `var specularElement = ...` in the
> test itself.

I updated PS 5.

[philipj_slow, 2015-06-01 15:14:29]

I'm going to say LGTM, but please wait for pdr's verdict as both fs and me seem
to be slightly on the fence.

https://codereview.chromium.org/1155743010/diff/80001/LayoutTests/svg/dom/SVG...
File
LayoutTests/svg/dom/SVGFESpecularLightingElement-svgdom-kernelUnitLength.html
(right):

https://codereview.chromium.org/1155743010/diff/80001/LayoutTests/svg/dom/SVG...
LayoutTests/svg/dom/SVGFESpecularLightingElement-svgdom-kernelUnitLength.html:5:
var specularElement = document.createElementNS("http://www.w3.org/2000/svg",
"feSpecularLighting");
I meant you could put this inside the test function actually, although it
doesn't really matter.

[pdr., 2015-06-01 22:31:40]

On 2015/06/01 at 09:16:10, philipj wrote:
> pdr@, how do you feel about adding stuff to the SVG1DOM bucket? In this
particular case I don't think SVGFESpecularLightingElement is a candidate for
removal, but if the baseVal/animVal distinction can be eliminated then these
would be double rather than SVGAnimatedNumber.

SGTM

[philipj_slow, 2015-06-02 07:23:44]

The CQ bit was checked by philipj@opera.com

[philipj_slow, 2015-06-02 07:23:44]

lgtm

OK, let's do this.

[commit-bot: I haz the power, 2015-06-02 07:24:10]

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

[commit-bot: I haz the power, 2015-06-02 09:00:00]

Committed patchset #7 (id:120001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196300