Bug 78785 – Upstream RenderThemeBlackberry.h/.cpp into WebCore/platform/blackberry

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Mary Wu]

Upstream Blackberry implementation of RenderTheme.

[Mary Wu]

Created attachment 127487 [details]
Patch

[Mary Wu]

Created attachment 127491 [details]
Patch

[Antonio Gomes]

Comment on attachment 127491 [details]
Patch

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

> Source/WebCore/platform/blackberry/RenderThemeBlackBerry.cpp:159
> +    : RenderTheme()

is this call needed?

> Source/WebCore/platform/blackberry/RenderThemeBlackBerry.cpp:410
> +    switch (part) {
> +    case CheckboxPart:
> +            {
> +                FloatSize smallCorner(smallRadius, smallRadius);
> +                Path path;
> +                path.addRoundedRect(rect, smallCorner);
> +                info.context->drawPath(path);

the switch body is over indented.

[Mary Wu]

(In reply to comment #3)
> (From update of attachment 127491 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=127491&action=review
> 
> > Source/WebCore/platform/blackberry/RenderThemeBlackBerry.cpp:159
> > +    : RenderTheme()
> 
> is this call needed?
> 
> > Source/WebCore/platform/blackberry/RenderThemeBlackBerry.cpp:410
> > +    switch (part) {
> > +    case CheckboxPart:
> > +            {
> > +                FloatSize smallCorner(smallRadius, smallRadius);
> > +                Path path;
> > +                path.addRoundedRect(rect, smallCorner);
> > +                info.context->drawPath(path);
> 
> the switch body is over indented.

nice catch! Thanks

[Mary Wu]

Created attachment 127522 [details]
Patch

[Rob Buis]

Comment on attachment 127522 [details]
Patch

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

> Source/WebCore/platform/blackberry/RenderThemeBlackBerry.cpp:121
> +float RenderThemeBlackBerry::defaultFontSize = 16.0;

Could move this up with the other constants

> Source/WebCore/platform/blackberry/RenderThemeBlackBerry.cpp:180
> +    // in-page mode the same as in fullscreen mode..

I'd do either 1 or 3 periods :)

> Source/WebCore/platform/blackberry/RenderThemeBlackBerry.cpp:187
> +    return 0.0; // Turn off caret blinking.

Better use 0.

> Source/WebCore/platform/blackberry/RenderThemeBlackBerry.cpp:804
> +    // paint main slider bar

Not a sentence.

> Source/WebCore/platform/blackberry/RenderThemeBlackBerry.cpp:811
> +        // paint played part of bar (left of slider thumb) using selection color

Ditto.

[Antonio Gomes]

Comment on attachment 127522 [details]
Patch

r=me, but fix Rob's comments before pushing. thanks

[Mary Wu]

thanks, patch updated according to Rob's comments, will commit soon...

[Mary Wu]

Created attachment 127877 [details]
update patch

[Mary Wu]

Created attachment 127882 [details]
update patch

[Leo Yang]

Comment on attachment 127882 [details]
update patch

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

Pleas add the lost change log entry. Add address a nit. I'll help land.

> Source/WebCore/platform/blackberry/RenderThemeBlackBerry.cpp:107
> +float RenderThemeBlackBerry::defaultFontSize = 16.0;
> +

Better 16

[Mary Wu]

Created attachment 127887 [details]
update patch

Thanks, Leo, here is the update patch

[Leo Yang]

Comment on attachment 127887 [details]
update patch

Good. Let me send to cq.

[WebKit Review Bot]

Comment on attachment 127887 [details]
update patch

Clearing flags on attachment: 127887

Committed r108290: <http://trac.webkit.org/changeset/108290>