From 0893f0f1a6606e6068c6604292ba81cb81bea53c Mon Sep 17 00:00:00 2001 From: "keishi@webkit.org" Date: Mon, 16 Apr 2012 05:21:58 +0000 Subject: [PATCH] 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): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@114223 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Source/WebKit/chromium/ChangeLog | 38 ++++++ Source/WebKit/chromium/public/WebAutofillClient.h | 24 ++-- Source/WebKit/chromium/public/WebView.h | 13 +-- .../chromium/src/AutofillPopupMenuClient.cpp | 130 +++++++++------------ .../WebKit/chromium/src/AutofillPopupMenuClient.h | 17 +-- Source/WebKit/chromium/src/WebViewImpl.cpp | 7 +- Source/WebKit/chromium/src/WebViewImpl.h | 2 +- 7 files changed, 125 insertions(+), 106 deletions(-) diff --git a/Source/WebKit/chromium/ChangeLog b/Source/WebKit/chromium/ChangeLog index 1332c8d..04ac5b9 100644 --- a/Source/WebKit/chromium/ChangeLog +++ b/Source/WebKit/chromium/ChangeLog @@ -1,3 +1,41 @@ +2012-04-15 Keishi Hattori + + 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 [chromium] LayerRendererChromium shouldn't know anything about CCLayerImpl diff --git a/Source/WebKit/chromium/public/WebAutofillClient.h b/Source/WebKit/chromium/public/WebAutofillClient.h index 5824f17..7b48f57 100644 --- a/Source/WebKit/chromium/public/WebAutofillClient.h +++ b/Source/WebKit/chromium/public/WebAutofillClient.h @@ -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 diff --git a/Source/WebKit/chromium/public/WebView.h b/Source/WebKit/chromium/public/WebView.h index 1dfa3a6..bad8b53 100644 --- a/Source/WebKit/chromium/public/WebView.h +++ b/Source/WebKit/chromium/public/WebView.h @@ -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& names, const WebVector& labels, const WebVector& icons, - const WebVector& uniqueIDs, - int separatorIndex) = 0; + const WebVector& itemIDs, + int separatorIndex = -1) = 0; // Hides any popup (suggestions, selects...) that might be showing. virtual void hidePopups() = 0; diff --git a/Source/WebKit/chromium/src/AutofillPopupMenuClient.cpp b/Source/WebKit/chromium/src/AutofillPopupMenuClient.cpp index c84ea12..28a7ce7 100644 --- a/Source/WebKit/chromium/src/AutofillPopupMenuClient.cpp +++ b/Source/WebKit/chromium/src/AutofillPopupMenuClient.cpp @@ -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(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(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(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(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(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(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(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(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(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& names, const WebVector& labels, const WebVector& icons, - const WebVector& uniqueIDs, + const WebVector& itemIDs, int separatorIndex) { ASSERT(names.size() == labels.size()); ASSERT(names.size() == icons.size()); - ASSERT(names.size() == uniqueIDs.size()); - ASSERT(separatorIndex < static_cast(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 namesWithSeparator(names.size() + 1); + WebVector labelsWithSeparator(labels.size() + 1); + WebVector iconsWithSeparator(icons.size() + 1); + WebVector itemIDsWithSeparator(itemIDs.size() + 1); + for (size_t i = 0; i < names.size(); ++i) { + size_t j = i < static_cast(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& names, const WebVector& labels, const WebVector& icons, - const WebVector& uniqueIDs, - int separatorIndex) + const WebVector& itemIDs) { ASSERT(names.size() == labels.size()); ASSERT(names.size() == icons.size()); - ASSERT(names.size() == uniqueIDs.size()); - ASSERT(separatorIndex < static_cast(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(names.size())) setSelectedIndex(-1); } -int AutofillPopupMenuClient::convertListIndexToInternalIndex(unsigned listIndex) const -{ - if (listIndex == static_cast(m_separatorIndex)) - return -1; - - if (m_separatorIndex == -1 || listIndex < static_cast(m_separatorIndex)) - return listIndex; - return listIndex - 1; -} - WebViewImpl* AutofillPopupMenuClient::getWebView() const { Frame* frame = m_textField->document()->frame(); diff --git a/Source/WebKit/chromium/src/AutofillPopupMenuClient.h b/Source/WebKit/chromium/src/AutofillPopupMenuClient.h index 73cc180..588e19b 100644 --- a/Source/WebKit/chromium/src/AutofillPopupMenuClient.h +++ b/Source/WebKit/chromium/src/AutofillPopupMenuClient.h @@ -106,20 +106,15 @@ public: const WebVector& names, const WebVector& labels, const WebVector& icons, - const WebVector& uniqueIDs, + const WebVector& itemIDs, int separatorIndex); void setSuggestions(const WebVector& names, const WebVector& labels, const WebVector& icons, - const WebVector& uniqueIDs, - int separatorIndex); + const WebVector& 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 m_names; Vector m_labels; Vector m_icons; - Vector m_uniqueIDs; - - // The index of the separator. -1 if there is no separator. - int m_separatorIndex; + Vector m_itemIDs; // The index of the selected item. -1 if there is no selected item. int m_selectedIndex; @@ -144,6 +136,9 @@ private: RefPtr m_textField; OwnPtr m_regularStyle; OwnPtr m_warningStyle; + + // Use legacy behavior while the chromium side hasn't been updated. + bool m_useLegacyBehavior; }; } // namespace WebKit diff --git a/Source/WebKit/chromium/src/WebViewImpl.cpp b/Source/WebKit/chromium/src/WebViewImpl.cpp index 6268830..c2bbadb 100644 --- a/Source/WebKit/chromium/src/WebViewImpl.cpp +++ b/Source/WebKit/chromium/src/WebViewImpl.cpp @@ -2740,12 +2740,11 @@ void WebViewImpl::applyAutofillSuggestions( const WebVector& names, const WebVector& labels, const WebVector& icons, - const WebVector& uniqueIDs, + const WebVector& itemIDs, int separatorIndex) { ASSERT(names.size() == labels.size()); - ASSERT(names.size() == uniqueIDs.size()); - ASSERT(separatorIndex < static_cast(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; diff --git a/Source/WebKit/chromium/src/WebViewImpl.h b/Source/WebKit/chromium/src/WebViewImpl.h index 32379dd..981f608e 100644 --- a/Source/WebKit/chromium/src/WebViewImpl.h +++ b/Source/WebKit/chromium/src/WebViewImpl.h @@ -249,7 +249,7 @@ public: const WebVector& names, const WebVector& labels, const WebVector& icons, - const WebVector& uniqueIDs, + const WebVector& itemIDs, int separatorIndex); virtual void hidePopups(); virtual void setScrollbarColors(unsigned inactiveColor, -- 2.7.4