Bug 40397

Summary: [Chromium] Add the ability to specify a separator in AutoFillPopupMenuChromium
Product: WebKit Reporter: James Hawkins <jhawkins>
Component: WebKit APIAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, fishd, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Patch
none
Patch fishd: review+

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