Bug 39639

Summary:

[Chromium] Implement new PopupMenuClient methods for AutoFill.

Product:

WebKit

Reporter:

James Hawkins <jhawkins>

Component:

WebKit API

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, eric, fishd, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

All

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

James Hawkins

Reported 2010-05-24 19:12:18 PDT

Patch to be uploaded.

Attachments
Patch (11.57 KB, patch) 2010-05-24 19:13 PDT, James Hawkins	no flags	Details Formatted Diff Diff
Patch (11.74 KB, patch) 2010-06-01 13:34 PDT, James Hawkins	no flags	Details Formatted Diff Diff
Patch (11.66 KB, patch) 2010-06-01 13:52 PDT, James Hawkins	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

James Hawkins

Comment 1 2010-05-24 19:13:52 PDT

Created attachment 56959 [details] Patch

Darin Fisher (:fishd, Google)

Comment 2 2010-05-28 10:10:58 PDT

Comment on attachment 56959 [details] Patch 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.

James Hawkins

Comment 3 2010-06-01 13:26:38 PDT

(In reply to comment #2) > (From update of attachment 56959 [details]) > 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. > Done. > 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. > Done. > 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 ;-) > Done. > WebKit/chromium/src/SuggestionsPopupMenuClient.h:64 > + virtual void selectionChanged(unsigned, bool) {} > nit: please give these parameters some names... > > otherwise, LGTM. Done.

James Hawkins

Comment 4 2010-06-01 13:34:32 PDT

Created attachment 57588 [details] Patch

Eric Seidel (no email)

Comment 5 2010-06-01 13:41:33 PDT

Attachment 57588 [details] did not build on mac: Build output: http://webkit-commit-queue.appspot.com/results/2795065

James Hawkins

Comment 6 2010-06-01 13:52:50 PDT

Created attachment 57592 [details] Patch

James Hawkins

Comment 7 2010-06-01 13:53:29 PDT

Removed param names from the methods that left the params unused -> build fix.