[webkit-reviews] review denied: [Bug 35770] CSSPrimitiveValue.getFloatValue does not convert sizes : [Attachment 73745] [PATCH] Suggested solution

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Nov 12 09:20:09 PST 2010


mitz at webkit.org has denied Alexander Pavlov (apavlov) <apavlov at chromium.org>'s
request for review:
Bug 35770: CSSPrimitiveValue.getFloatValue does not convert sizes
https://bugs.webkit.org/show_bug.cgi?id=35770

Attachment 73745: [PATCH] Suggested solution
https://bugs.webkit.org/attachment.cgi?id=73745&action=review

------- Additional Comments from mitz at webkit.org
View in context: https://bugs.webkit.org/attachment.cgi?id=73745&action=review

> WebCore/css/CSSPrimitiveValue.cpp:48
> +#ifndef M_PI
> +#define M_PI 3.14159265358979323846
> +#endif

Please use piDouble. Include MathExtras.h if needed.

> WebCore/css/CSSPrimitiveValue.cpp:80
> +CSSPrimitiveValue::UnitCategories CSSPrimitiveValue::unitCategory(UnitTypes
type)

This can be a file-static function.

> WebCore/css/CSSPrimitiveValue.cpp:489
> +	       factor = 180.0 / M_PI;

Use piDouble and drop the .0

> WebCore/css/CSSPrimitiveValue.cpp:495
> +	       factor = 360.0;

Drop the .0

> WebCore/css/CSSPrimitiveValue.cpp:499
> +	       factor = 1000.0;

Drop the .0

> WebCore/css/CSSPrimitiveValue.h:98
> +    enum UnitCategories {

enum names are usually singular

> WebCore/css/CSSPrimitiveValue.h:99
> +	   UNIT_NUMBER,

This is not the standard style for enum values.


More information about the webkit-reviews mailing list