[Webkit-unassigned] [Bug 39639] [Chromium] Implement new PopupMenuClient methods for AutoFill.

[bugzilla-daemon at webkit.org]

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


Darin Fisher (:fishd, Google) <fishd at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #56959|review?                     |review+, commit-queue-
               Flag|                            |




--- Comment #2 from Darin Fisher (:fishd, Google) <fishd at chromium.org>  2010-05-28 10:10:58 PST ---
(From update of attachment 56959)
WebCore/rendering/RenderMenuList.h:104
> +      virtual void selectionChanged(unsigned, bool) {}
nit: please give these parameters some names.  the type names alone
do not adequately describe their purpose.

WebCore/rendering/RenderTextControlSingleLine.h:111
> +      virtual void selectionChanged(unsigned, bool) {}
nit: please give these parameters some names.  the type names alone
do not adequately describe their purpose.

WebKit/chromium/src/AutoFillPopupMenuClient.cpp:102
 +      // FIXME: Refactor this method.
nit: please be a bit more descriptive with this comment.  if someone else
is reading this, it'd be great to not have to guess what you had in mind.
i'm not sure what you intend to refactor ;-)

WebKit/chromium/src/SuggestionsPopupMenuClient.h:64
 +      virtual void selectionChanged(unsigned, bool) {}
nit: please give these parameters some names...

otherwise, LGTM.

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