Bug 58248

Summary: [Qt] Better padding for inputs and comboxes in mobile theme
Product: WebKit Reporter: Diego Gonzalez <diegohcg>
Component: WebKit QtAssignee: Diego Gonzalez <diegohcg>
Status: RESOLVED FIXED    
Severity: Normal CC: kenneth, menard
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Screenshot
none
Screenshot after patch
none
Patch
kenneth: review-
Patch 2 kenneth: review+

Description Diego Gonzalez 2011-04-11 11:19:43 PDT
Adjust padding for input and combos
Comment 1 Diego Gonzalez 2011-04-11 11:20:43 PDT
Created attachment 89036 [details]
Screenshot
Comment 2 Diego Gonzalez 2011-04-11 12:05:16 PDT
Created attachment 89044 [details]
Screenshot after patch
Comment 3 Diego Gonzalez 2011-04-11 12:06:47 PDT
Created attachment 89045 [details]
Patch
Comment 4 Kenneth Rohde Christiansen 2011-04-11 13:23:55 PDT
Comment on attachment 89045 [details]
Patch

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

> Source/WebCore/platform/qt/RenderThemeQt.cpp:109
> +static const float defaultButtonPaddingLeft = 18;

I am not sure the default is really needed here. We could also put them in an anomymous namespace so that they are only visible here.

Info here: http://msdn.microsoft.com/en-us/library/yct4x9k5(v=vs.80).aspx : You can declare an unnamed namespace as a superior alternative to the use of global static variable declarations.

> Source/WebCore/platform/qt/RenderThemeQt.cpp:113
> +static const float defaultComboBoxPadding = 9;

As it is the selector element, combobox might not be the best name
Comment 5 Diego Gonzalez 2011-04-11 15:30:16 PDT
Created attachment 89103 [details]
Patch 2
Comment 6 Kenneth Rohde Christiansen 2011-04-12 01:06:21 PDT
Comment on attachment 89103 [details]
Patch 2

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

I assume this doesnt break current non mobile theme theming

> Source/WebCore/platform/qt/RenderThemeQt.cpp:108
> +// Values to mobile theme padding

s/to/for - but useless comment :-) just remove
Comment 7 Diego Gonzalez 2011-04-12 14:32:34 PDT
Committed at r83615