 Chromium Code Reviews
 Chromium Code Reviews Issue 11198048:
  [Autofill] Update the autocomplete types implementation to match the current HTML spec.  (Closed) 
  Base URL: svn://svn.chromium.org/chrome/trunk/src
    
  
    Issue 11198048:
  [Autofill] Update the autocomplete types implementation to match the current HTML spec.  (Closed) 
  Base URL: svn://svn.chromium.org/chrome/trunk/src| 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..291d4b331247ece01a4e45959156808480ec670d 100644 | 
| --- a/chrome/browser/autofill/form_structure.cc | 
| +++ b/chrome/browser/autofill/form_structure.cc | 
| @@ -85,147 +85,141 @@ 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://www.whatwg.org/specs/web-apps/current-work/multipage/association-of-controls-and-forms.html#autofilling-form-controls:-the-autocomplete-attribute | 
| 
Dan Beam
2012/10/19 00:12:44
nit: it might be useful to URL shorten some of the
 
Ilya Sherman
2012/10/19 04:19:32
Done.
 | 
| +// Note that "fax" is intentionally ignored, as Chrome does not support filling | 
| +// fax information. | 
| +bool IsContactTypeHint(const string16& token) { | 
| + return | 
| 
Dan Beam
2012/10/19 00:12:44
nit: isn't the chromium style more
  return token
 
Ilya Sherman
2012/10/19 04:19:32
Done.
 
Dan Beam
2012/10/23 00:28:59
it doesn't seem like anything other than stripping
 
Ilya Sherman
2012/10/23 01:06:12
Whoops.  Actually done now.
 | 
| + token == ASCIIToUTF16("home") || | 
| + 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://www.whatwg.org/specs/web-apps/current-work/multipage/association-of-controls-and-forms.html#autofilling-form-controls:-the-autocomplete-attribute | 
| +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 && | 
| + field_type <= PHONE_HOME_WHOLE_NUMBER); | 
| } | 
| - if (autocomplete_type == ASCIIToUTF16("middle-initial")) { | 
| - field->set_heuristic_type(NAME_MIDDLE_INITIAL); | 
| - return true; | 
| + // The remaining type hints are only appropriate for phone number field types. | 
| + // Note that "fax" is intentionally ignored, as Chrome does not support | 
| + // filling fax information. | 
| + if (token == ASCIIToUTF16("mobile") || token == ASCIIToUTF16("pager")) { | 
| + 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://www.whatwg.org/specs/web-apps/current-work/multipage/association-of-controls-and-forms.html#autofilling-form-controls:-the-autocomplete-attribute | 
| +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")) | 
| 
Dan Beam
2012/10/19 00:12:44
organization == company always?
 
Ilya Sherman
2012/10/19 04:19:32
The spec defines this token as meaning "Company na
 
Dan Beam
2012/10/19 04:25:41
I know, I noticed that.  My issue is really with t
 | 
| + 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")) | 
| 
Dan Beam
2012/10/19 00:12:44
region == state always?
 
Ilya Sherman
2012/10/19 04:19:32
"state" is the name that Autofill internally uses
 
Dan Beam
2012/10/19 04:25:41
So we could have things like Bavaria as the state.
 
Ilya Sherman
2012/10/19 04:54:11
Yes, we could have things like Bavaria as the "sta
 | 
| + 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; | 
| 
Dan Beam
2012/10/19 00:12:44
postal-code == zip always?
 
Ilya Sherman
2012/10/19 04:19:32
Ditto: This is just what the Autofill code calls t
 | 
| - 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 +267,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 with that specifies | 
| 
Dan Beam
2012/10/19 00:12:44
If there is at least one form field that specifies
 
Ilya Sherman
2012/10/19 04:19:32
Done.
 | 
| + // 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 +873,96 @@ bool FormStructure::EncodeFormRequest( | 
| return true; | 
| } | 
| -void FormStructure::ParseAutocompletetypeAttributes(bool* found_attribute, | 
| - bool* found_sections) { | 
| - *found_attribute = false; | 
| +// TODO(isherman): This method could use a shorter name... | 
| 
Ilya Sherman
2012/10/18 00:11:19
Any suggestions?
 
Dan Beam
2012/10/19 00:12:44
Not really, maybe just FieldTypeFromAutocompleteAt
 
Ilya Sherman
2012/10/19 04:19:32
Ok.  I like having parse in the name, so I guess l
 | 
| +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) { | 
| - if ((*field)->autocomplete_type.empty()) | 
| + // Canonicalize the attribute value by trimming whitespace and converting to | 
| + // lowercase ASCII. | 
| + string16 autocomplete_attribute = (*field)->autocomplete_attribute; | 
| + 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 | 
| + // the form. | 
| + *found_types = true; | 
| + | 
| + // Tokenize the attribute value. Per the spec, the tokens are parsed in | 
| + // reverse order. | 
| + std::vector<string16> tokens; | 
| + Tokenize(autocomplete_attribute, ASCIIToUTF16(" "), &tokens); | 
| + | 
| + // 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. | 
| 
Dan Beam
2012/10/19 00:12:44
you may want to add/hint at that there could be va
 
Ilya Sherman
2012/10/19 04:19:32
Done.
 | 
| + 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 ignore these type hints. | 
| 
Dan Beam
2012/10/19 00:12:44
ignores
 
Ilya Sherman
2012/10/19 04:19:32
Done.
 | 
| + 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 | 
| 
Dan Beam
2012/10/19 00:12:44
you might want to add a comment here explaining wh
 
Ilya Sherman
2012/10/19 04:19:32
Done.
 | 
| + // other fields. | 
| + (*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()); | 
| + tokens.pop_back(); | 
| } | 
| + | 
| + // No other tokens are allowed. If there are any remaining, abort. | 
| + 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); | 
| } | 
| } |