Only strip separators from credit card numbers when implicitly learning or filling.

Preserve whatever the user typed.

BUG=78844
TEST=unit_tests --gtest_filter=*CreditCard*

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

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

6 files changed, 38 insertions, 16 deletions

diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc
index 5cedc73..be3358a 100644
--- a/chrome/browser/autofill/autofill_manager.cc
+++ b/chrome/browser/autofill/autofill_manager.cc
@@ -902,7 +902,10 @@ void AutofillManager::FillCreditCardFormField(const CreditCard* credit_card,
       field->value = year + ASCIIToUTF16("-") + month;
     }
   } else {
-    field->value = credit_card->GetInfo(type);
+    string16 value = credit_card->GetInfo(type);
+    if (type == CREDIT_CARD_NUMBER)
+      value = CreditCard::StripSeparators(value);
+    field->value = value;
   }
 }
 
diff --git a/chrome/browser/autofill/autofill_manager_unittest.cc b/chrome/browser/autofill/autofill_manager_unittest.cc
index 646b0fa..0bcc006 100644
--- a/chrome/browser/autofill/autofill_manager_unittest.cc
+++ b/chrome/browser/autofill/autofill_manager_unittest.cc
@@ -123,7 +123,7 @@ class TestPersonalDataManager : public PersonalDataManager {
   void CreateTestCreditCards(ScopedVector<CreditCard>* credit_cards) {
     CreditCard* credit_card = new CreditCard;
     autofill_test::SetCreditCardInfo(credit_card, "Elvis Presley",
-                                     "4234567890123456",  // Visa
+                                     "4234 5678 9012 3456",  // Visa
                                      "04", "2012");
     credit_card->set_guid("00000000-0000-0000-0000-000000000004");
     credit_cards->push_back(credit_card);
diff --git a/chrome/browser/autofill/credit_card.cc b/chrome/browser/autofill/credit_card.cc
index 6ddbc95..85e1f26 100644
--- a/chrome/browser/autofill/credit_card.cc
+++ b/chrome/browser/autofill/credit_card.cc
@@ -135,14 +135,6 @@ bool ConvertDate(const string16& date, int* num) {
   return true;
 }
 
-// Return a version of |number| that has any separator characters removed.
-const string16 StripSeparators(const string16& number) {
-  const char16 kSeparators[] = {'-', ' ', '\0'};
-  string16 stripped;
-  RemoveChars(number, kSeparators, &stripped);
-  return stripped;
-}
-
 }  // namespace
 
 CreditCard::CreditCard(const std::string& guid)
@@ -333,7 +325,8 @@ string16 CreditCard::ObfuscatedNumber() const {
   if (number_.size() < 4)
     return number_;
 
-  string16 result(number_.size() - 4, kCreditCardObfuscationSymbol);
+  string16 number = StripSeparators(number_);
+  string16 result(number.size() - 4, kCreditCardObfuscationSymbol);
   result.append(LastFourDigits());
 
   return result;
@@ -342,10 +335,11 @@ string16 CreditCard::ObfuscatedNumber() const {
 string16 CreditCard::LastFourDigits() const {
   static const size_t kNumLastDigits = 4;
 
-  if (number().size() < kNumLastDigits)
+  string16 number = StripSeparators(number_);
+  if (number.size() < kNumLastDigits)
     return string16();
 
-  return number().substr(number().size() - kNumLastDigits, kNumLastDigits);
+  return number.substr(number.size() - kNumLastDigits, kNumLastDigits);
 }
 
 void CreditCard::operator=(const CreditCard& credit_card) {
@@ -390,6 +384,14 @@ bool CreditCard::operator!=(const CreditCard& credit_card) const {
 }
 
 // static
+const string16 CreditCard::StripSeparators(const string16& number) {
+  const char16 kSeparators[] = {'-', ' ', '\0'};
+  string16 stripped;
+  RemoveChars(number, kSeparators, &stripped);
+  return stripped;
+}
+
+// static
 bool CreditCard::IsValidCreditCardNumber(const string16& text) {
   string16 number = StripSeparators(text);
 
@@ -475,8 +477,8 @@ void CreditCard::SetExpirationYearFromString(const string16& text) {
 }
 
 void CreditCard::SetNumber(const string16& number) {
-  number_ = StripSeparators(number);
-  type_ = ASCIIToUTF16(GetCreditCardType(number_));
+  number_ = number;
+  type_ = ASCIIToUTF16(GetCreditCardType(StripSeparators(number_)));
 }
 
 void CreditCard::SetExpirationMonth(int expiration_month) {
@@ -496,7 +498,7 @@ void CreditCard::SetExpirationYear(int expiration_year) {
 }
 
 bool CreditCard::IsNumber(const string16& text) const {
-  return StripSeparators(text) == number_;
+  return StripSeparators(text) == StripSeparators(number_);
 }
 
 bool CreditCard::IsNameOnCard(const string16& text) const {
diff --git a/chrome/browser/autofill/credit_card.h b/chrome/browser/autofill/credit_card.h
index 8f6c3be..ed8a828 100644
--- a/chrome/browser/autofill/credit_card.h
+++ b/chrome/browser/autofill/credit_card.h
@@ -62,6 +62,9 @@ class CreditCard : public FormGroup {
   bool operator==(const CreditCard& credit_card) const;
   bool operator!=(const CreditCard& credit_card) const;
 
+  // Return a version of |number| that has any separator characters removed.
+  static const string16 StripSeparators(const string16& number);
+
   // Returns true if |text| looks like a valid credit card number.
   // Uses the Luhn formula to validate the number.
   static bool IsValidCreditCardNumber(const string16& text);
diff --git a/chrome/browser/autofill/credit_card_unittest.cc b/chrome/browser/autofill/credit_card_unittest.cc
index 69c0351..488c0c8 100644
--- a/chrome/browser/autofill/credit_card_unittest.cc
+++ b/chrome/browser/autofill/credit_card_unittest.cc
@@ -124,3 +124,13 @@ TEST(CreditCardTest, InvalidMastercardNumber) {
                                    "5200000000000004", "01", "2010");
   EXPECT_EQ(ASCIIToUTF16("genericCC"), card.type());
 }
+
+// Verify that we preserve exactly what the user typed for credit card numbers.
+TEST(CreditCardTest, SetInfoCreditCardNumber) {
+  CreditCard card;
+
+  autofill_test::SetCreditCardInfo(&card, "Bob Dylan",
+                                   "4321-5432-6543-xxxx", "07", "2013");
+  EXPECT_EQ(ASCIIToUTF16("4321-5432-6543-xxxx"),
+            card.GetInfo(CREDIT_CARD_NUMBER));
+}
diff --git a/chrome/browser/autofill/personal_data_manager.cc b/chrome/browser/autofill/personal_data_manager.cc
index 367bace..46fee8a3 100644
--- a/chrome/browser/autofill/personal_data_manager.cc
+++ b/chrome/browser/autofill/personal_data_manager.cc
@@ -217,6 +217,10 @@ bool PersonalDataManager::ImportFormData(
             DCHECK_EQ(CREDIT_CARD_EXP_MONTH, field_type);
             local_imported_credit_card->SetInfoForMonthInputType(value);
           } else {
+            if (field_type == CREDIT_CARD_NUMBER) {
+              // Clean up any imported credit card numbers.
+              value = CreditCard::StripSeparators(value);
+            }
             local_imported_credit_card->SetInfo(field_type, value);
           }
           ++importable_credit_card_fields;