[webkit-reviews] review denied: [Bug 58505] [Chromium]UI polishes and tweaks to Autofill dropdown menu. : [Attachment 89952] Patch

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied Naoki Takano
<takano.naoki at gmail.com>'s request for review:
Bug 58505: [Chromium]UI polishes and tweaks to Autofill dropdown menu.
https://bugs.webkit.org/show_bug.cgi?id=58505

Attachment 89952: Patch
https://bugs.webkit.org/attachment.cgi?id=89952&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=89952&action=review

> Source/WebCore/platform/PopupMenuStyle.h:38
> +    PopupMenuStyle(const Color& foreground, const Color& background, const
Font& font, bool visible, bool isDisplayNone, Length textIndent, TextDirection
textDirection, bool hasTextDirectionOverride, bool isAutofillStyle)

Please avoid new bool parameter.  We prefer introducing enum. Also, we can
avoid updating RenderMenuList and RenderTextConstrolSingleLine by a default
parameter.

enum PopupMenuType { SelectPopup, AutofillPopup };
PopupMenuStyle(const Color&...., PopupMenuType menuType = SelectPopup)

> Source/WebCore/platform/chromium/PopupMenuChromium.cpp:78
> +static const int kLinePaddingHeight = 3; // Padding height put at the top
and bottom of each line.

Do not use "k" prefix for constant values.

> Source/WebCore/platform/chromium/PopupMenuChromium.cpp:1000
> +	   d.setComputedSize(d.computedSize()*0.8);

Add spaces around "*"