[webkit-reviews] review denied: [Bug 56752] [QT] Overlap of selection lists on www.travelocity.com : [Attachment 86354] Fixes the returned width of characters for selection boxes

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied  review:
Bug 56752: [QT] Overlap of selection lists on www.travelocity.com
https://bugs.webkit.org/show_bug.cgi?id=56752

Attachment 86354: Fixes the returned width of characters for selection boxes
https://bugs.webkit.org/attachment.cgi?id=86354&action=review

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=86354&action=review

R- because:
-change not justified. You do not explain why your code is correct, you just
said it fixes a particular website
-no tests

Note that you forgot the review flag.

> Source/WebCore/ChangeLog:12
> +	https://bugs.webkit.org/show_bug.cgi?id=56752
> +
> +	The method RenderThemeQt::minimumMenuListSize returns a width that too
wide for
> +	each character. This leaves extra spaces in a selection list and makes
> +	selection lists overlap each other in web pages.
> +
> +	Change minimumMenuListSize to return the size (fm.width*1) instead of
(fm.witdh*7).

Indent is wrong.

> Source/WebCore/platform/qt/RenderThemeQt.cpp:437
> +    return 1 * fm.width(QLatin1Char('x'));

You could just return fm.width(QLatin1Char('x'));, the "1 * " does not add
clarity.