[Autofill] Update the autocomplete types implementation to match the current HTML spec.

chrome/browser/autofill/form_structure.cc

Index: chrome/browser/autofill/form_structure.cc
diff --git a/chrome/browser/autofill/form_structure.cc b/chrome/browser/autofill/form_structure.cc
index 07af74814c5b73571643406b3e44d9b7dbed6f24..7f8f45dd2fb9943caef12d4b4ea0c2681ab62432 100644
--- a/chrome/browser/autofill/form_structure.cc
+++ b/chrome/browser/autofill/form_structure.cc
@@ -85,147 +85,138 @@ std::string EncodeFieldTypes(const FieldTypeSet& available_field_types) {
   return data_presence;
 }
 
-bool UpdateFromAutocompleteType(const string16& autocomplete_type,
-                                AutofillField* field) {
-  if (autocomplete_type == ASCIIToUTF16("given-name")) {
-    field->set_heuristic_type(NAME_FIRST);
-    return true;
-  }
+// Returns |true| iff the |token| is a type hint for a contact field, as
+// specified in the implementation section of http://is.gd/whatwg_autocomplete
+// Note that "fax" and "pager" are intentionally ignored, as Chrome does not
+// support filling either type of information.
+bool IsContactTypeHint(const string16& token) {
+  return
+      token == ASCIIToUTF16("home") ||

[Evan Stade, 2012/10/19 20:36:47]

use LowerCaseEqualsASCII (here and many other places)?

[Evan Stade, 2012/10/19 20:38:13]

(meant to delete this comment)

+      token == ASCIIToUTF16("work") ||
+      token == ASCIIToUTF16("mobile") ||
+      token == ASCIIToUTF16("pager");
+}
 
-  if (autocomplete_type == ASCIIToUTF16("middle-name")) {
-    field->set_heuristic_type(NAME_MIDDLE);
-    return true;
+// Returns |true| iff the |token| is a type hint appropriate for a field of the
+// given |field_type|, as specified in the implementation section of
+// http://is.gd/whatwg_autocomplete
+bool ContactTypeHintMatchesFieldType(const string16& token,
+                                     AutofillFieldType field_type) {
+  // The "home" and "work" type hints are only appropriate for email and phone
+  // number field types.
+  if (token == ASCIIToUTF16("home") || token == ASCIIToUTF16("work")) {
+    return field_type == EMAIL_ADDRESS ||
+        (field_type >= PHONE_HOME_NUMBER &&

[Evan Stade, 2012/10/19 20:36:47]

why is it called PHONE_HOME if it applies to work and mobile as well?

[Ilya Sherman, 2012/10/20 05:16:08]

Historical reasons.  We used to have more phone number types, but streamlined
them down to just this one.  We've kept the name to remain in sync with the
server code, which still supports all of the original types.

The same is true of addresses: we used to have separate types for billing
addresses; but now we always canonicalize to ADDRESS_HOME_*.

+         field_type <= PHONE_HOME_WHOLE_NUMBER);
   }
 
-  if (autocomplete_type == ASCIIToUTF16("middle-initial")) {
-    field->set_heuristic_type(NAME_MIDDLE_INITIAL);
-    return true;
+  // The "mobile" type hint is only appropriate for phone number field types.
+  // Note that "fax" and "pager" are intentionally ignored, as Chrome does not
+  // support filling either type of information.
+  if (token == ASCIIToUTF16("mobile")) {
+    return field_type >= PHONE_HOME_NUMBER &&
+        field_type <= PHONE_HOME_WHOLE_NUMBER;
   }
 
-  if (autocomplete_type == ASCIIToUTF16("surname")) {
-    field->set_heuristic_type(NAME_LAST);
-    return true;
-  }
+  return false;
+}
 
-  if (autocomplete_type == ASCIIToUTF16("full-name")) {
-    field->set_heuristic_type(NAME_FULL);
-    return true;
+// Returns the Chrome Autofill-supported field type corresponding to the given
+// |autocomplete_type|, if there is one, in the context of the given |field|.
+// Chrome Autofill supports a subset of the field types listed at
+// http://is.gd/whatwg_autocomplete
+AutofillFieldType FieldTypeFromAutocompleteType(
+    const string16& autocomplete_type,
+    const AutofillField& field) {
+  if (autocomplete_type == ASCIIToUTF16("name"))
+    return NAME_FULL;
+
+  if (autocomplete_type == ASCIIToUTF16("given-name"))
+    return NAME_FIRST;
+
+  if (autocomplete_type == ASCIIToUTF16("additional-name")) {
+    if (field.max_length == 1)
+      return NAME_MIDDLE_INITIAL;
+    else
+      return NAME_MIDDLE;
   }
 
-  if (autocomplete_type == ASCIIToUTF16("street-address") ||
-      autocomplete_type == ASCIIToUTF16("address-line1")) {
-    field->set_heuristic_type(ADDRESS_HOME_LINE1);
-    return true;
-  }
+  if (autocomplete_type == ASCIIToUTF16("family-name"))
+    return NAME_LAST;
 
-  if (autocomplete_type == ASCIIToUTF16("address-line2")) {
-    field->set_heuristic_type(ADDRESS_HOME_LINE2);
-    return true;
-  }
+  if (autocomplete_type == ASCIIToUTF16("honorific-suffix"))
+    return NAME_SUFFIX;
 
-  if (autocomplete_type == ASCIIToUTF16("locality") ||
-      autocomplete_type == ASCIIToUTF16("city")) {
-    field->set_heuristic_type(ADDRESS_HOME_CITY);
-    return true;
-  }
+  if (autocomplete_type == ASCIIToUTF16("organization"))
+    return COMPANY_NAME;
 
-  if (autocomplete_type == ASCIIToUTF16("administrative-area") ||
-      autocomplete_type == ASCIIToUTF16("state") ||
-      autocomplete_type == ASCIIToUTF16("province") ||
-      autocomplete_type == ASCIIToUTF16("region")) {
-    field->set_heuristic_type(ADDRESS_HOME_STATE);
-    return true;
-  }
+  if (autocomplete_type == ASCIIToUTF16("street-address") ||
+      autocomplete_type == ASCIIToUTF16("address-line1"))
+    return ADDRESS_HOME_LINE1;
 
-  if (autocomplete_type == ASCIIToUTF16("postal-code")) {
-    field->set_heuristic_type(ADDRESS_HOME_ZIP);
-    return true;
-  }
+  if (autocomplete_type == ASCIIToUTF16("address-line2"))
+    return ADDRESS_HOME_LINE2;
 
-  if (autocomplete_type == ASCIIToUTF16("country")) {
-    field->set_heuristic_type(ADDRESS_HOME_COUNTRY);
-    return true;
-  }
+  if (autocomplete_type == ASCIIToUTF16("locality"))
+    return ADDRESS_HOME_CITY;
 
-  if (autocomplete_type == ASCIIToUTF16("organization")) {
-    field->set_heuristic_type(COMPANY_NAME);
-    return true;
-  }
+  if (autocomplete_type == ASCIIToUTF16("region"))
+    return ADDRESS_HOME_STATE;
 
-  if (autocomplete_type == ASCIIToUTF16("email")) {
-    field->set_heuristic_type(EMAIL_ADDRESS);
-    return true;
-  }
+  if (autocomplete_type == ASCIIToUTF16("country"))
+    return ADDRESS_HOME_COUNTRY;
 
-  if (autocomplete_type == ASCIIToUTF16("phone-full")) {
-    field->set_heuristic_type(PHONE_HOME_WHOLE_NUMBER);
-    return true;
-  }
+  if (autocomplete_type == ASCIIToUTF16("postal-code"))
+    return ADDRESS_HOME_ZIP;
 
-  if (autocomplete_type == ASCIIToUTF16("phone-country-code")) {
-    field->set_heuristic_type(PHONE_HOME_COUNTRY_CODE);
-    return true;
-  }
+  if (autocomplete_type == ASCIIToUTF16("cc-name"))
+    return CREDIT_CARD_NAME;
 
-  if (autocomplete_type == ASCIIToUTF16("phone-national")) {
-    field->set_heuristic_type(PHONE_HOME_CITY_AND_NUMBER);
-    return true;
-  }
+  if (autocomplete_type == ASCIIToUTF16("cc-number"))
+    return CREDIT_CARD_NUMBER;
 
-  if (autocomplete_type == ASCIIToUTF16("phone-area-code")) {
-    field->set_heuristic_type(PHONE_HOME_CITY_CODE);
-    return true;
+  if (autocomplete_type == ASCIIToUTF16("cc-exp")) {
+    if (field.max_length == 5)
+      return CREDIT_CARD_EXP_DATE_2_DIGIT_YEAR;
+    else
+      return CREDIT_CARD_EXP_DATE_4_DIGIT_YEAR;
   }
 
-  if (autocomplete_type == ASCIIToUTF16("phone-local")) {
-    field->set_heuristic_type(PHONE_HOME_NUMBER);
-    return true;
-  }
+  if (autocomplete_type == ASCIIToUTF16("cc-exp-month"))
+    return CREDIT_CARD_EXP_MONTH;
 
-  if (autocomplete_type == ASCIIToUTF16("phone-local-prefix")) {
-    field->set_heuristic_type(PHONE_HOME_NUMBER);
-    field->set_phone_part(AutofillField::PHONE_PREFIX);
-    return true;
+  if (autocomplete_type == ASCIIToUTF16("cc-exp-year")) {
+    if (field.max_length == 2)
+      return CREDIT_CARD_EXP_2_DIGIT_YEAR;
+    else
+      return CREDIT_CARD_EXP_4_DIGIT_YEAR;
   }
 
-  if (autocomplete_type == ASCIIToUTF16("phone-local-suffix")) {
-    field->set_heuristic_type(PHONE_HOME_NUMBER);
-    field->set_phone_part(AutofillField::PHONE_SUFFIX);
-    return true;
-  }
+  if (autocomplete_type == ASCIIToUTF16("tel"))
+    return PHONE_HOME_WHOLE_NUMBER;
 
-  if (autocomplete_type == ASCIIToUTF16("cc-full-name")) {
-    field->set_heuristic_type(CREDIT_CARD_NAME);
-    return true;
-  }
+  if (autocomplete_type == ASCIIToUTF16("tel-country-code"))
+    return PHONE_HOME_COUNTRY_CODE;
 
-  if (autocomplete_type == ASCIIToUTF16("cc-number")) {
-    field->set_heuristic_type(CREDIT_CARD_NUMBER);
-    return true;
-  }
+  if (autocomplete_type == ASCIIToUTF16("tel-national"))
+    return PHONE_HOME_CITY_AND_NUMBER;
 
-  if (autocomplete_type == ASCIIToUTF16("cc-exp-month")) {
-    field->set_heuristic_type(CREDIT_CARD_EXP_MONTH);
-    return true;
-  }
+  if (autocomplete_type == ASCIIToUTF16("tel-area-code"))
+    return PHONE_HOME_CITY_CODE;
 
-  if (autocomplete_type == ASCIIToUTF16("cc-exp-year")) {
-    if (field->max_length == 2)
-      field->set_heuristic_type(CREDIT_CARD_EXP_2_DIGIT_YEAR);
-    else
-      field->set_heuristic_type(CREDIT_CARD_EXP_4_DIGIT_YEAR);
-    return true;
-  }
+  if (autocomplete_type == ASCIIToUTF16("tel-local"))
+    return PHONE_HOME_NUMBER;
 
-  if (autocomplete_type == ASCIIToUTF16("cc-exp")) {
-    if (field->max_length == 5)
-      field->set_heuristic_type(CREDIT_CARD_EXP_DATE_2_DIGIT_YEAR);
-    else
-      field->set_heuristic_type(CREDIT_CARD_EXP_DATE_4_DIGIT_YEAR);
-    return true;
-  }
+  if (autocomplete_type == ASCIIToUTF16("tel-local-prefix"))
+    return PHONE_HOME_NUMBER;
 
-  return false;
+  if (autocomplete_type == ASCIIToUTF16("tel-local-suffix"))
+    return PHONE_HOME_NUMBER;
+
+  if (autocomplete_type == ASCIIToUTF16("email"))
+    return EMAIL_ADDRESS;
+
+  return UNKNOWN_TYPE;
 }
 
 }  // namespace
@@ -273,12 +264,13 @@ FormStructure::FormStructure(const FormData& form)
 FormStructure::~FormStructure() {}
 
 void FormStructure::DetermineHeuristicTypes() {
-  // First, try to detect field types based on the fields' |autocompletetype|
-  // attributes.  If there is at least one form field with this attribute, don't
-  // try to apply other heuristics to match fields in this form.
+  // First, try to detect field types based on each field's |autocomplete|
+  // attribute value.  If there is at least one form field that specifies an
+  // autocomplete type hint, don't try to apply other heuristics to match fields
+  // in this form.
   bool has_author_specified_sections;
-  ParseAutocompletetypeAttributes(&has_author_specified_types_,
-                                  &has_author_specified_sections);
+  ParseFieldTypesFromAutocompleteAttributes(&has_author_specified_types_,
+                                            &has_author_specified_sections);
 
   if (!has_author_specified_types_) {
     FieldTypeMap field_type_map;
@@ -878,32 +870,99 @@ bool FormStructure::EncodeFormRequest(
   return true;
 }
 
-void FormStructure::ParseAutocompletetypeAttributes(bool* found_attribute,
-                                                    bool* found_sections) {
-  *found_attribute = false;
+void FormStructure::ParseFieldTypesFromAutocompleteAttributes(
+    bool* found_types,
+    bool* found_sections) {
+  *found_types = false;
   *found_sections = false;
   for (std::vector<AutofillField*>::iterator field = fields_.begin();
        field != fields_.end(); ++field) {

[Evan Stade, 2012/10/19 20:36:47]

nit: can you call field iter, and then do

AutofillField* field = *iter;

I think this will improve readability for the rest of the loop.

[Ilya Sherman, 2012/10/20 05:16:08]

Done.

-    if ((*field)->autocomplete_type.empty())
+    // Canonicalize the attribute value by trimming whitespace and converting to
+    // lowercase ASCII.
+    string16 autocomplete_attribute = (*field)->autocomplete_attribute;

[Evan Stade, 2012/10/19 20:36:47]

see my note elsewhere that the field's attribute should be std::string. However,
if it has to be string16 for some reason, I would still say it's easier to
convert to std::string here rather than peppering so many ASCIIToUTF16
everywhere.

[Ilya Sherman, 2012/10/20 05:16:08]

Done.

+    TrimWhitespace(autocomplete_attribute, TRIM_ALL, &autocomplete_attribute);
+    autocomplete_attribute = StringToLowerASCII(autocomplete_attribute);
+
+    // The autocomplete attribute is overloaded: it can specify either a field
+    // type hint or whether autocomplete should be enabled at all.  Ignore the
+    // latter type of attribute value.
+    if (autocomplete_attribute.empty() ||
+        autocomplete_attribute == ASCIIToUTF16("on") ||
+        autocomplete_attribute == ASCIIToUTF16("off")) {
+      continue;
+    }
+
+    // Any other value, even it is invalid, is considered to be a type hint.
+    // This allows a website's author to specify an attribute like
+    // autocomplete="other" on a field to disable all Autofill heuristics for

[Evan Stade, 2012/10/19 20:36:47]

don't follow this logic. Are you saying that autocomplete="other" on a single
field will disable autofill for the entire form? That's not intuitive to me, and
the spec already says how to do that: autocomplete="off" on the form owner.

[Ilya Sherman, 2012/10/20 05:16:08]

The two are different.  autocomplete="off" means don't invoke any form of
autocomplete, and hence don't save any of the field values for single-field
autocomplete (i.e. the sort of autocomplete that most browsers support). 
autocomplete="other" means "Hey, I'm telling you about the field types on this
form.  Don't apply any *heuristics* to try to figure out the field types.  This
is neither an address nor a credit card form."  Two possible forms:

<form>
  Favorite color: <input autocomplete="other">
  Favorite animal: <input>
  Favorite food: <input>
</form>

<form>
  Favorite color: <input autocomplete="other">
  Name: <input autocomplete="name">
  Email: <input autocomplete="email">
  Home Country: <input autocomplete="country">
</form>

For the first form, we won't try to autofill it at all, since at least one field
has an autocomplete type specified; and we don't recognize that type.  For the
second form, we'll autofill all but the first field.

Does that make sense?

+    // the form.
+    *found_types = true;
+
+    // Tokenize the attribute value.  Per the spec, the tokens are parsed in
+    // reverse order.
+    std::vector<string16> tokens;

[Evan Stade, 2012/10/19 20:36:47]

likewise, should be std::string

[Ilya Sherman, 2012/10/20 05:16:08]

Done.

+    Tokenize(autocomplete_attribute, ASCIIToUTF16(" "), &tokens);

[Evan Stade, 2012/10/19 20:59:44]

should this include other space characters, or were those converted to spaces at
some other point?

http://www.whatwg.org/specs/web-apps/current-work/multipage/common-microsynta...

[Ilya Sherman, 2012/10/20 05:16:08]

Done + added test coverage (to the advanced.html test).

+
+    // The final token must be the field type.
+    // If it is not one of the known types, abort.
+    DCHECK(!tokens.empty());
+    string16 field_type_token = tokens.back();
+    tokens.pop_back();
+    AutofillFieldType field_type =
+        FieldTypeFromAutocompleteType(field_type_token, **field);
+    if (field_type == UNKNOWN_TYPE)
       continue;
 
-    *found_attribute = true;
-    std::vector<string16> types;
-    Tokenize((*field)->autocomplete_type, ASCIIToUTF16(" "), &types);
+    // The preceding token, if any, may be a type hint.
+    if (!tokens.empty() && IsContactTypeHint(tokens.back())) {
+      // If it is, it must match the field type; otherwise, abort.
+      // Note that an invalid token invalidates the entire attribute value, even
+      // if the other tokens are valid.
+      if (!ContactTypeHintMatchesFieldType(tokens.back(), field_type))
+        continue;
 
-    // Look for a named section.
-    const string16 kSectionPrefix = ASCIIToUTF16("section-");
-    if (!types.empty() && StartsWith(types.front(), kSectionPrefix, true)) {
+      // Chrome Autofill ignores these type hints.
+      tokens.pop_back();
+    }
+
+    // The preceding token, if any, may be a fixed string that is either
+    // "shipping" or "billing".  Chrome Autofill treats these as implicit
+    // section name suffixes.
+    if (!tokens.empty() &&
+        (tokens.back() == ASCIIToUTF16("shipping") ||
+         tokens.back() == ASCIIToUTF16("billing"))) {
       *found_sections = true;
-      (*field)->set_section(types.front().substr(kSectionPrefix.size()));
+      (*field)->set_section(ASCIIToUTF16("-") + tokens.back());
+      tokens.pop_back();
+    } else {
+      // To prevent potential section name collisions, add a default suffix for
+      // other fields.  Without this, 'autocomplete' attribute values
+      // "section--shipping street-address" and "shipping street-address" would

[Evan Stade, 2012/10/19 20:36:47]

can you help me understand this section? It seems like without it, the former
would have section "-shipping" and no "shipping" hint, and the latter would have
no section but would have a "shipping" hint. Also, I don't see in the spec where
they say "section-default" is off limits to site developers.

[Evan Stade, 2012/10/19 20:57:27]

Dan explained this to me. So is it intentional that "section-blue shipping
region" creates a different section than "section-blue name"? What are we going
to do with section anyway?

[Ilya Sherman, 2012/10/20 05:16:08]

Yes, it's intentional.  We fill differently named sections separately, i.e. the
Autofill UI only affects one section at a time.  You can play around with this
at
https://www.corp.google.com/~isherman/no_crawl/autofill/shipping_and_billing_...
to get a feel for what I mean.

+      // be parsed identically.
+      (*field)->set_section(ASCIIToUTF16("-default"));
     }
 
-    // Look for specified types.
-    for (std::vector<string16>::const_iterator type = types.begin();
-         type != types.end(); ++type) {
-      if (UpdateFromAutocompleteType(*type, *field))
-        break;
+    // The preceding token, if any, may be a named section.
+    const string16 kSectionPrefix = ASCIIToUTF16("section-");
+    if (!tokens.empty() && StartsWith(tokens.back(), kSectionPrefix, true)) {
+      *found_sections = true;
+      // Prepend this section name to the suffix set in the preceding block.
+      (*field)->set_section(
+          tokens.back().substr(kSectionPrefix.size()) + (*field)->section());

[Evan Stade, 2012/10/19 20:57:27]

do you need to check for "section- " (i.e. with no actual string following the
hyphen)

[Ilya Sherman, 2012/10/20 05:16:08]

It's covered in the tests: we treat this as equivalent to an empty section name.

+      tokens.pop_back();
     }
+
+    // No other tokens are allowed.  If there are any remaining, abort.

[Evan Stade, 2012/10/19 20:36:47]

if you are aborting, and you just set found_sections to true, should it return
to what it was before?

[Ilya Sherman, 2012/10/20 05:16:08]

Good call.  Fixed, and added test coverage.  The fix isn't perfect, as the
fields don't get "-default" appended to their sections... but I think it's an ok
tradeoff of code complexity vs. gracefully degrading when parsing invalid
attributes.

+    if (!tokens.empty())
+      continue;
+
+    // No errors encountered while parsing!
+    // Update the |field|'s type based on what was parsed from the attribute.
+    (*field)->set_heuristic_type(field_type);
+    if (field_type_token == ASCIIToUTF16("tel-local-prefix"))
+      (*field)->set_phone_part(AutofillField::PHONE_PREFIX);
+    else if (field_type_token == ASCIIToUTF16("tel-local-suffix"))
+      (*field)->set_phone_part(AutofillField::PHONE_SUFFIX);
   }
 }