DataList UI (Chromium part)

chrome/renderer/autofill/autofill_agent.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/renderer/autofill/autofill_agent.h"
 
 #include "base/bind.h"
 #include "base/message_loop.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/common/autofill_messages.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/renderer/autofill/form_autofill_util.h"
 #include "chrome/renderer/autofill/password_autofill_manager.h"
 #include "content/public/renderer/render_view.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebRect.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebDataListElement.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebDocument.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebFormControlElement.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebFrame.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebInputEvent.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebNode.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebNodeCollection.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebOptionElement.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebView.h"
-#include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebRect.h"
 #include "ui/base/keycodes/keyboard_codes.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "webkit/forms/form_data.h"
 #include "webkit/forms/form_data_predictions.h"
 #include "webkit/forms/form_field.h"
 #include "webkit/forms/password_form.h"
 
+using WebKit::WebDataListElement;
 using WebKit::WebFormControlElement;
 using WebKit::WebFormElement;
 using WebKit::WebFrame;
 using WebKit::WebInputElement;
 using WebKit::WebKeyboardEvent;
 using WebKit::WebNode;
+using WebKit::WebNodeCollection;
+using WebKit::WebOptionElement;
 using WebKit::WebString;
 using webkit::forms::FormData;
 using webkit::forms::FormDataPredictions;
 
 namespace {
 
 // The size above which we stop triggering autofill for an input text field
 // (so to avoid sending long strings through IPC).
 const size_t kMaximumTextSizeForAutofill = 1000;
 
 }  // namespace
 
 namespace autofill {
 
 AutofillAgent::AutofillAgent(
     content::RenderView* render_view,
     PasswordAutofillManager* password_autofill_manager)
     : content::RenderViewObserver(render_view),
       password_autofill_manager_(password_autofill_manager),
       autofill_query_id_(0),
       autofill_action_(AUTOFILL_NONE),
       display_warning_if_disabled_(false),
       was_query_node_autofilled_(false),
-      suggestions_clear_index_(-1),
-      suggestions_options_index_(-1),
       has_shown_autofill_popup_for_current_edit_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
   render_view->GetWebView()->setAutofillClient(this);
 }
 
 AutofillAgent::~AutofillAgent() {}
 
 bool AutofillAgent::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(AutofillAgent, message)
@@ skipping 66 matching lines @@
 
 bool AutofillAgent::InputElementLostFocus() {
   Send(new AutofillHostMsg_HideAutofillPopup(routing_id()));
 
   return false;
 }
 
 void AutofillAgent::didAcceptAutofillSuggestion(const WebNode& node,
                                                 const WebString& value,
                                                 const WebString& label,
-                                                int unique_id,
+                                                int item_id,
                                                 unsigned index) {
   if (password_autofill_manager_->DidAcceptAutofillSuggestion(node, value))
     return;
 
   DCHECK(node == autofill_query_element_);
 
-  if (suggestions_options_index_ != -1 &&
-      index == static_cast<unsigned>(suggestions_options_index_)) {
-    // User selected 'Autofill Options'.

[Ilya Sherman, 2012/04/11 00:18:04]

nit: Please preserve these comments.  I admit that they're fairly clear from the
code as written, but they still add some value in that they map to the user
actions that cause us to reach the code.

[keishi, 2012/04/11 11:48:08]

Done.

-    Send(new AutofillHostMsg_ShowAutofillDialog(routing_id()));
-  } else if (suggestions_clear_index_ != -1 &&
-             index == static_cast<unsigned>(suggestions_clear_index_)) {
-    // User selected 'Clear form'.
-    form_cache_.ClearFormWithElement(autofill_query_element_);
-  } else if (!unique_id) {
-    // User selected an Autocomplete entry, so we fill directly.
-    WebInputElement element = node.toConst<WebInputElement>();
-    SetNodeText(value, &element);
-  } else {
-    // Fill the values for the whole form.
-    FillAutofillFormData(node, unique_id, AUTOFILL_FILL);
+  switch (item_id) {
+    case WebKit::WarningMessageMenuItemID:

[Ilya Sherman, 2012/04/11 00:18:04]

If we are going to be relying on all warnings being assigned this constant id,
please also update the code in chrome/browser/autofill/ (in particular,
autofill_manager.cc and autofill_external_delegate.cc) to use this named
constant.

[keishi, 2012/04/11 11:48:08]

Done.

+    case WebKit::SeparatorMenuItemID: {
+      NOTREACHED();
+      break;
+    }

[Ilya Sherman, 2012/04/11 00:18:04]

nit: No need for curly braces.

[keishi, 2012/04/11 11:48:08]

Done.

+    case WebKit::AutofillOptionsMenuItemID: {
+      Send(new AutofillHostMsg_ShowAutofillDialog(routing_id()));
+      break;
+    } 
+    case WebKit::ClearFormMenuItemID: {
+      form_cache_.ClearFormWithElement(autofill_query_element_);
+      break;
+    }
+    case WebKit::AutocompleteEntryMenuItemID:
+    case WebKit::PasswordEntryMenuItemID:
+    case WebKit::DataListEntryMenuItemID: {
+      WebInputElement element = node.toConst<WebInputElement>();

[Ilya Sherman, 2012/04/11 00:18:04]

nit: Perhaps we can skip this cast by using |autofill_query_element_|?  Also, it
seems rather bad that the constness of |node| is completely ignored... we should
probably update the method signature not to take |node| as a const value.

[keishi, 2012/04/11 11:48:08]

Done.

+      SetNodeText(value, &element);
+      break;
+    }
+    default: {
+      // Fill the values for the whole form.

[Ilya Sherman, 2012/04/11 00:18:04]

nit: It's probably a good idea to DCHECK_GT(item_id, 0) and mention in a comment
that positive ids are unique ids for Autofill (vs. Autocomplete) suggestions.

[keishi, 2012/04/11 11:48:08]

Done.

+      FillAutofillFormData(node, item_id, AUTOFILL_FILL);
+    }
   }
-
-  suggestions_clear_index_ = -1;
-  suggestions_options_index_ = -1;
 }
 
 void AutofillAgent::didSelectAutofillSuggestion(const WebNode& node,
                                                 const WebString& value,
                                                 const WebString& label,
-                                                int unique_id) {
-  DCHECK_GE(unique_id, 0);
+                                                int item_id) {
+  DCHECK_GE(item_id, 0);

[Ilya Sherman, 2012/04/11 00:18:04]

Is this still valid?  Which sorts of items are selectable?  Presumably at least
password items -- with id "-2" -- should be, given the if-stmt immediately
below.

[keishi, 2012/04/11 11:48:08]

No, it was wrong. Selecting a non autofill entry should clear the autofill
preview.

   if (password_autofill_manager_->DidSelectAutofillSuggestion(node))
     return;
 
   didClearAutofillSelection(node);
-  FillAutofillFormData(node, unique_id, AUTOFILL_PREVIEW);
+  FillAutofillFormData(node, item_id, AUTOFILL_PREVIEW);
 }
 
 void AutofillAgent::didClearAutofillSelection(const WebNode& node) {
   if (password_autofill_manager_->DidClearAutofillSelection(node))
     return;
 
   if (!autofill_query_element_.isNull() && node == autofill_query_element_) {
     ClearPreviewedFormWithElement(autofill_query_element_,
                                   was_query_node_autofilled_);
   } else {
     // TODO(isherman): There seem to be rare cases where this code *is*
     // reachable: see [ http://crbug.com/96321#c6 ].  Ideally we would
     // understand those cases and fix the code to avoid them.  However, so far I
     // have been unable to reproduce such a case locally.  If you hit this
     // NOTREACHED(), please file a bug against me.
     NOTREACHED();
   }
 }
 
 void AutofillAgent::removeAutocompleteSuggestion(const WebString& name,
                                                  const WebString& value) {
-  // The index of clear & options will have shifted down.
-  if (suggestions_clear_index_ != -1)
-    suggestions_clear_index_--;
-  if (suggestions_options_index_ != -1)
-    suggestions_options_index_--;
-
   Send(new AutofillHostMsg_RemoveAutocompleteEntry(routing_id(), name, value));
 }
 
 void AutofillAgent::textFieldDidEndEditing(const WebInputElement& element) {
   password_autofill_manager_->TextFieldDidEndEditing(element);
   has_shown_autofill_popup_for_current_edit_ = false;
   Send(new AutofillHostMsg_DidEndTextFieldEditing(routing_id()));
 }
 
 void AutofillAgent::textFieldDidChange(const WebInputElement& element) {
@@ skipping 47 matching lines @@
   if (values.empty()) {
     // No suggestions, any popup currently showing is obsolete.
     web_view->hidePopups();
     return;
   }
 
   std::vector<string16> v(values);
   std::vector<string16> l(labels);
   std::vector<string16> i(icons);
   std::vector<int> ids(unique_ids);
-  int separator_index = -1;
 
   DCHECK_GT(ids.size(), 0U);
   if (!autofill_query_element_.isNull() &&
       !autofill_query_element_.autoComplete()) {
     // If autofill is disabled and we had suggestions, show a warning instead.
     v.assign(1, l10n_util::GetStringUTF16(IDS_AUTOFILL_WARNING_FORM_DISABLED));
     l.assign(1, string16());
     i.assign(1, string16());
-    ids.assign(1, -1);
+    ids.assign(1, WebKit::WarningMessageMenuItemID);
   } else if (ids[0] < 0 && ids.size() > 1) {
     // If we received a warning instead of suggestions from autofill but regular
     // suggestions from autocomplete, don't show the autofill warning.
     v.erase(v.begin());
     l.erase(l.begin());
     i.erase(i.begin());
     ids.erase(ids.begin());
   }
 
   // If we were about to show a warning and we shouldn't, don't.
@@ skipping 10 matching lines @@
     }
   }
 
   // The form has been auto-filled, so give the user the chance to clear the
   // form.  Append the 'Clear form' menu item.
   if (has_autofill_item &&
       FormWithElementIsAutofilled(autofill_query_element_)) {
     v.push_back(l10n_util::GetStringUTF16(IDS_AUTOFILL_CLEAR_FORM_MENU_ITEM));
     l.push_back(string16());
     i.push_back(string16());
-    ids.push_back(0);
-    suggestions_clear_index_ = v.size() - 1;
-    separator_index = v.size() - 1;
+    ids.push_back(WebKit::ClearFormMenuItemID);
+
+    v.push_back(string16());
+    l.push_back(string16());
+    i.push_back(string16());
+    ids.push_back(WebKit::SeparatorMenuItemID);

[Ilya Sherman, 2012/04/11 00:18:04]

This was not actually showing up as a separator in the mac build [1] you linked
to.  Was that build compiled before you added this, or is there something going
wrong here?
[1] https://docs.google.com/open?id=0BwRi59l_ri74a1N4d2NSYXlIdzg

[keishi, 2012/04/11 11:48:08]

The separator should come before the menu item. Fixed.

   }
 
   if (has_autofill_item) {
     // Append the 'Chrome Autofill options' menu item;
     v.push_back(l10n_util::GetStringUTF16(IDS_AUTOFILL_OPTIONS_POPUP));
     l.push_back(string16());
     i.push_back(string16());
-    ids.push_back(0);
-    suggestions_options_index_ = v.size() - 1;
-    separator_index = values.size();
+    ids.push_back(WebKit::AutofillOptionsMenuItemID);
+
+    v.push_back(string16());
+    l.push_back(string16());
+    i.push_back(string16());
+    ids.push_back(WebKit::SeparatorMenuItemID);

[Ilya Sherman, 2012/04/11 00:18:04]

This can add two separators where we should have only one: the "Clear form" and
"Autofill options" items should have only one separator above them (and no
separators between them).

[keishi, 2012/04/11 11:48:08]

Done.

   }
 
   // Send to WebKit for display.
   if (!v.empty() && !autofill_query_element_.isNull() &&
       autofill_query_element_.isFocusable()) {
-    web_view->applyAutofillSuggestions(
-        autofill_query_element_, v, l, i, ids, separator_index);
+    CombineDataListEntriesAndShow(autofill_query_element_, v, l, i, ids);
   }
 
   Send(new AutofillHostMsg_DidShowAutofillSuggestions(
       routing_id(),
       has_autofill_item && !has_shown_autofill_popup_for_current_edit_));
   has_shown_autofill_popup_for_current_edit_ |= has_autofill_item;
 }
 
+void AutofillAgent::CombineDataListEntriesAndShow(const WebKit::WebInputElement& element,
+                                      const std::vector<string16>& values,
+                                      const std::vector<string16>& labels,
+                                      const std::vector<string16>& icons,
+                                      const std::vector<int>& item_ids) {

[Ilya Sherman, 2012/04/11 00:18:04]

nit: 80-col, wrapping

[keishi, 2012/04/11 11:48:08]

Done.

+  std::vector<string16> v;
+  std::vector<string16> l;
+  std::vector<string16> i;
+  std::vector<int> ids;
+                                        
+  WebDataListElement dataList = element.dataList();
+  if (!dataList.isNull()) {
+    WebNodeCollection options = dataList.options();
+    if (options.length() > 0) {

[Ilya Sherman, 2012/04/11 00:18:04]

nit: Is this check needed, or would it also be covered by the "!option.isNull()"
check in the while loop?

[keishi, 2012/04/11 11:48:08]

True! Removed.

+      WebOptionElement option = options.firstItem().to<WebOptionElement>();
+      while (!option.isNull()) {
+        v.push_back(option.value());
+        if (option.value() != option.label())
+          l.push_back(option.label());
+        else
+          l.push_back(string16());
+        i.push_back(string16());
+        ids.push_back(WebKit::DataListEntryMenuItemID);
+        option = options.nextItem().to<WebOptionElement>();
+      }
+    }
+  }

[Ilya Sherman, 2012/04/11 00:18:04]

nit: This code has a lot of levels of nesting.  That could be reduced by
decomposing this to a helper function with early returns.

[keishi, 2012/04/11 11:48:08]

Done.

+
+  if (v.size() > 0 && values.size() > 0) {

[Ilya Sherman, 2012/04/11 00:18:04]

nit: Please add a comment above this if-stmt -- something like "If there are
both <datalist> items and Autofill suggestions, add a separator between them."

[keishi, 2012/04/11 11:48:08]

Done.

+    v.push_back(string16());
+    l.push_back(string16());
+    i.push_back(string16());
+    ids.push_back(WebKit::SeparatorMenuItemID);
+  }
+
+  v.insert(v.end(), values.begin(), values.end());
+  l.insert(l.end(), labels.begin(), labels.end());
+  i.insert(i.end(), icons.begin(), icons.end());
+  ids.insert(ids.end(), item_ids.begin(), item_ids.end());
+  
+  render_view()->GetWebView()->applyAutofillSuggestions(element, v, l, i, ids);
+}
+
 void AutofillAgent::OnFormDataFilled(int query_id,
                                      const webkit::forms::FormData& form) {
   if (!render_view()->GetWebView() || query_id != autofill_query_id_)
     return;
 
   was_query_node_autofilled_ = autofill_query_element_.isAutofilled();
 
   switch (autofill_action_) {
     case AUTOFILL_FILL:
       FillForm(form, autofill_query_element_);
@@ skipping 52 matching lines @@
   bool handled = password_autofill_manager_->DidAcceptAutofillSuggestion(
       autofill_query_element_,
       value);
   DCHECK(handled);
 }
 
 void AutofillAgent::ShowSuggestions(const WebInputElement& element,
                                     bool autofill_on_empty_values,
                                     bool requires_caret_at_end,
                                     bool display_warning_if_disabled) {
-  // If autocomplete is disabled at the form level, then we might want to show
-  // a warning in place of suggestions. However, if autocomplete is disabled
-  // specifically for this field, we never want to show a warning. Otherwise,
-  // we might interfere with custom popups (e.g. search suggestions) used by
-  // the website.
-  const WebFormElement form = element.form();
-  if (!element.isEnabled() || element.isReadOnly() ||
-      (!element.autoComplete() && (form.isNull() || form.autoComplete())) ||
-      !element.isTextField() || element.isPasswordField() ||
-      !element.suggestedValue().isEmpty())
-    return;
-
-  // If the field has no name, then we won't have values.
-  if (element.nameForAutofill().isEmpty())
+  if (!element.isEnabled() || element.isReadOnly() || !element.isTextField() || element.isPasswordField() || !element.suggestedValue().isEmpty())

[Ilya Sherman, 2012/04/11 00:18:04]

nit: 80-col

[keishi, 2012/04/11 11:48:08]

Done.

     return;
 
   // Don't attempt to autofill with values that are too large or if filling
   // criteria are not met.
   WebString value = element.value();
   if (value.length() > kMaximumTextSizeForAutofill ||
       (!autofill_on_empty_values && value.isEmpty()) ||
       (requires_caret_at_end &&
        (element.selectionStart() != element.selectionEnd() ||
         element.selectionEnd() != static_cast<int>(value.length())))) {
     // Any popup currently showing is obsolete.
     WebKit::WebView* web_view = render_view()->GetWebView();
     if (web_view)
       web_view->hidePopups();
 
     return;
   }
+  
+  // If autocomplete is disabled at the form level, then we might want to show
+  // a warning in place of suggestions. However, if autocomplete is disabled
+  // specifically for this field, we never want to show a warning. Otherwise,
+  // we might interfere with custom popups (e.g. search suggestions) used by
+  // the website.
+  const WebFormElement form = element.form();
+  if ((!element.autoComplete() && (form.isNull() || form.autoComplete())) ||
+      // If the field has no name, then we won't have values.

[Ilya Sherman, 2012/04/11 00:18:04]

nit: Please move this comment to be above the if-stmt.

[keishi, 2012/04/11 11:48:08]

Done.

+      element.nameForAutofill().isEmpty()) {
+    CombineDataListEntriesAndShow(element, std::vector<string16>(), std::vector<string16>(), std::vector<string16>(), std::vector<int>());

[Ilya Sherman, 2012/04/11 00:18:04]

nit: 80-col.

[keishi, 2012/04/11 11:48:08]

Done.

+    return;
+  }
 
   QueryAutofillSuggestions(element, display_warning_if_disabled);
 }
 
 void AutofillAgent::QueryAutofillSuggestions(const WebInputElement& element,
                                              bool display_warning_if_disabled) {
   static int query_counter = 0;
   autofill_query_id_ = query_counter++;
   autofill_query_element_ = element;
   display_warning_if_disabled_ = display_warning_if_disabled;
@@ skipping 11 matching lines @@
 
   Send(new AutofillHostMsg_QueryFormFieldAutofill(routing_id(),
                                                   autofill_query_id_,
                                                   form,
                                                   field,
                                                   bounding_box,
                                                   display_warning_if_disabled));
 }
 
 void AutofillAgent::FillAutofillFormData(const WebNode& node,
-                                         int unique_id,
+                                         int item_id,
                                          AutofillAction action) {
   static int query_counter = 0;
   autofill_query_id_ = query_counter++;
 
   webkit::forms::FormData form;
   webkit::forms::FormField field;
   if (!FindFormAndFieldForInputElement(node.toConst<WebInputElement>(), &form,
                                        &field, REQUIRE_AUTOCOMPLETE)) {
     return;
   }
 
   autofill_action_ = action;
   Send(new AutofillHostMsg_FillAutofillFormData(
-      routing_id(), autofill_query_id_, form, field, unique_id));
+      routing_id(), autofill_query_id_, form, field, item_id));
 }
 
 void AutofillAgent::SetNodeText(const string16& value,
                                 WebKit::WebInputElement* node) {
   string16 substring = value;
   substring = substring.substr(0, node->maxLength());
 
   node->setValue(substring, true);
 }
 
 }  // namespace autofill