[Webkit-unassigned] [Bug 58505] [Chromium]UI polishes and tweaks to Autofill dropdown menu.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=58505


Kent Tamura <tkent at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #89952|review?                     |review-
               Flag|                            |




--- Comment #26 from Kent Tamura <tkent at chromium.org>  2011-04-17 23:10:02 PST ---
(From update of attachment 89952)
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 "*"

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.