Adjust autofill phone number format.

- Use '+' sign for out of country call, except US.
- Keep valid national number input as it is.

R=isherman@chromium.org
BUG=98911

Review URL: https://codereview.chromium.org/355823007

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

11 files changed, 137 insertions, 35 deletions

diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillDialogControllerTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillDialogControllerTest.java
index 0e9342f..f6360ec 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillDialogControllerTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillDialogControllerTest.java
@@ -51,7 +51,7 @@ public class AutofillDialogControllerTest extends ChromeShellTestBase {
 
     private static final String TEST_SHIPPING_NAME = "Mister Receiver";
     private static final String TEST_SHIPPING_PHONE = "+46 8 713 99 99";
-    private static final String TEST_SHIPPING_PHONE_UNFORMATTED = "4687139999";
+    private static final String TEST_SHIPPING_PHONE_UNFORMATTED = "+4687139999";
     private static final String TEST_SHIPPING1 = "19 Farstaplan";
     private static final String TEST_SHIPPING2 = "Third floor";
     private static final String TEST_SHIPPING3 = "please call first";
diff --git a/chrome/browser/autofill/autofill_browsertest.cc b/chrome/browser/autofill/autofill_browsertest.cc
index e565056..0249d06 100644
--- a/chrome/browser/autofill/autofill_browsertest.cc
+++ b/chrome/browser/autofill/autofill_browsertest.cc
@@ -659,6 +659,78 @@ IN_PROC_BROWSER_TEST_F(AutofillTest, AppendCountryCodeForAggregatedPhones) {
                 PHONE_HOME_WHOLE_NUMBER));
 }
 
+// Test that Autofill uses '+' sign for international numbers.
+// This applies to the following cases:
+//   The phone number has a leading '+'.
+//   The phone number does not have a leading '+'.
+//   The phone number has a leading international direct dialing (IDD) code.
+// This does not apply to US numbers. For US numbers, '+' is removed.
+IN_PROC_BROWSER_TEST_F(AutofillTest, UsePlusSignForInternaltionalNumber) {
+  ASSERT_TRUE(test_server()->Start());
+  std::vector<FormMap> profiles;
+
+  FormMap data1;
+  data1["NAME_FIRST"] = "Bonnie";
+  data1["NAME_LAST"] = "Smith";
+  data1["ADDRESS_HOME_LINE1"] = "6723 Roadway Rd";
+  data1["ADDRESS_HOME_CITY"] = "Reading";
+  data1["ADDRESS_HOME_STATE"] = "Berkshire";
+  data1["ADDRESS_HOME_ZIP"] = "RG12 3BR";
+  data1["ADDRESS_HOME_COUNTRY"] = "United Kingdom";
+  data1["PHONE_HOME_WHOLE_NUMBER"] = "+44 7624-123456";
+  profiles.push_back(data1);
+
+  FormMap data2;
+  data2["NAME_FIRST"] = "John";
+  data2["NAME_LAST"] = "Doe";
+  data2["ADDRESS_HOME_LINE1"] = "987 H St";
+  data2["ADDRESS_HOME_CITY"] = "Reading";
+  data2["ADDRESS_HOME_STATE"] = "BerkShire";
+  data2["ADDRESS_HOME_ZIP"] = "RG12 3BR";
+  data2["ADDRESS_HOME_COUNTRY"] = "United Kingdom";
+  data2["PHONE_HOME_WHOLE_NUMBER"] = "44 7624 123456";
+  profiles.push_back(data2);
+
+  FormMap data3;
+  data3["NAME_FIRST"] = "Jane";
+  data3["NAME_LAST"] = "Doe";
+  data3["ADDRESS_HOME_LINE1"] = "1523 Garcia St";
+  data3["ADDRESS_HOME_CITY"] = "Reading";
+  data3["ADDRESS_HOME_STATE"] = "BerkShire";
+  data3["ADDRESS_HOME_ZIP"] = "RG12 3BR";
+  data3["ADDRESS_HOME_COUNTRY"] = "United Kingdom";
+  data3["PHONE_HOME_WHOLE_NUMBER"] = "0044 7624 123456";
+  profiles.push_back(data3);
+
+  FormMap data4;
+  data4["NAME_FIRST"] = "Bob";
+  data4["NAME_LAST"] = "Smith";
+  data4["ADDRESS_HOME_LINE1"] = "123 Cherry Ave";
+  data4["ADDRESS_HOME_CITY"] = "Mountain View";
+  data4["ADDRESS_HOME_STATE"] = "CA";
+  data4["ADDRESS_HOME_ZIP"] = "94043";
+  data4["ADDRESS_HOME_COUNTRY"] = "United States";
+  data4["PHONE_HOME_WHOLE_NUMBER"] = "+1 (408) 871-4567";
+  profiles.push_back(data4);
+
+  for (size_t i = 0; i < profiles.size(); ++i)
+    FillFormAndSubmit("autofill_test_form.html", profiles[i]);
+
+  ASSERT_EQ(4u, personal_data_manager()->GetProfiles().size());
+  ASSERT_EQ(ASCIIToUTF16("+447624123456"),
+            personal_data_manager()->GetProfiles()[0]->GetInfo(
+                AutofillType(PHONE_HOME_WHOLE_NUMBER), ""));
+  ASSERT_EQ(ASCIIToUTF16("+447624123456"),
+            personal_data_manager()->GetProfiles()[1]->GetInfo(
+                AutofillType(PHONE_HOME_WHOLE_NUMBER), ""));
+  ASSERT_EQ(ASCIIToUTF16("+447624123456"),
+            personal_data_manager()->GetProfiles()[2]->GetInfo(
+                AutofillType(PHONE_HOME_WHOLE_NUMBER), ""));
+  ASSERT_EQ(ASCIIToUTF16("14088714567"),
+            personal_data_manager()->GetProfiles()[3]->GetInfo(
+                AutofillType(PHONE_HOME_WHOLE_NUMBER), ""));
+}
+
 // Test CC info not offered to be saved when autocomplete=off for CC field.
 // If the credit card number field has autocomplete turned off, then the credit
 // card infobar should not offer to save the credit card info. The credit card
diff --git a/chrome/browser/ui/autofill/data_model_wrapper_unittest.cc b/chrome/browser/ui/autofill/data_model_wrapper_unittest.cc
index 2df388e..050cba7 100644
--- a/chrome/browser/ui/autofill/data_model_wrapper_unittest.cc
+++ b/chrome/browser/ui/autofill/data_model_wrapper_unittest.cc
@@ -98,19 +98,19 @@ TEST(DataModelWrapperTest, GetDisplayText) {
   base::string16 vertical, horizontal;
   EXPECT_TRUE(
       AutofillProfileWrapper(&profile).GetDisplayText(&horizontal, &vertical));
-  EXPECT_EQ(
-      ASCIIToUTF16("John H. Doe, 666 Erebus St., Apt 8, Elysium, CA 91111\n"
-                   "johndoe@hades.com\n"
-                   "+1 650-211-1111"),
-      horizontal);
-  EXPECT_EQ(
-      ASCIIToUTF16("John H. Doe\n"
-                   "666 Erebus St.\n"
-                   "Apt 8\n"
-                   "Elysium, CA 91111\n"
-                   "johndoe@hades.com\n"
-                   "+1 650-211-1111"),
-      vertical);
+  EXPECT_EQ(ASCIIToUTF16(
+                "John H. Doe, 666 Erebus St., Apt 8, Elysium, CA 91111\n"
+                "johndoe@hades.com\n"
+                "1 650-211-1111"),
+            horizontal);
+  EXPECT_EQ(ASCIIToUTF16(
+                "John H. Doe\n"
+                "666 Erebus St.\n"
+                "Apt 8\n"
+                "Elysium, CA 91111\n"
+                "johndoe@hades.com\n"
+                "1 650-211-1111"),
+            vertical);
 
   // A Japanese address.
   AutofillProfile foreign_profile(
@@ -170,8 +170,9 @@ TEST(DataModelWrapperTest, GetDisplayPhoneNumber) {
   const base::string16 national_unformatted = ASCIIToUTF16("3104567890");
   const base::string16 national_formatted = ASCIIToUTF16("(310) 456-7890");
   const base::string16 international_unformatted = ASCIIToUTF16("13104567890");
-  const base::string16 international_formatted =
-      ASCIIToUTF16("+1 310-456-7890");
+  const base::string16 international_unformatted_with_plus =
+      ASCIIToUTF16("+13104567890");
+  const base::string16 international_formatted = ASCIIToUTF16("1 310-456-7890");
   const base::string16 user_formatted = ASCIIToUTF16("310.456 78 90");
 
   scoped_ptr<wallet::WalletItems::MaskedInstrument> instrument(
@@ -210,6 +211,15 @@ TEST(DataModelWrapperTest, GetDisplayPhoneNumber) {
             address_wrapper.GetInfoForDisplay(
                 AutofillType(PHONE_HOME_WHOLE_NUMBER)));
 
+  const_cast<wallet::Address*>(&instrument->address())
+      ->SetPhoneNumber(international_unformatted_with_plus);
+  EXPECT_EQ(national_formatted,
+            instrument_wrapper.GetInfoForDisplay(
+                AutofillType(PHONE_HOME_WHOLE_NUMBER)));
+  EXPECT_EQ(
+      national_formatted,
+      address_wrapper.GetInfoForDisplay(AutofillType(PHONE_HOME_WHOLE_NUMBER)));
+
   // Autofill numbers that are unformatted get formatted either nationally or
   // internationally depending on the presence of a country code. Formatted
   // numbers stay formatted.
@@ -217,6 +227,11 @@ TEST(DataModelWrapperTest, GetDisplayPhoneNumber) {
   EXPECT_EQ(international_formatted,
             profile_wrapper.GetInfoForDisplay(
                 AutofillType(PHONE_HOME_WHOLE_NUMBER)));
+  profile.SetRawInfo(PHONE_HOME_WHOLE_NUMBER,
+                     international_unformatted_with_plus);
+  EXPECT_EQ(
+      international_formatted,
+      profile_wrapper.GetInfoForDisplay(AutofillType(PHONE_HOME_WHOLE_NUMBER)));
   profile.SetRawInfo(PHONE_HOME_WHOLE_NUMBER, national_unformatted);
   EXPECT_EQ(national_formatted,
             profile_wrapper.GetInfoForDisplay(
@@ -229,7 +244,6 @@ TEST(DataModelWrapperTest, GetDisplayPhoneNumber) {
   EXPECT_EQ(user_formatted,
             profile_wrapper.GetInfoForDisplay(
                 AutofillType(PHONE_HOME_WHOLE_NUMBER)));
-
 }
 
 }  // namespace autofill
diff --git a/components/autofill/core/browser/autofill_ie_toolbar_import_win_unittest.cc b/components/autofill/core/browser/autofill_ie_toolbar_import_win_unittest.cc
index ed064ce..f6b4faf 100644
--- a/components/autofill/core/browser/autofill_ie_toolbar_import_win_unittest.cc
+++ b/components/autofill/core/browser/autofill_ie_toolbar_import_win_unittest.cc
@@ -175,8 +175,7 @@ TEST_F(AutofillIeToolbarImportTest, TestAutofillImport) {
             profiles[1].GetInfo(AutofillType(PHONE_HOME_CITY_CODE), "US"));
   EXPECT_EQ(L"5555555",
             profiles[1].GetInfo(AutofillType(PHONE_HOME_NUMBER), "US"));
-  EXPECT_EQ(L"+1 650-555-5555",
-            profiles[1].GetRawInfo(PHONE_HOME_WHOLE_NUMBER));
+  EXPECT_EQ(L"1 650-555-5555", profiles[1].GetRawInfo(PHONE_HOME_WHOLE_NUMBER));
 
   EXPECT_EQ(profile2[0].value, profiles[0].GetRawInfo(NAME_FIRST));
   EXPECT_EQ(profile2[1].value, profiles[0].GetRawInfo(NAME_LAST));
diff --git a/components/autofill/core/browser/phone_number_i18n.cc b/components/autofill/core/browser/phone_number_i18n.cc
index 10a71d6..246c5eb 100644
--- a/components/autofill/core/browser/phone_number_i18n.cc
+++ b/components/autofill/core/browser/phone_number_i18n.cc
@@ -65,12 +65,24 @@ void FormatValidatedNumber(const PhoneNumber& number,
   std::string processed_number;
   phone_util->Format(number, format, &processed_number);
 
+  std::string region_code;
+  phone_util->GetRegionCodeForNumber(number, &region_code);
+
+  // Drop the leading '+' for US numbers as some US sites can't handle the "+",
+  // and in the US dialing "+1..." is the same as dialing "1...".
+  std::string prefix;
+  if (processed_number[0] == '+') {
+    processed_number = processed_number.substr(1);
+    if (region_code != "US")
+      prefix = "+";
+  }
+
   if (formatted_number)
-    *formatted_number = base::UTF8ToUTF16(processed_number);
+    *formatted_number = base::UTF8ToUTF16(prefix + processed_number);
 
   if (normalized_number) {
     phone_util->NormalizeDigitsOnly(&processed_number);
-    *normalized_number = base::UTF8ToUTF16(processed_number);
+    *normalized_number = base::UTF8ToUTF16(prefix + processed_number);
   }
 }
 
diff --git a/components/autofill/core/browser/phone_number_i18n_unittest.cc b/components/autofill/core/browser/phone_number_i18n_unittest.cc
index 37592d3..e7ca9a2 100644
--- a/components/autofill/core/browser/phone_number_i18n_unittest.cc
+++ b/components/autofill/core/browser/phone_number_i18n_unittest.cc
@@ -149,58 +149,58 @@ TEST(PhoneNumberI18NTest, ConstructPhoneNumber) {
                                    ASCIIToUTF16("2345678"),
                                    "US",
                                    &number));
-  EXPECT_EQ(number, ASCIIToUTF16("+1 650-234-5678"));
+  EXPECT_EQ(ASCIIToUTF16("1 650-234-5678"), number);
   EXPECT_TRUE(ConstructPhoneNumber(base::string16(),
                                    ASCIIToUTF16("650"),
                                    ASCIIToUTF16("2345678"),
                                    "US",
                                    &number));
-  EXPECT_EQ(number, ASCIIToUTF16("(650) 234-5678"));
+  EXPECT_EQ(ASCIIToUTF16("(650) 234-5678"), number);
   EXPECT_TRUE(ConstructPhoneNumber(ASCIIToUTF16("1"),
                                    base::string16(),
                                    ASCIIToUTF16("6502345678"),
                                    "US",
                                    &number));
-  EXPECT_EQ(number, ASCIIToUTF16("+1 650-234-5678"));
+  EXPECT_EQ(ASCIIToUTF16("1 650-234-5678"), number);
   EXPECT_TRUE(ConstructPhoneNumber(base::string16(),
                                    base::string16(),
                                    ASCIIToUTF16("6502345678"),
                                    "US",
                                    &number));
-  EXPECT_EQ(number, ASCIIToUTF16("(650) 234-5678"));
+  EXPECT_EQ(ASCIIToUTF16("(650) 234-5678"), number);
 
   EXPECT_FALSE(ConstructPhoneNumber(base::string16(),
                                     ASCIIToUTF16("650"),
                                     ASCIIToUTF16("234567890"),
                                     "US",
                                     &number));
-  EXPECT_EQ(number, base::string16());
+  EXPECT_EQ(base::string16(), number);
   // Italian number
   EXPECT_TRUE(ConstructPhoneNumber(ASCIIToUTF16("39"),
                                    ASCIIToUTF16("347"),
                                    ASCIIToUTF16("2345678"),
                                    "IT",
                                    &number));
-  EXPECT_EQ(number, ASCIIToUTF16("+39 347 234 5678"));
+  EXPECT_EQ(ASCIIToUTF16("+39 347 234 5678"), number);
   EXPECT_TRUE(ConstructPhoneNumber(base::string16(),
                                    ASCIIToUTF16("347"),
                                    ASCIIToUTF16("2345678"),
                                    "IT",
                                    &number));
-  EXPECT_EQ(number, ASCIIToUTF16("347 234 5678"));
+  EXPECT_EQ(ASCIIToUTF16("347 234 5678"), number);
   // German number.
   EXPECT_TRUE(ConstructPhoneNumber(ASCIIToUTF16("49"),
                                    ASCIIToUTF16("024"),
                                    ASCIIToUTF16("2345678901"),
                                    "DE",
                                    &number));
-  EXPECT_EQ(number, ASCIIToUTF16("+49 2423 45678901"));
+  EXPECT_EQ(ASCIIToUTF16("+49 2423 45678901"), number);
   EXPECT_TRUE(ConstructPhoneNumber(base::string16(),
                                    ASCIIToUTF16("024"),
                                    ASCIIToUTF16("2345678901"),
                                    "DE",
                                    &number));
-  EXPECT_EQ(number, ASCIIToUTF16("02423 45678901"));
+  EXPECT_EQ(ASCIIToUTF16("02423 45678901"), number);
 }
 
 TEST(PhoneNumberI18NTest, PhoneNumbersMatch) {
diff --git a/components/autofill/core/browser/phone_number_unittest.cc b/components/autofill/core/browser/phone_number_unittest.cc
index b858768..9486e6a 100644
--- a/components/autofill/core/browser/phone_number_unittest.cc
+++ b/components/autofill/core/browser/phone_number_unittest.cc
@@ -107,7 +107,7 @@ TEST(PhoneNumberTest, SetInfo) {
   EXPECT_TRUE(phone.SetInfo(AutofillType(PHONE_HOME_WHOLE_NUMBER),
                             ASCIIToUTF16("+18887776666"),
                             "US"));
-  EXPECT_EQ(ASCIIToUTF16("+1 888-777-6666"),
+  EXPECT_EQ(ASCIIToUTF16("1 888-777-6666"),
             phone.GetRawInfo(PHONE_HOME_WHOLE_NUMBER));
 
   // Differently formatted numbers should be left alone.
@@ -173,7 +173,7 @@ TEST(PhoneNumberTest, PhoneCombineHelper) {
   base::string16 parsed_phone;
   EXPECT_TRUE(number1.ParseNumber(profile, "en-US", &parsed_phone));
   // International format as it has a country code.
-  EXPECT_EQ(ASCIIToUTF16("+1 650-234-5678"), parsed_phone);
+  EXPECT_EQ(ASCIIToUTF16("1 650-234-5678"), parsed_phone);
 
   PhoneNumber::PhoneCombineHelper number3;
   EXPECT_TRUE(number3.SetInfo(AutofillType(PHONE_HOME_CITY_CODE),
diff --git a/components/test/data/autofill/OWNERS b/components/test/data/autofill/OWNERS
new file mode 100644
index 0000000..3883338
--- /dev/null
+++ b/components/test/data/autofill/OWNERS
@@ -0,0 +1,5 @@
+estade@chromium.org
+isherman@chromium.org
+
+# Owner for password autofill/generation only.
+gcasto@chromium.org
diff --git a/components/test/data/autofill/merge/output/multimerge.out b/components/test/data/autofill/merge/output/multimerge.out
index ecefee1..9120c75 100644
--- a/components/test/data/autofill/merge/output/multimerge.out
+++ b/components/test/data/autofill/merge/output/multimerge.out
@@ -14,5 +14,5 @@ ADDRESS_HOME_CITY: San Francisco
 ADDRESS_HOME_STATE: CA
 ADDRESS_HOME_ZIP: 94102
 ADDRESS_HOME_COUNTRY: US
-PHONE_HOME_WHOLE_NUMBER: +1 650-210-1111
+PHONE_HOME_WHOLE_NUMBER: 1 650-210-1111
 PHONE_HOME_WHOLE_NUMBER: (650) 234-3333
diff --git a/components/test/data/autofill/merge/output/primarycase.out b/components/test/data/autofill/merge/output/primarycase.out
index ecefee1..9120c75 100644
--- a/components/test/data/autofill/merge/output/primarycase.out
+++ b/components/test/data/autofill/merge/output/primarycase.out
@@ -14,5 +14,5 @@ ADDRESS_HOME_CITY: San Francisco
 ADDRESS_HOME_STATE: CA
 ADDRESS_HOME_ZIP: 94102
 ADDRESS_HOME_COUNTRY: US
-PHONE_HOME_WHOLE_NUMBER: +1 650-210-1111
+PHONE_HOME_WHOLE_NUMBER: 1 650-210-1111
 PHONE_HOME_WHOLE_NUMBER: (650) 234-3333
diff --git a/components/test/data/autofill/merge/output/singlemerge.out b/components/test/data/autofill/merge/output/singlemerge.out
index 76e4e25..17e2663 100644
--- a/components/test/data/autofill/merge/output/singlemerge.out
+++ b/components/test/data/autofill/merge/output/singlemerge.out
@@ -10,4 +10,4 @@ ADDRESS_HOME_CITY: San Francisco
 ADDRESS_HOME_STATE: NY
 ADDRESS_HOME_ZIP: 11001
 ADDRESS_HOME_COUNTRY: CA
-PHONE_HOME_WHOLE_NUMBER: +1 650-210-1111
+PHONE_HOME_WHOLE_NUMBER: 1 650-210-1111