[Autofill] Autofill fails to show suggestions for credit card split across fields.

components/autofill/core/browser/autofill_manager.cc

 // Copyright 2013 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 "components/autofill/core/browser/autofill_manager.h"
 
 #include <stddef.h>
 
 #include <limits>
 #include <map>
@@ skipping 138 matching lines @@
       it->GetMatchingTypes(value, app_locale, &matching_types);
     }
 
     if (matching_types.empty())
       matching_types.insert(UNKNOWN_TYPE);
 
     field->set_possible_types(matching_types);
   }
 }
 
+// Helper function to get the suggetions for |user_input| matching to
+// |card_number_filed| split.
+void GetCredtCardSuggetionsForNumberSplit(
+    const base::string16& user_input,
+    const AutofillField& card_number_filed,
+    PersonalDataManager* personal_data,
+    std::vector<base::string16>* values,
+    std::vector<base::string16>* labels,
+    std::vector<base::string16>* icons,
+    std::vector<GUIDPair>* guid_pairs) {
+  const std::vector<CreditCard*>& credit_cards =
+      personal_data->GetCreditCards();
+  for (std::vector<CreditCard*>::const_iterator iter = credit_cards.begin();
+       iter != credit_cards.end();
+       ++iter) {
+    CreditCard* credit_card = *iter;
+    base::string16 stored_card_number =
+        CreditCard::StripSeparators(credit_card->number());
+
+    // Exclude first split, as it would have already included in the
+    // suggestions.
+    if (!stored_card_number.empty() &&
+        !StartsWith(stored_card_number, user_input, false)) {
+
+      // Make sure the suggetions get include for rest of the
+      // |card_number_filed|s.
+      stored_card_number = AutofillField::GetCreditCardNumberValue(
+          card_number_filed, stored_card_number);
+      if (StartsWith(stored_card_number, user_input, false)) {
+        personal_data->AppendCreditCardSuggetions(
+            credit_card,
+            AutofillType(CREDIT_CARD_NUMBER),
+            user_input,
+            values,
+            labels,
+            icons,
+            guid_pairs);
+      }
+    }
+  }
+}

[Ilya Sherman, 2014/10/07 21:49:10]

Can all of this logic be moved into the PersonalDataManager class, as it is for
phone numbers?

[Pritam Nikam, 2014/10/08 05:17:17]

In my opinion, we *cannot* move this logic to PersonalDataManager class straight
away.
Retrieving card number substring is poised on
AutofillField::credit_card_number_offset(). 

Moreover, changing API signature of
PersonalDataManager::GetCreditCardSuggestions() will apparently impact usages
from other code excerpts [1] where passing AutofillField reference as argument
won't be easy.

And as I understand, phone number prefix/suffix suggestions handling logic is
part of AutofillManager[2] as of now, and reason is the same mentioned [1].

[1].
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

[2].
https://code.google.com/p/chromium/codesearch#chromium/src/components/autofil...

[Ilya Sherman, 2014/10/08 21:04:38]

Phone numbers are also handled within the PersonalDataManager code [1].  The
AutofillManager code that you linked to just adjusts what data will be filled;
it's much simpler and clearer than the credit card code that you've added in
this CL.  I'd prefer to align the two implementations.

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/components/autofil...

[Pritam Nikam, 2014/10/09 06:11:11]

Acknowledged.

I'll align my changes along these logic i.e. sub-string match and upload patch
for review.

By I've query related to same, it's a sub-string matching instead of pre-fix
matching. But don't we need field match as well?

For instance, say we have 3 credit cards saved.
/*  Name On Card  |  Card Number          | mm  | yyyy  */
A. {"Miku Hatsune", "4111-1111-1111-1111", "07", "2015"}
B. {"Elvis Presley", "4231-2341-1111-1017", "04", "2012"}
C. {"Buddy Holly", "5632-1111-4111-2341", "10", "2014"}

If suppose, a HTML form having card number split across 4 input fields. And
autofilled with card [A] i.e. cc-number "4111-1111-1111-1111". In my opinion,
double click on 4'th cc-number input field may bring all 3 card suggestions, and
user may be expecting to see only A'th card instead, OR am I missing something
here?

[Ilya Sherman, 2014/10/09 21:35:12]

That's true.  It's such an edge case that I'm honestly not too worried about it
-- most users will have at most one credit card saved, and most websites only
have a single credit card field anyway.  If you really want to handle this case
correctly, I'd recommend (a) also improving the handling of phone numbers to
match, and (b) extending the PersonalDataManager::GetCreditCardSuggestions()
method with the necessary logic, possibly by passing an additional parameter for
the current field value.

[Pritam Nikam, 2014/10/13 04:33:52]

Currently, *current field value* (i.e. const base::string16& field_contents) is
getting pass to this function and that seems not enough to get the exact
suggestions for matching field input. I'll further investigate on these lines.

Moreover, I've added a patch addressing your earlier suggestions i.e. sub-string
instead of prefix match.

+
 }  // namespace
 
 AutofillManager::AutofillManager(
     AutofillDriver* driver,
     AutofillClient* client,
     const std::string& app_locale,
     AutofillDownloadManagerState enable_download_manager)
     : driver_(driver),
       client_(client),
       app_locale_(app_locale),
@@ skipping 297 matching lines @@
   AutofillField* autofill_field = NULL;
   if (RefreshDataModels() &&
       driver_->RendererIsAvailable() &&
       GetCachedFormAndField(form, field, &form_structure, &autofill_field) &&
       // Don't send suggestions for forms that aren't auto-fillable.
       form_structure->IsAutofillable()) {
     AutofillType type = autofill_field->Type();
     bool is_filling_credit_card = (type.group() == CREDIT_CARD);
     if (is_filling_credit_card) {
       GetCreditCardSuggestions(
-          field, type, &values, &labels, &icons, &unique_ids);
+          field, *autofill_field, &values, &labels, &icons, &unique_ids);
     } else {
       GetProfileSuggestions(*form_structure,
                             field,
                             *autofill_field,
                             &values,
                             &labels,
                             &icons,
                             &unique_ids);
     }
 
@@ skipping 623 matching lines @@
   }
 
   for (size_t i = 0; i < guid_pairs.size(); ++i) {
     unique_ids->push_back(PackGUIDs(GUIDPair(std::string(), 0),
                                     guid_pairs[i]));
   }
 }
 
 void AutofillManager::GetCreditCardSuggestions(
     const FormFieldData& field,
-    const AutofillType& type,
+    const AutofillField& autofill_field,
     std::vector<base::string16>* values,
     std::vector<base::string16>* labels,
     std::vector<base::string16>* icons,
     std::vector<int>* unique_ids) const {
   std::vector<GUIDPair> guid_pairs;
+  AutofillType type = autofill_field.Type();
   personal_data_->GetCreditCardSuggestions(
       type, field.value, values, labels, icons, &guid_pairs);
 
+  // Populate suggetions for number split case (http://crbug.com/420323).
+  if (!field.value.empty() &&
+      autofill_field.Type().GetStorableType() == CREDIT_CARD_NUMBER)
+    GetCredtCardSuggetionsForNumberSplit(field.value,
+                                         autofill_field,
+                                         personal_data_,
+                                         values,
+                                         labels,
+                                         icons,
+                                         &guid_pairs);

[Ilya Sherman, 2014/10/07 21:49:10]

nit: Please add curly braces, since the body spans multiple lines of text.

[Pritam Nikam, 2014/10/08 05:17:17]

Done.

+
   for (size_t i = 0; i < guid_pairs.size(); ++i) {
     unique_ids->push_back(PackGUIDs(guid_pairs[i], GUIDPair(std::string(), 0)));
   }
 }
 
 void AutofillManager::ParseForms(const std::vector<FormData>& forms) {
   std::vector<FormStructure*> non_queryable_forms;
   for (std::vector<FormData>::const_iterator iter = forms.begin();
        iter != forms.end(); ++iter) {
     scoped_ptr<FormStructure> form_structure(new FormStructure(*iter));
@@ skipping 103 matching lines @@
     return false;
 
   // Disregard forms that we wouldn't ever autofill in the first place.
   if (!form.ShouldBeParsed())
     return false;
 
   return true;
 }
 
 }  // namespace autofill