AutoFill: Don't parse hidden fields for labels. Also don't count hidden fields

as being auto-fillable.

BUG=46463
TEST=AutoFillManagerTest.HiddenFields, FormManagerTest.LabelsHiddenFields,
FormManagerTest.LabelForElementHidden

Review URL: http://codereview.chromium.org/2809005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@49753 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 207 insertions, 3 deletions

diff --git a/chrome/browser/autofill/autofill_infobar_delegate.cc b/chrome/browser/autofill/autofill_infobar_delegate.cc
index c092249..d0465dc 100644
--- a/chrome/browser/autofill/autofill_infobar_delegate.cc
+++ b/chrome/browser/autofill/autofill_infobar_delegate.cc
@@ -24,7 +24,10 @@ AutoFillInfoBarDelegate::AutoFillInfoBarDelegate(TabContents* tab_contents,
       browser_(NULL),
       host_(host) {
   if (tab_contents) {
-    browser_ = tab_contents->delegate()->GetBrowser();
+    // This is NULL for TestTabContents.
+    if (tab_contents->delegate())
+      browser_ = tab_contents->delegate()->GetBrowser();
+
     PrefService* prefs = tab_contents->profile()->GetPrefs();
     prefs->SetBoolean(prefs::kAutoFillInfoBarShown, true);
     tab_contents->AddInfoBar(this);
diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc
index 51eba37..6754c2a 100644
--- a/chrome/browser/autofill/autofill_manager.cc
+++ b/chrome/browser/autofill/autofill_manager.cc
@@ -23,6 +23,7 @@ using webkit_glue::FormData;
 using webkit_glue::FormField;
 
 namespace {
+
 // We only send a fraction of the forms to upload server.
 // The rate for positive/negative matches potentially could be different.
 const double kAutoFillPositiveUploadRateDefaultValue = 0.01;
@@ -33,6 +34,7 @@ const int kAutoFillPhoneNumberPrefixOffset = 0;
 const int kAutoFillPhoneNumberPrefixCount = 3;
 const int kAutoFillPhoneNumberSuffixOffset = 3;
 const int kAutoFillPhoneNumberSuffixCount = 4;
+
 }  // namespace
 
 // TODO(jhawkins): Maybe this should be in a grd file?
diff --git a/chrome/browser/autofill/autofill_manager_unittest.cc b/chrome/browser/autofill/autofill_manager_unittest.cc
index c1d7c72..33ac34c 100644
--- a/chrome/browser/autofill/autofill_manager_unittest.cc
+++ b/chrome/browser/autofill/autofill_manager_unittest.cc
@@ -14,10 +14,15 @@
 #include "chrome/browser/autofill/autofill_profile.h"
 #include "chrome/browser/autofill/credit_card.h"
 #include "chrome/browser/autofill/personal_data_manager.h"
+#include "chrome/browser/pref_service.h"
+#include "chrome/browser/profile.h"
 #include "chrome/browser/renderer_host/test/test_render_view_host.h"
+#include "chrome/browser/tab_contents/test_tab_contents.h"
 #include "chrome/common/ipc_test_sink.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/common/render_messages.h"
 #include "googleurl/src/gurl.h"
+#include "testing/gtest/include/gtest/gtest.h"
 #include "webkit/glue/form_data.h"
 #include "webkit/glue/form_field.h"
 
@@ -33,6 +38,7 @@ class TestPersonalDataManager : public PersonalDataManager {
   }
 
   virtual void InitializeIfNeeded() {}
+  virtual void SaveImportedFormData() {}
 
   AutoFillProfile* GetLabeledProfile(const char* label) {
     for (std::vector<AutoFillProfile *>::iterator it = web_profiles_.begin();
@@ -201,6 +207,8 @@ class AutoFillManagerTest : public RenderViewHostTestHarness {
     autofill_manager_.reset(new TestAutoFillManager(contents()));
   }
 
+  Profile* profile() { return contents()->profile(); }
+
   bool GetAutoFillSuggestionsMessage(int *page_id,
                                      std::vector<string16>* values,
                                      std::vector<string16>* labels) {
@@ -702,4 +710,60 @@ TEST_F(AutoFillManagerTest, FormChangesAddField) {
   EXPECT_TRUE(field.StrictlyEqualsHack(results.fields[4]));
 }
 
+TEST_F(AutoFillManagerTest, InfoBarShown) {
+  FormData form;
+  form.name = ASCIIToUTF16("MyForm");
+  form.method = ASCIIToUTF16("POST");
+  form.origin = GURL("http://myform.com/form.html");
+  form.action = GURL("http://myform.com/submit.html");
+
+  webkit_glue::FormField field;
+  CreateTestFormField("E-mail", "one", "one", "text", &field);
+  form.fields.push_back(field);
+  CreateTestFormField("E-mail", "two", "two", "text", &field);
+  form.fields.push_back(field);
+  CreateTestFormField("E-mail", "three", "three", "text", &field);
+  form.fields.push_back(field);
+
+  // Set up our FormStructures.
+  std::vector<FormData> forms;
+  forms.push_back(form);
+  autofill_manager_->FormsSeen(forms);
+
+  // Submit the form.
+  autofill_manager_->FormSubmitted(form);
+
+  // Check that the 'AutoFill InfoBar shown' pref is set.
+  PrefService* prefs = profile()->GetPrefs();
+  EXPECT_TRUE(prefs->GetBoolean(prefs::kAutoFillInfoBarShown));
+}
+
+TEST_F(AutoFillManagerTest, HiddenFields) {
+  FormData form;
+  form.name = ASCIIToUTF16("MyForm");
+  form.method = ASCIIToUTF16("POST");
+  form.origin = GURL("http://myform.com/form.html");
+  form.action = GURL("http://myform.com/submit.html");
+
+  webkit_glue::FormField field;
+  CreateTestFormField("E-mail", "one", "one", "hidden", &field);
+  form.fields.push_back(field);
+  CreateTestFormField("E-mail", "two", "two", "hidden", &field);
+  form.fields.push_back(field);
+  CreateTestFormField("E-mail", "three", "three", "hidden", &field);
+  form.fields.push_back(field);
+
+  // Set up our FormStructures.
+  std::vector<FormData> forms;
+  forms.push_back(form);
+  autofill_manager_->FormsSeen(forms);
+
+  // Submit the form.
+  autofill_manager_->FormSubmitted(form);
+
+  // Check that the 'AutoFill InfoBar shown' pref is not set.
+  PrefService* prefs = profile()->GetPrefs();
+  EXPECT_FALSE(prefs->GetBoolean(prefs::kAutoFillInfoBarShown));
+}
+
 }  // namespace
diff --git a/chrome/browser/autofill/form_field.cc b/chrome/browser/autofill/form_field.cc
index f14e172..e4f9a36 100644
--- a/chrome/browser/autofill/form_field.cc
+++ b/chrome/browser/autofill/form_field.cc
@@ -14,6 +14,13 @@
 #include "third_party/WebKit/WebKit/chromium/public/WebRegularExpression.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebString.h"
 
+namespace {
+
+// The name of the hidden form control element.
+const char* const kControlTypeHidden = "hidden";
+
+}  // namespace
+
 class EmailField : public FormField {
  public:
   virtual bool GetFieldInfo(FieldTypeMap* field_type_map) const {
@@ -216,6 +223,13 @@ FormFieldSet::FormFieldSet(FormStructure* fields) {
   // Parse fields.
   std::vector<AutoFillField*>::const_iterator field = fields->begin();
   while (field != fields->end() && *field != NULL) {
+    // Don't parse hidden fields.
+    if (LowerCaseEqualsASCII((*field)->form_control_type(),
+                             kControlTypeHidden)) {
+      field++;
+      continue;
+    }
+
     FormField* form_field = FormField::ParseFormField(&field, is_ecml);
     if (!form_field) {
       field++;
diff --git a/chrome/browser/autofill/personal_data_manager.h b/chrome/browser/autofill/personal_data_manager.h
index 789baac..b8c7f2f 100644
--- a/chrome/browser/autofill/personal_data_manager.h
+++ b/chrome/browser/autofill/personal_data_manager.h
@@ -66,7 +66,7 @@ class PersonalDataManager
 
   // Saves |imported_profile_| and |imported_credit_card_| to the WebDB if they
   // exist.
-  void SaveImportedFormData();
+  virtual void SaveImportedFormData();
 
   // Gets |imported_profile_| and |imported_credit_card_| and returns their
   // values in |profile| and |credit_card| parameters respectively.  One or
diff --git a/chrome/renderer/form_manager.cc b/chrome/renderer/form_manager.cc
index 4008fbc3..e2d0c65 100644
--- a/chrome/renderer/form_manager.cc
+++ b/chrome/renderer/form_manager.cc
@@ -258,6 +258,10 @@ void FormManager::WebFormControlElementToFormField(
 
 // static
 string16 FormManager::LabelForElement(const WebFormControlElement& element) {
+  // Don't scrape labels for hidden elements.
+  if (element.formControlType() == WebString::fromUTF8("hidden"))
+    return string16();
+
   WebNodeList labels = element.document().getElementsByTagName("label");
   for (unsigned i = 0; i < labels.length(); ++i) {
     WebElement e = labels.item(i).to<WebElement>();
@@ -349,7 +353,9 @@ bool FormManager::WebFormElementToFormData(const WebFormElement& element,
     WebLabelElement label = labels.item(i).to<WebLabelElement>();
     WebFormControlElement field_element =
         label.correspondingControl().to<WebFormControlElement>();
-    if (field_element.isNull() || !field_element.isFormControlElement())
+    if (field_element.isNull() ||
+        !field_element.isFormControlElement() ||
+        field_element.formControlType() == WebString::fromUTF8("hidden"))
       continue;
 
     std::map<string16, FormField*>::iterator iter =
@@ -685,6 +691,10 @@ bool FormManager::FormElementToFormData(const WebFrame* frame,
 // static
 string16 FormManager::InferLabelForElement(
     const WebFormControlElement& element) {
+  // Don't scrape labels for hidden elements.
+  if (element.formControlType() == WebString::fromUTF8("hidden"))
+    return string16();
+
   string16 inferred_label = InferLabelFromPrevious(element);
 
   // If we didn't find a label, check for table cell case.
diff --git a/chrome/renderer/form_manager_unittest.cc b/chrome/renderer/form_manager_unittest.cc
index e5b9ffd..88011b6 100644
--- a/chrome/renderer/form_manager_unittest.cc
+++ b/chrome/renderer/form_manager_unittest.cc
@@ -8,6 +8,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebDocument.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebElement.h"
+#include "third_party/WebKit/WebKit/chromium/public/WebFormControlElement.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebFormElement.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebInputElement.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebNode.h"
@@ -17,6 +18,7 @@
 
 using WebKit::WebDocument;
 using WebKit::WebElement;
+using WebKit::WebFormControlElement;
 using WebKit::WebFormElement;
 using WebKit::WebFrame;
 using WebKit::WebInputElement;
@@ -855,6 +857,49 @@ TEST_F(FormManagerTest, LabelsInferredFromText) {
             fields[2]);
 }
 
+TEST_F(FormManagerTest, LabelsInferredFromTextHidden) {
+  LoadHTML("<FORM name=\"TestForm\" action=\"http://cnn.com\" method=\"post\">"
+           "  First name:"
+           "    <INPUT type=\"hidden\" id=\"firstname\" value=\"John\"/>"
+           "  Last name:"
+           "    <INPUT type=\"hidden\" id=\"lastname\" value=\"Smith\"/>"
+           "  <INPUT type=\"submit\" name=\"reply-send\" value=\"Send\"/>"
+           "</FORM>");
+
+  WebFrame* web_frame = GetMainFrame();
+  ASSERT_NE(static_cast<WebFrame*>(NULL), web_frame);
+
+  FormManager form_manager;
+  form_manager.ExtractForms(web_frame);
+
+  std::vector<FormData> forms;
+  form_manager.GetForms(FormManager::REQUIRE_NONE, &forms);
+  ASSERT_EQ(1U, forms.size());
+
+  const FormData& form = forms[0];
+  EXPECT_EQ(ASCIIToUTF16("TestForm"), form.name);
+  EXPECT_EQ(GURL(web_frame->url()), form.origin);
+  EXPECT_EQ(GURL("http://cnn.com"), form.action);
+
+  const std::vector<FormField>& fields = form.fields;
+  ASSERT_EQ(3U, fields.size());
+  EXPECT_TRUE(fields[0].StrictlyEqualsHack(FormField(string16(),
+                                                     ASCIIToUTF16("firstname"),
+                                                     ASCIIToUTF16("John"),
+                                                     ASCIIToUTF16("hidden"),
+                                                     0)));
+  EXPECT_TRUE(fields[1].StrictlyEqualsHack(FormField(string16(),
+                                                     ASCIIToUTF16("lastname"),
+                                                     ASCIIToUTF16("Smith"),
+                                                     ASCIIToUTF16("hidden"),
+                                                     0)));
+  EXPECT_TRUE(fields[2].StrictlyEqualsHack(FormField(string16(),
+                                                     ASCIIToUTF16("reply-send"),
+                                                     string16(),
+                                                     ASCIIToUTF16("submit"),
+                                                     0)));
+}
+
 TEST_F(FormManagerTest, LabelsInferredFromParagraph) {
   LoadHTML("<FORM name=\"TestForm\" action=\"http://cnn.com\" method=\"post\">"
            "  <P>First name:</P><INPUT type=\"text\" "
@@ -2340,4 +2385,70 @@ TEST_F(FormManagerTest, FormWithNodeIsAutoFilled) {
   EXPECT_TRUE(form_manager.FormWithNodeIsAutoFilled(firstname));
 }
 
+TEST_F(FormManagerTest, LabelsHiddenFields) {
+  LoadHTML("<FORM name=\"TestForm\" action=\"http://cnn.com\" method=\"post\">"
+           "  <LABEL for=\"firstname\"> First name: </LABEL>"
+           "    <INPUT type=\"hidden\" id=\"firstname\" value=\"John\"/>"
+           "  <LABEL for=\"lastname\"> Last name: </LABEL>"
+           "    <INPUT type=\"hidden\" id=\"lastname\" value=\"Smith\"/>"
+           "  <INPUT type=\"submit\" name=\"reply-send\" value=\"Send\"/>"
+           "</FORM>");
+
+  WebFrame* web_frame = GetMainFrame();
+  ASSERT_NE(static_cast<WebFrame*>(NULL), web_frame);
+
+  FormManager form_manager;
+  form_manager.ExtractForms(web_frame);
+
+  std::vector<FormData> forms;
+  form_manager.GetForms(FormManager::REQUIRE_NONE, &forms);
+  ASSERT_EQ(1U, forms.size());
+
+  const FormData& form = forms[0];
+  EXPECT_EQ(ASCIIToUTF16("TestForm"), form.name);
+  EXPECT_EQ(GURL(web_frame->url()), form.origin);
+  EXPECT_EQ(GURL("http://cnn.com"), form.action);
+
+  const std::vector<FormField>& fields = form.fields;
+  ASSERT_EQ(3U, fields.size());
+  EXPECT_TRUE(fields[0].StrictlyEqualsHack(
+      FormField(string16(),
+                ASCIIToUTF16("firstname"),
+                ASCIIToUTF16("John"),
+                ASCIIToUTF16("hidden"),
+                0)));
+  EXPECT_TRUE(fields[1].StrictlyEqualsHack(
+      FormField(string16(),
+                ASCIIToUTF16("lastname"),
+                ASCIIToUTF16("Smith"),
+                ASCIIToUTF16("hidden"),
+                0)));
+  EXPECT_TRUE(fields[2].StrictlyEqualsHack(
+      FormField(string16(),
+                ASCIIToUTF16("reply-send"),
+                string16(),
+                ASCIIToUTF16("submit"),
+                0)));
+}
+
+TEST_F(FormManagerTest, LabelForElementHidden) {
+  LoadHTML("<FORM name=\"TestForm\" action=\"http://cnn.com\" method=\"post\">"
+           "  <LABEL for=\"firstname\"> First name: </LABEL>"
+           "    <INPUT type=\"hidden\" id=\"firstname\" value=\"John\"/>"
+           "  <LABEL for=\"lastname\"> Last name: </LABEL>"
+           "    <INPUT type=\"hidden\" id=\"lastname\" value=\"Smith\"/>"
+           "  <INPUT type=\"submit\" name=\"reply-send\" value=\"Send\"/>"
+           "</FORM>");
+
+  WebFrame* web_frame = GetMainFrame();
+  ASSERT_NE(static_cast<WebFrame*>(NULL), web_frame);
+
+  WebElement e = web_frame->document().getElementById("firstname");
+  WebFormControlElement firstname = e.to<WebFormControlElement>();
+
+  // Hidden form control element should not have a label set.
+  FormManager form_manager;
+  EXPECT_EQ(string16(), form_manager.LabelForElement(firstname));
+}
+
 }  // namespace