Hi, <select>s are truncated on http://www.ryanair.com. The last character on the two 'Depart Date' fields is truncated and not visible. It's displayed fine on Firefox (Linux) and Safari (Windows). Problem can be observed with Arora and Qt-WebKit 4.5.2 on Linux. Screen shots and a patch is attached. Cheers, Tim
Created attachment 39933 [details] Patch to fix issue
Created attachment 39934 [details] Screenshot (before)
Created attachment 39935 [details] Screenshot (after)
Thanks for your bug report. In the future, please follow the QtWebKit bug reporting guidlines: http://trac.webkit.org/wiki/QtWebKitContrib#ReportingBugs In particular: - All bugs related to the Qt port of WebKit should have the keyword 'Qt' - The 'WebKit Qt' component should only be used for the QtWebKit API layer
Reproduced in trunk. Tim, your patch attachment misses a ChangeLog and the review=? flag. Please see http://trac.webkit.org/wiki/QtWebKitContrib for more information.
Created attachment 51901 [details] Same patch in proper format
Comment on attachment 51901 [details] Same patch in proper format I take it that Qt does not run pixel tests yet?
Created attachment 52544 [details] MacOS screenshots. I tryed this patch in macos and the result was not so good. Attached is an image that has three screenshots of QtLauncher showing a simple page with three combos. The difference between them is the padding used in the combo. The first screenshot uses the proposed patch. Why don't just to increase the right padding and keep left and bottom padding as they are now?
Comment on attachment 51901 [details] Same patch in proper format Luiz' screenshot is very convincing ;(
(In reply to comment #8) > Created an attachment (id=52544) [details] > MacOS screenshots. > > I tryed this patch in macos and the result was not so good. > > Attached is an image that has three screenshots of QtLauncher showing a simple page with three combos. The difference between them is the padding used in the combo. The first screenshot uses the proposed patch. > > Why don't just to increase the right padding and keep left and bottom padding as they are now? Luiz, do you still have your test case for this? Were you constraining the size of the combo box?
Worth noting that the site fixes the width of the menulist to 90px and specifies 9px for the font. There are a few things wrong with the way QtWebKit is rendering menulists: - There's no bottom padding so the text is positioned too low in the drop-down box. - The menulist widgets are much taller than they need to be. QtWebKit needs to follow Firefox and others by by hugging the text a little more. The net effect is a lot neater. - Glyphs like g and j have their tails obscured in most styles I've tried in QtTestBrowser with this page. - The bug highlighted here, which seems (i) at least partly a consequence of QtWebKit's hefty left and right padding to cater for styles like oxigen, far exceeding the padding used by other ports (4 is the largest I can find anywhere else, in RenderThemeSafari). And (ii) you can't help feeling there's a font-size bug in here somewhere. The first three are probably separate bugs. For this one I'd really like to try the test case behind Luiz's screenshots as I've found motif, windows, oxygen and gtk all behave quite well when the right padding on menulists is reduced to zero.
(In reply to comment #10) > (In reply to comment #8) > > Created an attachment (id=52544) [details] [details] > > MacOS screenshots. > > > > I tryed this patch in macos and the result was not so good. > > > > Attached is an image that has three screenshots of QtLauncher showing a simple page with three combos. The difference between them is the padding used in the combo. The first screenshot uses the proposed patch. > > > > Why don't just to increase the right padding and keep left and bottom padding as they are now? > > Luiz, do you still have your test case for this? Were you constraining the size of the combo box? No, I do not have it. The one I found here is a little bit different but I would say that the one used at that time was just <select><option>option 10</option></select> <select><option>option 100</option></select> <select><option>option 1000</option></select> with no size constraint. This simple html in QtTestBrowser reproduced exactly the second screenshot of those three, as expected.
(In reply to comment #12) > > No, I do not have it. The one I found here is a little bit different but I would say that the one used at that time was just > > <select><option>option 10</option></select> > <select><option>option 100</option></select> > <select><option>option 1000</option></select> > > with no size constraint. This simple html in QtTestBrowser reproduced exactly the second screenshot of those three, as expected. There seems to be something amiss with the MacOS style: pixelMetric() is not useful to tell us the width of the combo-box drop down button. The first example in your screenshot shows this - if the only padding is the width of the button icon, the text should not impinge on it. So we need to find another way of gauging the width of the button icon - or fix the value reported by the MacOS style.
(In reply to comment #13) > There seems to be something amiss with the MacOS style: pixelMetric() is not useful to tell us the width of the combo-box drop down button. The first example in your screenshot shows this - if the only padding is the width of the button icon, the text should not impinge on it. So we need to find another way of gauging the width of the button icon - or fix the value reported by the MacOS style. Right enough, QMacStyle::pixelMetric() does not support QStyle::PM_ButtonIconSize so we get the default windows style value instead. Maybe I'm looking in the wrong place but the padding for Mac does not seem to be available through style metrics, instead they have to do stuff like: const int* RenderThemeSafari::popupButtonPadding(NSControlSize size) const { static const int padding[3][4] = { { 2, 26, 3, 8 }, { 2, 23, 3, 8 }, { 2, 22, 3, 10 } }; return padding[size]; } So it looks like we need a compile-guard for MAC builds to retain the current (large) padding on OSX and allow other styles to use QStyle::PM_ButtonIconSize + a padding of 2. Sound sensible?
Created attachment 83326 [details] patch This patch reduces padding as previous one and introduces a QMacStyle specific adjustment. The reason for this mac specific adjustment is explained in a comment in the code.
Comment on attachment 83326 [details] patch Clearing flags on attachment: 83326 Committed r79407: <http://trac.webkit.org/changeset/79407>
(In reply to comment #16) > Committed r79407: <http://trac.webkit.org/changeset/79407> I updated Qt specific expected files: http://trac.webkit.org/changeset/79414 Guys, don't you run layout tests before committing? Can we close this bug now?
(In reply to comment #17) > (In reply to comment #16) > > Committed r79407: <http://trac.webkit.org/changeset/79407> > > I updated Qt specific expected files: http://trac.webkit.org/changeset/79414 > Guys, don't you run layout tests before committing? sure > > Can we close this bug now? I think we can.