Bug 40397 - [Chromium] Add the ability to specify a separator in AutoFillPopupMenuChromium
Summary: [Chromium] Add the ability to specify a separator in AutoFillPopupMenuChromium
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-09 17:53 PDT by James Hawkins
Modified: 2010-06-10 18:10 PDT (History)
4 users (show)

See Also:


Attachments
Patch (10.68 KB, patch)
2010-06-09 17:57 PDT, James Hawkins
no flags Details | Formatted Diff | Diff
Patch (11.05 KB, patch)
2010-06-10 10:52 PDT, James Hawkins
fishd: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James Hawkins 2010-06-09 17:53:46 PDT
Will upload patch .
Comment 1 James Hawkins 2010-06-09 17:57:27 PDT
Created attachment 58316 [details]
Patch
Comment 2 Darin Fisher (:fishd, Google) 2010-06-09 22:25:35 PDT
Comment on attachment 58316 [details]
Patch

WebKit/chromium/public/WebViewClient.h:295
 +      virtual void didAcceptAutoFillSuggestion(const WebNode&,
perhaps you should keep the old form of this method around and make
the default implementation of this method call that one?  that way
you won't require a two-sided patch landing to avoid regressing chrome?

WebKit/chromium/src/AutoFillPopupMenuClient.cpp:63
 +        return suggestion;
nit: indent by 4 spaces

looks ok otherwise.
Comment 3 James Hawkins 2010-06-10 10:52:23 PDT
Created attachment 58387 [details]
Patch
Comment 4 James Hawkins 2010-06-10 10:53:36 PDT
(In reply to comment #2)
> (From update of attachment 58316 [details])
> WebKit/chromium/public/WebViewClient.h:295
>  +      virtual void didAcceptAutoFillSuggestion(const WebNode&,
> perhaps you should keep the old form of this method around and make
> the default implementation of this method call that one?  that way
> you won't require a two-sided patch landing to avoid regressing chrome?
> 

I'll commit the chromium side first, which adds the new method and deprecates the old, obviating the need for the old method in the API.

> WebKit/chromium/src/AutoFillPopupMenuClient.cpp:63
>  +        return suggestion;
> nit: indent by 4 spaces
> 

Done.
Comment 5 James Hawkins 2010-06-10 17:51:48 PDT
Committed r60985: <http://trac.webkit.org/changeset/60985>
Comment 6 WebKit Review Bot 2010-06-10 18:10:43 PDT
http://trac.webkit.org/changeset/60985 might have broken Chromium Mac Release
The following changes are on the blame list:
http://trac.webkit.org/changeset/60984
http://trac.webkit.org/changeset/60985