Bug 58248

Summary:

[Qt] Better padding for inputs and comboxes in mobile theme

Product:

WebKit

Reporter:

Diego Gonzalez <diegohcg>

Component:

WebKit Qt

Assignee:

Diego Gonzalez <diegohcg>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

kenneth, menard

Priority:

Keywords:

Qt, QtTriaged

Version:

528+ (Nightly build)

Hardware:

OS:

Linux

Attachments:

Description	Flags
Screenshot	none
Screenshot after patch	none
Patch	kenneth: review-
Patch 2	kenneth: review+

Diego Gonzalez

Reported 2011-04-11 11:19:43 PDT

Adjust padding for input and combos

Attachments
Screenshot (47.33 KB, image/png) 2011-04-11 11:20 PDT, Diego Gonzalez	no flags	Details
Screenshot after patch (28.34 KB, image/png) 2011-04-11 12:05 PDT, Diego Gonzalez	no flags	Details
Patch (3.91 KB, patch) 2011-04-11 12:06 PDT, Diego Gonzalez	kenneth: review-	Details Formatted Diff Diff
Patch 2 (3.79 KB, patch) 2011-04-11 15:30 PDT, Diego Gonzalez	kenneth: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Diego Gonzalez

Comment 1 2011-04-11 11:20:43 PDT

Created attachment 89036 [details] Screenshot

Diego Gonzalez

Comment 2 2011-04-11 12:05:16 PDT

Created attachment 89044 [details] Screenshot after patch

Diego Gonzalez

Comment 3 2011-04-11 12:06:47 PDT

Created attachment 89045 [details] Patch

Kenneth Rohde Christiansen

Comment 4 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

Diego Gonzalez

Comment 5 2011-04-11 15:30:16 PDT

Created attachment 89103 [details] Patch 2

Kenneth Rohde Christiansen

Comment 6 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

Diego Gonzalez

Comment 7 2011-04-12 14:32:34 PDT

Committed at r83615

Note You need to log in before you can comment on or make changes to this bug.