Bug 42210

Summary: [chromium] Autofill menu shows seperator at the wrong place when an entry is deleted
Product: WebKit Reporter: Scott Violet <sky>
Component: WebKit Misc.Assignee: Scott Violet <sky>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dhollowa, jhawkins, levin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows 7   
Attachments:
Description Flags
Initial fix
levin: review-, levin: commit-queue-
Addresses review feedback none

Description Scott Violet 2010-07-13 16:32:41 PDT
- Trigger the autofill popup in a form text input where you have autofill and autocomplete values.
- In the popup, select the autocomplete value and press the delete key

Expected:
The selected entry is deleted and the menu shows the remaining entries, the separator and "Autofill options"

Actual:
The selected entry is deleted but "Autofill options" menu shows before the separator and if selected fills the text input with the actula "Autofill options" string
Comment 1 Scott Violet 2010-07-13 16:36:08 PDT
Created attachment 61430 [details]
Initial fix
Comment 2 James Hawkins 2010-07-13 16:43:17 PDT
LGTM
Comment 3 David Levin 2010-07-13 23:02:48 PDT
Comment on attachment 61430 [details]
Initial fix

> Index: ChangeLog
> +        * ../../WebKit/chromium/src/AutoFillPopupMenuClient.cpp:
>
A few probloems:
1. The is either the wrong ChangeLog or else this patch is malformed. It should be the ChangeLog at WebKit/chromium/ChangeLog.
2. The path in the ChangeLog is messed up.
3. The function being modified is missing in the ChangeLog.
My bad if I missed any of these things before.


Lastly,it is nice to give a *small* comment by the function in the ChangeLog to explain what was done. (It looks like the change involves account for the separator index.)
Comment 4 Scott Violet 2010-07-14 11:04:46 PDT
Created attachment 61539 [details]
Addresses review feedback

Adds call to popupmenuclient to make sure the removal is allowed. Without this the WebViewClient was notified even if the remove wasn't allowed.
Comment 5 James Hawkins 2010-07-14 11:36:46 PDT
Patch set 2 LGTM, much better.
Comment 6 David Holloway 2010-07-14 12:41:37 PDT
LGTM.
Comment 7 WebKit Commit Bot 2010-07-14 20:00:55 PDT
Comment on attachment 61539 [details]
Addresses review feedback

Clearing flags on attachment: 61539

Committed r63395: <http://trac.webkit.org/changeset/63395>
Comment 8 WebKit Commit Bot 2010-07-14 20:01:00 PDT
All reviewed patches have been landed.  Closing bug.