Bug 48715 – Enable StrictTypeChecking for all types using SVG(Animated)PropertyTearOff

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Nikolas Zimmermann]

Enable StrictTypeChecking for all types using SVG(Animated)PropertyTearOff

[Nikolas Zimmermann]

Created attachment 72475 [details]
Patch

Added lots of new tests, and a little code :-)

[WebKit Review Bot]

Attachment 72475 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/svg/SVGLength.cpp:206:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
WebCore/bindings/v8/custom/V8SVGLengthCustom.cpp:82:  Tab found; better to use spaces  [whitespace/tab] [1]
Total errors found: 2 in 57 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Nikolas Zimmermann]

Created attachment 72476 [details]
Patch v2

Fix style issues, more explicit ChangeLog.

[Rob Buis]

Comment on attachment 72476 [details]
Patch v2

View in context: https://bugs.webkit.org/attachment.cgi?id=72476&action=review

I only found small issues that can easily be corrected, so r=me.

> WebCore/ChangeLog:11
> +        When testing SVGLength it became obvious that quite a lot functionality was missing.

quite a lot *of* functionality

> WebCore/ChangeLog:44
> +        * svg/SVGLength.cpp: Implement all missing features of SVGLength.e

Remove the extra e.

> WebCore/ChangeLog:47
> +        (WebCore::SVGLength::value): Add ExceptionCode parameter throw NOT_SUPPORTED_ERR when trying to obtain to relative units, when no context is given (SVGLength created by createSVGLength()).

Should be rephrased, hard to read.

> WebCore/ChangeLog:49
> +        (WebCore::SVGLength::valueAsPercentage): s/100.0f/100/, use m_valueInSpecifiedUnits instead of valueInSpecifiedUnits().

Maybe not mention s/100.0f/100/, since it is the only mention of adapting to the code guideline style but other places in SVGLength.cpp were adapted too.
Or have a general remark in the ChangeLog that this style is now preferred because of the Webkit Coding style guideline.

> WebCore/svg/SVGLength.cpp:421
> +    return value / style->fontSize();

Can reuse fontSize local variable here.

> LayoutTests/svg/dom/SVGAnimatedAngle-expected.txt:16
> +Check assigning to baseVal has no effect, as no setter is defined

Check that.

> LayoutTests/svg/dom/SVGAnimatedLength-expected.txt:16
> +Check assigning to baseVal has no effect, as no setter is defined

Check that.

> LayoutTests/svg/dom/SVGAnimatedLengthList-expected.txt:16
> +Check assigning to baseVal has no effect, as no setter is defined

Check that.

> LayoutTests/svg/dom/SVGAnimatedRect-expected.txt:16
> +Check assigning to baseVal has no effect, as no setter is defined

Ditto.

[Nikolas Zimmermann]

Thanks for the quick review, fixed all issues, landing now...

[Nikolas Zimmermann]

Landed in r71014.

[Nikolas Zimmermann]

(In reply to comment #6)
> Landed in r71014.

Forgot to land the svg/dynamic-updates changes, fixed in r71015.