[webkit-reviews] review denied: [Bug 100760] [BlackBerry] Update BB10 form theme. : [Attachment 171454] webkit patch

[bugzilla-daemon at webkit.org]

Rob Buis <rwlbuis at gmail.com> has denied Tiancheng Jiang <tijiang at rim.com>'s
request for review:
Bug 100760: [BlackBerry] Update BB10 form theme.
https://bugs.webkit.org/show_bug.cgi?id=100760

Attachment 171454: webkit patch
https://bugs.webkit.org/attachment.cgi?id=171454&action=review

------- Additional Comments from Rob Buis <rwlbuis at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=171454&action=review


Don't forget that you need a ChangeLog! If you do one mention PR and internal
reviewer.

> Source/WebCore/platform/blackberry/RenderThemeBlackBerry.cpp:74
> +// Slider Aura, caculated from UX spec

Typo caculated.

> Source/WebCore/platform/blackberry/RenderThemeBlackBerry.cpp:77
> +// Dropdown arrow position, caculated from UX spec

Ditto.

> Source/WebCore/platform/blackberry/RenderThemeBlackBerry.cpp:478
> +	   if (!disabled || !background || !inactive || !pressed || !active ||
!activeMark || !disableMark)

You probably want to log something if this failed.

> Source/WebCore/platform/blackberry/RenderThemeBlackBerry.cpp:486
> +    float centerY = (float(inactive->height()) -
float(activeMark->height())) / 2 / float(inactive->height()) * tmpRect.height()
+ tmpRect.y();

the double '/' looks weird, please try to add more parentheses.

> Source/WebCore/platform/blackberry/RenderThemeBlackBerry.cpp:541
> +    float centerY = (float(inactive->height()) -
float(activeMark->height())) / 2 / float(inactive->height()) * tmpRect.height()
+ tmpRect.y();

Ditto.