WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
57007
[Qt] Re-draw the <input> fields for Qt Mobile Theme to do not override elements already styled
https://bugs.webkit.org/show_bug.cgi?id=57007
Summary
[Qt] Re-draw the <input> fields for Qt Mobile Theme to do not override elemen...
Diego Gonzalez
Reported
2011-03-24 05:45:08 PDT
Check if the element is styled and Draw the inputs for mobile theme instead of use the css theme.
Attachments
Patch
(13.50 KB, patch)
2011-03-27 22:26 PDT
,
Diego Gonzalez
no flags
Details
Formatted Diff
Diff
Inputs with mobile theme
(76.39 KB, image/png)
2011-03-28 11:38 PDT
,
Diego Gonzalez
no flags
Details
Gmail mobile with no overridden inputs
(31.70 KB, image/png)
2011-03-28 11:39 PDT
,
Diego Gonzalez
no flags
Details
Patch 2
(14.09 KB, patch)
2011-03-28 11:41 PDT
,
Diego Gonzalez
no flags
Details
Formatted Diff
Diff
Patch 3
(16.59 KB, patch)
2011-03-29 14:19 PDT
,
Diego Gonzalez
no flags
Details
Formatted Diff
Diff
Current screenshot
(79.77 KB, image/png)
2011-03-30 04:54 PDT
,
Diego Gonzalez
no flags
Details
Combo as buttons screenshot
(22.52 KB, image/png)
2011-03-30 06:05 PDT
,
Diego Gonzalez
no flags
Details
Patch drawing combo as buttons
(16.57 KB, patch)
2011-03-30 06:06 PDT
,
Diego Gonzalez
no flags
Details
Formatted Diff
Diff
More screenshots
(88.45 KB, image/png)
2011-03-30 06:29 PDT
,
Diego Gonzalez
no flags
Details
Wrongly drawn radio buttons
(65.86 KB, image/png)
2011-03-30 11:43 PDT
,
Kenneth Rohde Christiansen
no flags
Details
Current theme screenshot after patch
(117.61 KB, image/png)
2011-04-02 11:57 PDT
,
Diego Gonzalez
no flags
Details
Patch
(17.40 KB, patch)
2011-04-02 12:03 PDT
,
Diego Gonzalez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(10)
View All
Add attachment
proposed patch, testcase, etc.
Diego Gonzalez
Comment 1
2011-03-27 22:26:54 PDT
Created
attachment 87099
[details]
Patch First patch to see what you guys think and test
WebKit Review Bot
Comment 2
2011-03-27 22:30:23 PDT
Attachment 87099
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/qt/QtMobileWebStyle.h:32: The parameter name "element" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/qt/QtMobileWebStyle.h:32: The parameter name "option" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/qt/QtMobileWebStyle.h:32: The parameter name "painter" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/qt/QtMobileWebStyle.h:32: The parameter name "widget" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 4 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Kenneth Rohde Christiansen
Comment 3
2011-03-28 00:48:29 PDT
Can you post a few screenshots?
Diego Gonzalez
Comment 4
2011-03-28 11:38:31 PDT
Created
attachment 87172
[details]
Inputs with mobile theme
Diego Gonzalez
Comment 5
2011-03-28 11:39:22 PDT
Created
attachment 87173
[details]
Gmail mobile with no overridden inputs
Diego Gonzalez
Comment 6
2011-03-28 11:41:57 PDT
Created
attachment 87174
[details]
Patch 2 Some more adjustments
WebKit Review Bot
Comment 7
2011-03-28 11:44:16 PDT
Attachment 87174
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/qt/QtMobileWebStyle.h:32: The parameter name "element" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/qt/QtMobileWebStyle.h:32: The parameter name "option" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/qt/QtMobileWebStyle.h:32: The parameter name "painter" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/qt/QtMobileWebStyle.h:32: The parameter name "widget" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 4 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Kenneth Rohde Christiansen
Comment 8
2011-03-28 14:08:29 PDT
The input fields look quite nice, but there is something wrong with the buttons. Are you looking into that?
Diego Gonzalez
Comment 9
2011-03-28 20:32:38 PDT
(In reply to
comment #8
)
> The input fields look quite nice, but there is something wrong with the buttons. Are you looking into that?
Hummmm will check it
Diego Gonzalez
Comment 10
2011-03-29 14:19:26 PDT
Created
attachment 87409
[details]
Patch 3 Added buttons padding
WebKit Review Bot
Comment 11
2011-03-29 14:21:49 PDT
Attachment 87409
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/qt/QtMobileWebStyle.h:32: The parameter name "element" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/qt/QtMobileWebStyle.h:32: The parameter name "option" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/qt/QtMobileWebStyle.h:32: The parameter name "painter" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/qt/QtMobileWebStyle.h:32: The parameter name "widget" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 4 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Kenneth Rohde Christiansen
Comment 12
2011-03-29 14:36:05 PDT
New screenshots? Also, can you make sure that we paint scrollbars as 0 pixel wide scrollbars? :-) ie, not paint them and not making them take up space at all.
Kenneth Rohde Christiansen
Comment 13
2011-03-29 14:39:04 PDT
Also post a screenshot of the current mobile theme for comparison, including at least radio button (different states, sizes) checkboxes (different states, sizes) input fields text areas scrollbars select/comboboxes (singles and multiple)
Diego Gonzalez
Comment 14
2011-03-30 04:54:39 PDT
Created
attachment 87503
[details]
Current screenshot PS: There is no styled defined for scrollbars in current mobile theme
Kenneth Rohde Christiansen
Comment 15
2011-03-30 05:18:18 PDT
IT (In reply to
comment #14
)
> Created an attachment (id=87503) [details] > Current screenshot > > PS: There is no styled defined for scrollbars in current mobile theme
That might be the issue because it shown the windows like scrollbar.
Kenneth Rohde Christiansen
Comment 16
2011-03-30 05:19:21 PDT
Looks better, but comboboxes are still wrong, ie they do not look like buttons.
Kenneth Rohde Christiansen
Comment 17
2011-03-30 05:21:48 PDT
You screenshots shows the elements zoomed in, but not scaled up. Try applying a scale on the QGraphicsWebView.
Diego Gonzalez
Comment 18
2011-03-30 06:05:29 PDT
Created
attachment 87513
[details]
Combo as buttons screenshot
Diego Gonzalez
Comment 19
2011-03-30 06:06:59 PDT
Created
attachment 87515
[details]
Patch drawing combo as buttons
WebKit Review Bot
Comment 20
2011-03-30 06:10:37 PDT
Attachment 87515
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/qt/QtMobileWebStyle.h:32: The parameter name "element" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/qt/QtMobileWebStyle.h:32: The parameter name "option" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/qt/QtMobileWebStyle.h:32: The parameter name "painter" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/qt/QtMobileWebStyle.h:32: The parameter name "widget" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 4 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Kenneth Rohde Christiansen
Comment 21
2011-03-30 06:28:34 PDT
I would say that the combos should have the same size of the buttons, currently the text placement looks quite bad in them. Apart from that it looks quite nice. Did you test the scrollbars yet?
Diego Gonzalez
Comment 22
2011-03-30 06:29:08 PDT
Created
attachment 87521
[details]
More screenshots
Kenneth Rohde Christiansen
Comment 23
2011-03-30 06:30:50 PDT
Oh the text placement also seems an issue for the text area. Please write some text in the input fields next time around. Actually, I don't think the buttons need to be so wide vertically (high).
Kenneth Rohde Christiansen
Comment 24
2011-03-30 11:38:51 PDT
Comment on
attachment 87515
[details]
Patch drawing combo as buttons View in context:
https://bugs.webkit.org/attachment.cgi?id=87515&action=review
> Source/WebCore/platform/qt/QtMobileWebStyle.cpp:192 > default: > QWindowsStyle::drawControl(element, option, painter, widget); > }
It can be this part of the code that makes the scrollbar drawn as a Windows control
> Source/WebCore/platform/qt/QtMobileWebStyle.cpp:326 > + QLinearGradient linearGradient(rect.bottomLeft(), QPoint(rect.bottomLeft().x(), rect.bottomLeft().y() - 20));
Add a comment explaining the 20, or use a variable instead with a good descriptive name
> Source/WebCore/platform/qt/QtMobileWebStyle.h:33 > > void drawControl(ControlElement element, const QStyleOption* option, QPainter* painter, const QWidget* widget = 0) const; > void drawComplexControl(ComplexControl cc, const QStyleOptionComplex* option, QPainter* painter, const QWidget* widget = 0) const; > + void drawPrimitive(PrimitiveElement element, const QStyleOption* option, QPainter* painter, const QWidget* widget = 0) const; >
You should clean up the style in another patch, or it would be good doing so. I can easily review that.
Kenneth Rohde Christiansen
Comment 25
2011-03-30 11:42:32 PDT
Hi Diego, I found one more issue that you need to fix. Radio buttons are shown wrong on
http://www.amw.com/missing_persons/search.cfm
Going to upload screenshot.
Kenneth Rohde Christiansen
Comment 26
2011-03-30 11:43:05 PDT
Created
attachment 87584
[details]
Wrongly drawn radio buttons
Diego Gonzalez
Comment 27
2011-03-30 14:03:44 PDT
(In reply to
comment #26
)
> Created an attachment (id=87584) [details] > Wrongly drawn radio buttons
Nice catch Kenneth! It's also reproducible in current mobile theme implementation. Investigating it :)
Diego Gonzalez
Comment 28
2011-04-02 11:57:22 PDT
Created
attachment 87981
[details]
Current theme screenshot after patch
Diego Gonzalez
Comment 29
2011-04-02 12:03:02 PDT
Created
attachment 87982
[details]
Patch
Kenneth Rohde Christiansen
Comment 30
2011-04-04 01:22:31 PDT
I still think that the comboboxes needs a bit more left padding, but it is a very good improvement.
Kenneth Rohde Christiansen
Comment 31
2011-04-04 01:25:50 PDT
What is missing now.: Better padding at least in text area Combobox left padding Show that you are never painting scrollbars.
Diego Gonzalez
Comment 32
2011-04-06 08:44:29 PDT
Fixed at
r83051
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug