Show display warning if the form is autocomplete off.

chrome/renderer/autofill/autofill_agent.cc

 // Copyright (c) 2011 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/message_loop.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/common/autofill_messages.h"
 #include "chrome/common/chrome_constants.h"
@@ skipping 12 matching lines @@
 #include "webkit/glue/form_data_predictions.h"
 #include "webkit/glue/form_field.h"
 #include "webkit/glue/password_form.h"
 
 using WebKit::WebFormControlElement;
 using WebKit::WebFormElement;
 using WebKit::WebFrame;
 using WebKit::WebInputElement;
 using WebKit::WebKeyboardEvent;
 using WebKit::WebNode;
+using WebKit::WebElement;
 using WebKit::WebString;
 using webkit_glue::FormData;
 using webkit_glue::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;
 
+bool autoCompleteOff(const WebNode& node) {

[Ilya Sherman, 2011/07/27 23:41:30]

nit: Chromium function names start with capital letters, and boolean function
names typically start with "Is".  Hence, I would name this function something
along the lines of "IsAutocompleteDisabled()"

Alternately, you could just update the type for autofill_query_node_ to be a
WebInputElement, and skip these type checks and casts.

[honten.org, 2011/07/28 06:03:06]

Change the type.

On 2011/07/27 23:41:30, Ilya Sherman wrote:

+  if (node.isNull())
+    return false;
+
+  if (node.nodeType() != WebNode::ElementNode &&
+      node.nodeType() != WebNode::TextNode)

[Ilya Sherman, 2011/07/27 23:41:30]

I don't think you want TextNode here...

+    return false;
+
+  const WebElement element = node.toConst<WebElement>();
+  const WebInputElement* input_element = toWebInputElement(&element);
+  return input_element && !input_element->autoComplete();
+}
+
 }  // namespace
 
 namespace autofill {
 
 AutofillAgent::AutofillAgent(
     RenderView* render_view,
     PasswordAutofillManager* password_autofill_manager)
     : RenderViewObserver(render_view),
       password_autofill_manager_(password_autofill_manager),
       autofill_query_id_(0),
@@ skipping 174 matching lines @@
     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;
 
-  if (ids[0] < 0 && ids.size() > 1) {
+  if (autoCompleteOff(autofill_query_node_)) {
+    // If autofill is disabled and we had suggestions, show a warning instead.
+    v.assign(1,
+             l10n_util::GetStringUTF16(IDS_AUTOFILL_WARNING_FORM_DISABLED));

[Ilya Sherman, 2011/07/27 23:41:30]

nit: This looks like it could fit on a single line.

[honten.org, 2011/07/28 06:03:06]

Done.

+    l.assign(1, string16());
+    i.assign(1, string16());
+    ids.assign(1, -1);
+  } 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.
   if (ids[0] < 0 && !display_warning_if_disabled_)
@@ skipping 65 matching lines @@
     const std::vector<FormDataPredictions>& forms) {
   for (size_t i = 0; i < forms.size(); ++i) {
     form_manager_.ShowPredictions(forms[i]);
   }
 }
 
 void AutofillAgent::ShowSuggestions(const WebInputElement& element,
                                     bool autofill_on_empty_values,
                                     bool requires_caret_at_end,
                                     bool display_warning_if_disabled) {
-  if (!element.isEnabled() || element.isReadOnly() || !element.autoComplete() ||
+  const WebFormElement form = element.form();
+  if (!element.isEnabled() || element.isReadOnly() ||
+      (!element.autoComplete() && form.autoComplete()) ||

[Ilya Sherman, 2011/07/27 23:41:30]

nit: This line is rather confusing at a glance, and could use an explanatory
comment.

[honten.org, 2011/07/28 00:04:50]

Should I use extra bool flag like this?

// We should not show warning message for disabled autocomple when
// field autocomplete is off and form autocomplete is on.
bool disabled_autocomplete_off_warning = !element.autoComplete() 
    && form.autocomplete();

Or just adding comment is fine?

On 2011/07/27 23:41:30, Ilya Sherman wrote:

[Ilya Sherman, 2011/07/28 00:29:33]

This comment only describes part of the intent.  The full intent is better
captured by something along the lines of "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."
Just adding a comment is fine, though you're welcome to break this logic out
into a boolean variable if you think that is more readable.  If you prefer to
add a variable, please try to find a clearer name for it. 
"disabled_autocomplete_off_warning" reads like a noun (with modifiers), whereas
boolean variables -- like boolean functions -- should typically read like full
sentences in the form of a question.

[honten.org, 2011/07/28 06:03:06]

Just added comment.

On 2011/07/28 00:29:33, Ilya Sherman wrote:

       !element.isTextField() || element.isPasswordField() ||
       !element.suggestedValue().isEmpty())
     return;
 
   // If the field has no name, then we won't have values.
   if (element.nameForAutofill().isEmpty())
     return;
 
   // Don't attempt to autofill with values that are too large.
   WebString value = element.value();
@@ skipping 63 matching lines @@
   // WebFormControlElementToFormField does not scrape the DOM for the field
   // label, so find the label here.
   // TODO(isherman): Add form and field identities so we can use the cached form
   // data in FormManager.
   field->label = FormManager::LabelForElement(element);
 
   return true;
 }
 
 }  // namespace autofill