Bug 48822 - Convert SVGPreserveAspectRatio to the new SVGPropertyTearOff concept
Summary: Convert SVGPreserveAspectRatio to the new SVGPropertyTearOff concept
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nikolas Zimmermann
URL:
Keywords:
Depends on:
Blocks: 47905
  Show dependency treegraph
 
Reported: 2010-11-02 00:17 PDT by Nikolas Zimmermann
Modified: 2010-11-04 08:53 PDT (History)
6 users (show)

See Also:


Attachments
Patch (57.43 KB, patch)
2010-11-02 00:23 PDT, Nikolas Zimmermann
no flags Details | Formatted Diff | Diff
Patch v2 (57.60 KB, patch)
2010-11-02 00:31 PDT, Nikolas Zimmermann
rwlbuis: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolas Zimmermann 2010-11-02 00:17:16 PDT
Convert SVGPreserveAspectRatio to the new SVGPropertyTearOff concept.
Comment 1 Nikolas Zimmermann 2010-11-02 00:23:21 PDT
Created attachment 72634 [details]
Patch
Comment 2 Nikolas Zimmermann 2010-11-02 00:31:15 PDT
Created attachment 72635 [details]
Patch v2

Recreate patch, after updating to trunk, so it applies.
Comment 3 WebKit Review Bot 2010-11-02 00:34:00 PDT
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.
Comment 4 Nikolas Zimmermann 2010-11-02 00:37:40 PDT
(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.
Comment 5 Rob Buis 2010-11-02 00:48:57 PDT
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 :)
Comment 6 Early Warning System Bot 2010-11-02 01:10:19 PDT
Attachment 72635 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4979013
Comment 7 Nikolas Zimmermann 2010-11-02 01:36:14 PDT
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.
Comment 8 Eric Seidel (no email) 2010-11-02 02:19:26 PDT
Attachment 72635 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/4959011
Comment 9 Nikolas Zimmermann 2010-11-04 08:53:49 PDT
Forgot to close.