Bug 78785

Summary: Upstream RenderThemeBlackberry.h/.cpp into WebCore/platform/blackberry
Product: WebKit Reporter: Mary Wu <mawu>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: leo.yang, rwlbuis, tonikitoo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 73144, 78920    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
tonikitoo: review+, tonikitoo: commit-queue-
update patch
none
update patch
none
update patch none

Description Mary Wu 2012-02-16 01:15:38 PST
Upstream Blackberry implementation of RenderTheme.
Comment 1 Mary Wu 2012-02-16 17:56:28 PST
Created attachment 127487 [details]
Patch
Comment 2 Mary Wu 2012-02-16 18:31:21 PST
Created attachment 127491 [details]
Patch
Comment 3 Antonio Gomes 2012-02-16 19:17:05 PST
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.
Comment 4 Mary Wu 2012-02-16 21:20:19 PST
(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
Comment 5 Mary Wu 2012-02-16 22:07:34 PST
Created attachment 127522 [details]
Patch
Comment 6 Rob Buis 2012-02-17 12:12:25 PST
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.
Comment 7 Antonio Gomes 2012-02-17 15:36:13 PST
Comment on attachment 127522 [details]
Patch

r=me, but fix Rob's comments before pushing. thanks
Comment 8 Mary Wu 2012-02-19 19:03:51 PST
thanks, patch updated according to Rob's comments, will commit soon...
Comment 9 Mary Wu 2012-02-20 17:45:56 PST
Created attachment 127877 [details]
update patch
Comment 10 Mary Wu 2012-02-20 17:56:13 PST
Created attachment 127882 [details]
update patch
Comment 11 Leo Yang 2012-02-20 18:07:35 PST
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
Comment 12 Mary Wu 2012-02-20 18:23:31 PST
Created attachment 127887 [details]
update patch

Thanks, Leo, here is the update patch
Comment 13 Leo Yang 2012-02-20 18:31:17 PST
Comment on attachment 127887 [details]
update patch

Good. Let me send to cq.
Comment 14 WebKit Review Bot 2012-02-20 19:19:02 PST
Comment on attachment 127887 [details]
update patch

Clearing flags on attachment: 127887

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