Introduce MenuItemID to autofill popup
authorkeishi@webkit.org <keishi@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Apr 2012 05:21:58 +0000 (05:21 +0000)
committerkeishi@webkit.org <keishi@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Apr 2012 05:21:58 +0000 (05:21 +0000)
https://bugs.webkit.org/show_bug.cgi?id=83777

Introducing MenuItemIDs because we want to add multiple separators and
need to identify non-autofill menu items without resorting to the separator position.

Reviewed by Kent Tamura.

* public/WebAutofillClient.h:
(WebKit::WebAutofillClient::didAcceptAutofillSuggestion): Changed uniqueID to itemID because they aren't unique.
(WebKit::WebAutofillClient::didSelectAutofillSuggestion):
* public/WebView.h:
(WebView):
* src/AutofillPopupMenuClient.cpp:
(WebKit::AutofillPopupMenuClient::AutofillPopupMenuClient):
(WebKit::AutofillPopupMenuClient::getSuggestionsCount):
(WebKit::AutofillPopupMenuClient::getSuggestion):
(WebKit::AutofillPopupMenuClient::getLabel):
(WebKit::AutofillPopupMenuClient::getIcon):
(WebKit::AutofillPopupMenuClient::removeSuggestionAtIndex):
(WebKit::AutofillPopupMenuClient::canRemoveSuggestionAtIndex):
(WebKit::AutofillPopupMenuClient::valueChanged):
(WebKit::AutofillPopupMenuClient::selectionChanged):
(WebKit::AutofillPopupMenuClient::itemIsSeparator):
(WebKit::AutofillPopupMenuClient::itemIsWarning):
(WebKit::AutofillPopupMenuClient::initialize):
(WebKit::AutofillPopupMenuClient::setSuggestions):
* src/AutofillPopupMenuClient.h:  Removed m_separatorIndex because now we use itemID to identify separators.
Added m_useLegacyBehavior which is true when it is initialized with a valid separator index. This is to keep
the autofill working even when the chromium side hasn't been updated yet.
(AutofillPopupMenuClient):
* src/WebViewImpl.cpp:
(WebKit::WebViewImpl::applyAutofillSuggestions):
* src/WebViewImpl.h:
(WebViewImpl):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@114223 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/public/WebAutofillClient.h
Source/WebKit/chromium/public/WebView.h
Source/WebKit/chromium/src/AutofillPopupMenuClient.cpp
Source/WebKit/chromium/src/AutofillPopupMenuClient.h
Source/WebKit/chromium/src/WebViewImpl.cpp
Source/WebKit/chromium/src/WebViewImpl.h

index 1332c8d1709a474cc8270bdb7e7a223c6752cfe7..04ac5b9c63b6d26304ee5e34edf82d110cd2cddc 100644 (file)
@@ -1,3 +1,41 @@
+2012-04-15  Keishi Hattori  <keishi@webkit.org>
+
+        Introduce MenuItemID to autofill popup
+        https://bugs.webkit.org/show_bug.cgi?id=83777
+
+        Introducing MenuItemIDs because we want to add multiple separators and
+        need to identify non-autofill menu items without resorting to the separator position.
+
+        Reviewed by Kent Tamura.
+
+        * public/WebAutofillClient.h:
+        (WebKit::WebAutofillClient::didAcceptAutofillSuggestion): Changed uniqueID to itemID because they aren't unique.
+        (WebKit::WebAutofillClient::didSelectAutofillSuggestion):
+        * public/WebView.h:
+        (WebView):
+        * src/AutofillPopupMenuClient.cpp:
+        (WebKit::AutofillPopupMenuClient::AutofillPopupMenuClient):
+        (WebKit::AutofillPopupMenuClient::getSuggestionsCount):
+        (WebKit::AutofillPopupMenuClient::getSuggestion):
+        (WebKit::AutofillPopupMenuClient::getLabel):
+        (WebKit::AutofillPopupMenuClient::getIcon):
+        (WebKit::AutofillPopupMenuClient::removeSuggestionAtIndex):
+        (WebKit::AutofillPopupMenuClient::canRemoveSuggestionAtIndex):
+        (WebKit::AutofillPopupMenuClient::valueChanged):
+        (WebKit::AutofillPopupMenuClient::selectionChanged):
+        (WebKit::AutofillPopupMenuClient::itemIsSeparator):
+        (WebKit::AutofillPopupMenuClient::itemIsWarning):
+        (WebKit::AutofillPopupMenuClient::initialize):
+        (WebKit::AutofillPopupMenuClient::setSuggestions):
+        * src/AutofillPopupMenuClient.h:  Removed m_separatorIndex because now we use itemID to identify separators.
+        Added m_useLegacyBehavior which is true when it is initialized with a valid separator index. This is to keep
+        the autofill working even when the chromium side hasn't been updated yet.
+        (AutofillPopupMenuClient):
+        * src/WebViewImpl.cpp:
+        (WebKit::WebViewImpl::applyAutofillSuggestions):
+        * src/WebViewImpl.h:
+        (WebViewImpl):
+
 2012-04-15  James Robinson  <jamesr@chromium.org>
 
         [chromium] LayerRendererChromium shouldn't know anything about CCLayerImpl
index 5824f170a38cff7febc61ffa3c51c8e37852d085..7b48f57dbe46ea37ef6f4994c513cb2f93d687fc 100644 (file)
@@ -40,16 +40,26 @@ class WebString;
 
 class WebAutofillClient {
 public:
+    enum {
+        MenuItemIDAutocompleteEntry = 0,
+        MenuItemIDWarningMessage = -1,
+        MenuItemIDPasswordEntry = -2,
+        MenuItemIDSeparator = -3,
+        MenuItemIDClearForm = -4,
+        MenuItemIDAutofillOptions = -5,
+        MenuItemIDDataListEntry = -6
+    };
+
     // Informs the browser that the user has accepted an Autofill suggestion for
-    // a WebNode.  |uniqueID| is used as a key into the set of Autofill profiles,
-    // and should never be negative.  If it is 0, then the suggestion is an
-    // Autocomplete suggestion; and |value| stores the suggested text.  |index|
-    // is an index of the selected suggestion in the list of suggestions provided
-    // by the client.
+    // a WebNode. A positive |itemID| is a unique id used to identify the set
+    // of Autofill profiles. If it is AutocompleteEntryMenuItemID, then the
+    // suggestion is an Autocomplete suggestion; and |value| stores the
+    // suggested text. |index| is an index of the selected suggestion in the
+    // list of suggestions provided by the client.
     virtual void didAcceptAutofillSuggestion(const WebNode&,
                                              const WebString& value,
                                              const WebString& label,
-                                             int uniqueID,
+                                             int itemID,
                                              unsigned index) { }
 
     // Informs the browser that the user has selected an Autofill suggestion for
@@ -58,7 +68,7 @@ public:
     virtual void didSelectAutofillSuggestion(const WebNode&,
                                              const WebString& name,
                                              const WebString& label,
-                                             int uniqueID) { }
+                                             int itemID) { }
 
     // Informs the browser that the user has cleared the selection from the
     // Autofill suggestions popup. This happens when a user uses the arrow
index 1dfa3a6a77961c10b908398bb12592b52e079afc..bad8b53bdccaed08d72792cc7c8fe3de0b893166 100644 (file)
@@ -350,19 +350,16 @@ public:
     // Autofill  -----------------------------------------------------------
 
     // Notifies the WebView that Autofill suggestions are available for a node.
-    // |uniqueIDs| is a vector of IDs that represent the unique ID of each
-    // Autofill profile in the suggestions popup. If a unique ID is 0, then the
-    // corresponding suggestion comes from Autocomplete rather than Autofill.
-    // If a unique ID is negative, then the corresponding "suggestion" is
-    // actually a user-facing warning, e.g. explaining why Autofill is
-    // unavailable for the current form.
+    // |itemIDs| is a vector of IDs for the menu items. A positive itemID is a
+    // unique ID for the Autofill entries. Other MenuItemIDs are defined in
+    // WebAutofillClient.h
     virtual void applyAutofillSuggestions(
         const WebNode&,
         const WebVector<WebString>& names,
         const WebVector<WebString>& labels,
         const WebVector<WebString>& icons,
-        const WebVector<int>& uniqueIDs,
-        int separatorIndex) = 0;
+        const WebVector<int>& itemIDs,
+        int separatorIndex = -1) = 0;
 
     // Hides any popup (suggestions, selects...) that might be showing.
     virtual void hidePopups() = 0;
index c84ea12f60e9c7539f921907383c46a77a709664..28a7ce7ae5e32309317c8b7b9286ecfe3afc2fc5 100644 (file)
@@ -52,9 +52,9 @@ using namespace WebCore;
 namespace WebKit {
 
 AutofillPopupMenuClient::AutofillPopupMenuClient()
-    : m_separatorIndex(-1)
-    , m_selectedIndex(-1)
+    : m_selectedIndex(-1)
     , m_textField(0)
+    , m_useLegacyBehavior(false)
 {
 }
 
@@ -64,37 +64,25 @@ AutofillPopupMenuClient::~AutofillPopupMenuClient()
 
 unsigned AutofillPopupMenuClient::getSuggestionsCount() const
 {
-    return m_names.size() + ((m_separatorIndex == -1) ? 0 : 1);
+    return m_names.size();
 }
 
 WebString AutofillPopupMenuClient::getSuggestion(unsigned listIndex) const
 {
-    int index = convertListIndexToInternalIndex(listIndex);
-    if (index == -1)
-        return WebString();
-
-    ASSERT(index >= 0 && static_cast<size_t>(index) < m_names.size());
-    return m_names[index];
+    ASSERT(listIndex < m_names.size());
+    return m_names[listIndex];
 }
 
 WebString AutofillPopupMenuClient::getLabel(unsigned listIndex) const
 {
-    int index = convertListIndexToInternalIndex(listIndex);
-    if (index == -1)
-        return WebString();
-
-    ASSERT(index >= 0 && static_cast<size_t>(index) < m_labels.size());
-    return m_labels[index];
+    ASSERT(listIndex < m_labels.size());
+    return m_labels[listIndex];
 }
 
 WebString AutofillPopupMenuClient::getIcon(unsigned listIndex) const
 {
-    int index = convertListIndexToInternalIndex(listIndex);
-    if (index == -1)
-        return WebString();
-
-    ASSERT(index >= 0 && static_cast<size_t>(index) < m_icons.size());
-    return m_icons[index];
+    ASSERT(listIndex < m_icons.size());
+    return m_icons[listIndex];
 }
 
 void AutofillPopupMenuClient::removeSuggestionAtIndex(unsigned listIndex)
@@ -102,26 +90,17 @@ void AutofillPopupMenuClient::removeSuggestionAtIndex(unsigned listIndex)
     if (!canRemoveSuggestionAtIndex(listIndex))
         return;
 
-    int index = convertListIndexToInternalIndex(listIndex);
-
-    ASSERT(static_cast<unsigned>(index) < m_names.size());
-
-    m_names.remove(index);
-    m_labels.remove(index);
-    m_icons.remove(index);
-    m_uniqueIDs.remove(index);
+    ASSERT(listIndex < m_names.size());
 
-    // Shift the separator index if necessary.
-    if (m_separatorIndex != -1)
-        m_separatorIndex--;
+    m_names.remove(listIndex);
+    m_labels.remove(listIndex);
+    m_icons.remove(listIndex);
+    m_itemIDs.remove(listIndex);
 }
 
 bool AutofillPopupMenuClient::canRemoveSuggestionAtIndex(unsigned listIndex)
 {
-    // Only allow deletion of items before the separator that have unique id 0
-    // (i.e. are autocomplete rather than autofill items).
-    int index = convertListIndexToInternalIndex(listIndex);
-    return !m_uniqueIDs[index] && (m_separatorIndex == -1 || listIndex < static_cast<unsigned>(m_separatorIndex));
+    return m_itemIDs[listIndex] == WebAutofillClient::MenuItemIDAutocompleteEntry || m_itemIDs[listIndex] == WebAutofillClient::MenuItemIDPasswordEntry;
 }
 
 void AutofillPopupMenuClient::valueChanged(unsigned listIndex, bool fireEvents)
@@ -130,15 +109,22 @@ void AutofillPopupMenuClient::valueChanged(unsigned listIndex, bool fireEvents)
     if (!webView)
         return;
 
-    if (m_separatorIndex != -1 && listIndex > static_cast<unsigned>(m_separatorIndex))
-        --listIndex;
-
     ASSERT(listIndex < m_names.size());
 
+    if (m_useLegacyBehavior) {
+        for (size_t i = 0; i < m_itemIDs.size(); ++i) {
+            if (m_itemIDs[i] == WebAutofillClient::MenuItemIDSeparator) {
+                if (listIndex > i)
+                    listIndex--;
+                break;
+            }
+        }
+    }
+
     webView->autofillClient()->didAcceptAutofillSuggestion(WebNode(getTextField()),
                                                            m_names[listIndex],
                                                            m_labels[listIndex],
-                                                           m_uniqueIDs[listIndex],
+                                                           m_itemIDs[listIndex],
                                                            listIndex);
 }
 
@@ -148,15 +134,12 @@ void AutofillPopupMenuClient::selectionChanged(unsigned listIndex, bool fireEven
     if (!webView)
         return;
 
-    if (m_separatorIndex != -1 && listIndex > static_cast<unsigned>(m_separatorIndex))
-        --listIndex;
-
     ASSERT(listIndex < m_names.size());
 
     webView->autofillClient()->didSelectAutofillSuggestion(WebNode(getTextField()),
                                                            m_names[listIndex],
                                                            m_labels[listIndex],
-                                                           m_uniqueIDs[listIndex]);
+                                                           m_itemIDs[listIndex]);
 }
 
 void AutofillPopupMenuClient::selectionCleared()
@@ -228,17 +211,12 @@ void AutofillPopupMenuClient::popupDidHide()
 
 bool AutofillPopupMenuClient::itemIsSeparator(unsigned listIndex) const
 {
-    return (m_separatorIndex != -1 && static_cast<unsigned>(m_separatorIndex) == listIndex);
+    return m_itemIDs[listIndex] == WebAutofillClient::MenuItemIDSeparator;
 }
 
 bool AutofillPopupMenuClient::itemIsWarning(unsigned listIndex) const
 {
-    int index = convertListIndexToInternalIndex(listIndex);
-    if (index == -1)
-        return false;
-
-    ASSERT(index >= 0 && static_cast<size_t>(index) < m_uniqueIDs.size());
-    return m_uniqueIDs[index] < 0;
+    return m_itemIDs[listIndex] == WebAutofillClient::MenuItemIDWarningMessage;
 }
 
 void AutofillPopupMenuClient::setTextFromItem(unsigned listIndex)
@@ -269,20 +247,36 @@ void AutofillPopupMenuClient::initialize(
     const WebVector<WebString>& names,
     const WebVector<WebString>& labels,
     const WebVector<WebString>& icons,
-    const WebVector<int>& uniqueIDs,
+    const WebVector<int>& itemIDs,
     int separatorIndex)
 {
     ASSERT(names.size() == labels.size());
     ASSERT(names.size() == icons.size());
-    ASSERT(names.size() == uniqueIDs.size());
-    ASSERT(separatorIndex < static_cast<int>(names.size()));
+    ASSERT(names.size() == itemIDs.size());
 
     m_selectedIndex = -1;
     m_textField = textField;
 
-    // The suggestions must be set before initializing the
-    // AutofillPopupMenuClient.
-    setSuggestions(names, labels, icons, uniqueIDs, separatorIndex);
+    if (separatorIndex == -1) {
+        // The suggestions must be set before initializing the
+        // AutofillPopupMenuClient.
+        setSuggestions(names, labels, icons, itemIDs);
+    } else {
+        m_useLegacyBehavior = true;
+        WebVector<WebString> namesWithSeparator(names.size() + 1);
+        WebVector<WebString> labelsWithSeparator(labels.size() + 1);
+        WebVector<WebString> iconsWithSeparator(icons.size() + 1);
+        WebVector<int> itemIDsWithSeparator(itemIDs.size() + 1);
+        for (size_t i = 0; i < names.size(); ++i) {
+            size_t j = i < static_cast<size_t>(separatorIndex) ? i : i + 1;
+            namesWithSeparator[j] = names[i];
+            labelsWithSeparator[j] = labels[i];
+            iconsWithSeparator[j] = icons[i];
+            itemIDsWithSeparator[j] = itemIDs[i];
+        }
+        itemIDsWithSeparator[separatorIndex] = WebAutofillClient::MenuItemIDSeparator;
+        setSuggestions(namesWithSeparator, labelsWithSeparator, iconsWithSeparator, itemIDsWithSeparator);
+    }
 
     FontDescription regularFontDescription;
     RenderTheme::defaultTheme()->systemFont(CSSValueWebkitControl,
@@ -313,42 +307,28 @@ void AutofillPopupMenuClient::initialize(
 void AutofillPopupMenuClient::setSuggestions(const WebVector<WebString>& names,
                                              const WebVector<WebString>& labels,
                                              const WebVector<WebString>& icons,
-                                             const WebVector<int>& uniqueIDs,
-                                             int separatorIndex)
+                                             const WebVector<int>& itemIDs)
 {
     ASSERT(names.size() == labels.size());
     ASSERT(names.size() == icons.size());
-    ASSERT(names.size() == uniqueIDs.size());
-    ASSERT(separatorIndex < static_cast<int>(names.size()));
+    ASSERT(names.size() == itemIDs.size());
 
     m_names.clear();
     m_labels.clear();
     m_icons.clear();
-    m_uniqueIDs.clear();
+    m_itemIDs.clear();
     for (size_t i = 0; i < names.size(); ++i) {
         m_names.append(names[i]);
         m_labels.append(labels[i]);
         m_icons.append(icons[i]);
-        m_uniqueIDs.append(uniqueIDs[i]);
+        m_itemIDs.append(itemIDs[i]);
     }
 
-    m_separatorIndex = separatorIndex;
-
     // Try to preserve selection if possible.
     if (getSelectedIndex() >= static_cast<int>(names.size()))
         setSelectedIndex(-1);
 }
 
-int AutofillPopupMenuClient::convertListIndexToInternalIndex(unsigned listIndex) const
-{
-    if (listIndex == static_cast<unsigned>(m_separatorIndex))
-        return -1;
-
-    if (m_separatorIndex == -1 || listIndex < static_cast<unsigned>(m_separatorIndex))
-        return listIndex;
-    return listIndex - 1;
-}
-
 WebViewImpl* AutofillPopupMenuClient::getWebView() const
 {
     Frame* frame = m_textField->document()->frame();
index 73cc180e00b4ceb5081595b46230a2fc7a3b89ac..588e19bec7353717ced3a1a892fb24b8d20774af 100644 (file)
@@ -106,20 +106,15 @@ public:
                     const WebVector<WebString>& names,
                     const WebVector<WebString>& labels,
                     const WebVector<WebString>& icons,
-                    const WebVector<int>& uniqueIDs,
+                    const WebVector<int>& itemIDs,
                     int separatorIndex);
 
     void setSuggestions(const WebVector<WebString>& names,
                         const WebVector<WebString>& labels,
                         const WebVector<WebString>& icons,
-                        const WebVector<int>& uniqueIDs,
-                        int separatorIndex);
+                        const WebVector<int>& itemIDs);
 
 private:
-    // Convert the specified index from an index into the visible list (which might
-    // include a separator entry) to an index to |m_names| and |m_labels|.
-    // Returns -1 if the given index points to the separator.
-    int convertListIndexToInternalIndex(unsigned) const;
     WebViewImpl* getWebView() const;
     WebCore::HTMLInputElement* getTextField() const { return m_textField.get(); }
     WebCore::RenderStyle* textFieldStyle() const;
@@ -133,10 +128,7 @@ private:
     Vector<WTF::String> m_names;
     Vector<WTF::String> m_labels;
     Vector<WTF::String> m_icons;
-    Vector<int> m_uniqueIDs;
-
-    // The index of the separator.  -1 if there is no separator.
-    int m_separatorIndex;
+    Vector<int> m_itemIDs;
 
     // The index of the selected item.  -1 if there is no selected item.
     int m_selectedIndex;
@@ -144,6 +136,9 @@ private:
     RefPtr<WebCore::HTMLInputElement> m_textField;
     OwnPtr<WebCore::PopupMenuStyle> m_regularStyle;
     OwnPtr<WebCore::PopupMenuStyle> m_warningStyle;
+
+    // Use legacy behavior while the chromium side hasn't been updated.
+    bool m_useLegacyBehavior;
 };
 
 } // namespace WebKit
index 626883030fa263606468dda2029d5af36ed2d299..c2bbadb9802ac533409b1e6dfb1b21df91bfafae 100644 (file)
@@ -2740,12 +2740,11 @@ void WebViewImpl::applyAutofillSuggestions(
     const WebVector<WebString>& names,
     const WebVector<WebString>& labels,
     const WebVector<WebString>& icons,
-    const WebVector<int>& uniqueIDs,
+    const WebVector<int>& itemIDs,
     int separatorIndex)
 {
     ASSERT(names.size() == labels.size());
-    ASSERT(names.size() == uniqueIDs.size());
-    ASSERT(separatorIndex < static_cast<int>(names.size()));
+    ASSERT(names.size() == itemIDs.size());
 
     if (names.isEmpty()) {
         hideAutofillPopup();
@@ -2770,7 +2769,7 @@ void WebViewImpl::applyAutofillSuggestions(
         m_autofillPopupClient = adoptPtr(new AutofillPopupMenuClient);
 
     m_autofillPopupClient->initialize(
-        inputElem, names, labels, icons, uniqueIDs, separatorIndex);
+        inputElem, names, labels, icons, itemIDs, separatorIndex);
 
     if (!m_autofillPopup) {
         PopupContainerSettings popupSettings = autofillPopupSettings;
index 32379dde5d5cb38cdb9b9fd5f201ce61d8036fbc..981f608e06bed5adcf8d192eded81a1e44063b6b 100644 (file)
@@ -249,7 +249,7 @@ public:
         const WebVector<WebString>& names,
         const WebVector<WebString>& labels,
         const WebVector<WebString>& icons,
-        const WebVector<int>& uniqueIDs,
+        const WebVector<int>& itemIDs,
         int separatorIndex);
     virtual void hidePopups();
     virtual void setScrollbarColors(unsigned inactiveColor,