Bug 48822 – Convert SVGPreserveAspectRatio to the new SVGPropertyTearOff concept

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

[Nikolas Zimmermann]

Convert SVGPreserveAspectRatio to the new SVGPropertyTearOff concept.

[Nikolas Zimmermann]

Created attachment 72634 [details]
Patch

[Nikolas Zimmermann]

Created attachment 72635 [details]
Patch v2

Recreate patch, after updating to trunk, so it applies.

[WebKit Review Bot]

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

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/svg/SVGPreserveAspectRatio.cpp:168:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
WebCore/svg/SVGPreserveAspectRatio.h:26:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 30 files


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

[Nikolas Zimmermann]

(In reply to comment #3)
> Attachment 72635 [details] did not pass style-queue:
> 
> Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
> WebCore/svg/SVGPreserveAspectRatio.cpp:168:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
> WebCore/svg/SVGPreserveAspectRatio.h:26:  Alphabetical sorting problem.  [build/include_order] [4]
> Total errors found: 2 in 30 files

Fixed the second issue, ignoring the first on purpose.
switch (blub) {
case foo:
{
    foobar
}
}

is just fine, the style guide doesn't mention how braces should be formatted when used inside a case statement. The bot only doesn't complain if I would have used:

switch (blub) {
case foo:
    {
        foobar
    }
}

which looks crazy.

[Rob Buis]

Comment on attachment 72635 [details]
Patch v2

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

Found only one minor issue, so r=me.

> LayoutTests/svg/dom/script-tests/SVGPreserveAspectRatio.js:1
> +description("This test checks the SVGPreserveAspectRatio API");

may add the  "utilizing" explanation for consistency :)

[Early Warning System Bot]

Attachment 72635 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4979013

[Nikolas Zimmermann]

Fixed style issue two, qt and efl builds (all related to SVGNames.h include missing in two places, funny that I don't need those includes on mac though....). Anyhow, landed in r71103. Closing bug after I get all bot results.

[Eric Seidel (no email)]

Attachment 72635 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/4959011

[Nikolas Zimmermann]

Forgot to close.