Skip to content

GitLab

T
tangled
Commit 663fafcf authored by estade@chromium.org's avatar estade@chromium.org
Browse files
Options

Change PhoneNumber::SetInfo to only apply formatting where there is none 

See discussion on https://codereview.chromium.org/347183005/

BUG=none

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@282761 0039d316-1c4b-4281-b951-d872f2087c98
parent 952e716e
Show whitespace changes
Inline Side-by-side
Showing with 123 additions and 87 deletions
chrome/browser/autofill/autofill_browsertest.cc
View file @ 663fafcf
......@@ -621,18 +621,16 @@ IN_PROC_BROWSER_TEST_F(AutofillTest, ProfileSavedWithValidCountryPhone) {
FillFormAndSubmit("autofill_test_form.html", profiles[i]);
ASSERT_EQ(2u, personal_data_manager()->GetProfiles().size());
ASSERT_EQ(ASCIIToUTF16("(408) 871-4567"),
ASSERT_EQ(ASCIIToUTF16("408-871-4567"),
personal_data_manager()->GetProfiles()[0]->GetRawInfo(
PHONE_HOME_WHOLE_NUMBER));
ASSERT_EQ(ASCIIToUTF16("+49 40 808179000"),
ASSERT_EQ(ASCIIToUTF16("+49 40-80-81-79-000"),
personal_data_manager()->GetProfiles()[1]->GetRawInfo(
PHONE_HOME_WHOLE_NUMBER));
}
// Test Autofill appends country codes to aggregated phone numbers.
// The country code is added for the following case:
// The phone number contains the correct national number size and
// is a valid format.
// Prepend country codes when formatting phone numbers, but only if the user
// provided one in the first place.
IN_PROC_BROWSER_TEST_F(AutofillTest, AppendCountryCodeForAggregatedPhones) {
ASSERT_TRUE(test_server()->Start());
FormMap data;
......@@ -643,13 +641,22 @@ IN_PROC_BROWSER_TEST_F(AutofillTest, AppendCountryCodeForAggregatedPhones) {
data["ADDRESS_HOME_STATE"] = "CA";
data["ADDRESS_HOME_ZIP"] = "95110";
data["ADDRESS_HOME_COUNTRY"] = "Germany";
data["PHONE_HOME_WHOLE_NUMBER"] = "(08) 450 777-777";
data["PHONE_HOME_WHOLE_NUMBER"] = "+4908450777777";
FillFormAndSubmit("autofill_test_form.html", data);
ASSERT_EQ(1u, personal_data_manager()->GetProfiles().size());
base::string16 phone = personal_data_manager()->GetProfiles()[0]->GetRawInfo(
PHONE_HOME_WHOLE_NUMBER);
ASSERT_TRUE(StartsWith(phone, ASCIIToUTF16("+49"), true));
data["ADDRESS_HOME_LINE1"] = "4321 H St.";
data["PHONE_HOME_WHOLE_NUMBER"] = "08450777777";
FillFormAndSubmit("autofill_test_form.html", data);
ASSERT_EQ(2u, personal_data_manager()->GetProfiles().size());
EXPECT_EQ(ASCIIToUTF16("+49 8450 777777"),
personal_data_manager()->GetProfiles()[0]->GetRawInfo(
PHONE_HOME_WHOLE_NUMBER));
FillFormAndSubmit("autofill_test_form.html", data);
EXPECT_EQ(ASCIIToUTF16("08450 777777"),
personal_data_manager()->GetProfiles()[1]->GetRawInfo(
PHONE_HOME_WHOLE_NUMBER));
}
// Test CC info not offered to be saved when autocomplete=off for CC field.
......
chrome/browser/ui/autofill/data_model_wrapper.cc
View file @ 663fafcf
......@@ -135,7 +135,7 @@ base::string16 AutofillProfileWrapper::GetInfoForDisplay(
// If there is no user-defined formatting at all, add some standard
// formatting.
if (base::ContainsOnlyChars(phone_number,
base::ASCIIToUTF16("0123456789"))) {
base::ASCIIToUTF16("+0123456789"))) {
std::string region = base::UTF16ToASCII(
GetInfo(AutofillType(HTML_TYPE_COUNTRY_CODE, HTML_MODE_NONE)));
i18n::PhoneObject phone(phone_number, region);
......
components/autofill/core/browser/personal_data_manager_unittest.cc
View file @ 663fafcf
......@@ -2339,9 +2339,19 @@ TEST_F(PersonalDataManagerTest, CaseInsensitiveMultiValueAggregation) {
base::MessageLoop::current()->Run();
AutofillProfile expected(base::GenerateGUID(), "https://www.example.com");
test::SetProfileInfo(&expected, "George", NULL,
"Washington", "theprez@gmail.com", NULL, "21 Laussat St", NULL,
"San Francisco", "California", "94102", NULL, "(817) 555-6789");
test::SetProfileInfo(&expected,
"George",
NULL,
"Washington",
"theprez@gmail.com",
NULL,
"21 Laussat St",
NULL,
"San Francisco",
"California",
"94102",
NULL,
"817-555-6789");
const std::vector<AutofillProfile*>& results1 = personal_data_->GetProfiles();
ASSERT_EQ(1U, results1.size());
EXPECT_EQ(0, expected.Compare(*results1[0]));
......@@ -2386,7 +2396,7 @@ TEST_F(PersonalDataManagerTest, CaseInsensitiveMultiValueAggregation) {
// Modify expected to include multi-valued fields.
std::vector<base::string16> values;
expected.GetRawMultiInfo(PHONE_HOME_WHOLE_NUMBER, &values);
values.push_back(ASCIIToUTF16("(214) 555-1234"));
values.push_back(ASCIIToUTF16("214-555-1234"));
expected.SetRawMultiInfo(PHONE_HOME_WHOLE_NUMBER, values);
ASSERT_EQ(1U, results2.size());
......
components/autofill/core/browser/phone_number.cc
View file @ 663fafcf
......@@ -134,9 +134,16 @@ bool PhoneNumber::SetInfo(const AutofillType& type,
if (number_.empty())
return true;
// Store a formatted (i.e., pretty printed) version of the number.
// Store a formatted (i.e., pretty printed) version of the number if either
// the number doesn't contain formatting marks.
UpdateCacheIfNeeded(app_locale);
if (base::ContainsOnlyChars(number_, base::ASCIIToUTF16("+0123456789"))) {
number_ = cached_parsed_phone_.GetFormattedNumber();
} else if (i18n::NormalizePhoneNumber(
number_, GetRegion(*profile_, app_locale)).empty()) {
// The number doesn't make sense for this region; clear it.
number_.clear();
}
return !number_.empty();
}
......
components/autofill/core/browser/phone_number_i18n.cc
View file @ 663fafcf
......@@ -100,7 +100,8 @@ bool ParsePhoneNumber(const base::string16& value,
// The |default_region| should already be sanitized.
DCHECK_EQ(2U, default_region.size());
if (phone_util->Parse(number_text, default_region, i18n_number) !=
if (phone_util->ParseAndKeepRawInput(
number_text, default_region, i18n_number) !=
PhoneNumberUtil::NO_PARSING_ERROR) {
return false;
}
......@@ -131,21 +132,13 @@ bool ParsePhoneNumber(const base::string16& value,
}
*number = base::UTF8ToUTF16(subscriber_number);
*city_code = base::UTF8ToUTF16(area_code);
*country_code = base::string16();
phone_util->NormalizeDigitsOnly(&number_text);
base::string16 normalized_number(base::UTF8ToUTF16(number_text));
// Check if parsed number has a country code that was not inferred from the
// region.
if (i18n_number->has_country_code()) {
if (i18n_number->has_country_code() &&
i18n_number->country_code_source() != PhoneNumber::FROM_DEFAULT_COUNTRY) {
*country_code = base::UTF8ToUTF16(
base::StringPrintf("%d", i18n_number->country_code()));
if (normalized_number.length() <= national_significant_number.length() &&
!StartsWith(normalized_number, *country_code,
true /* case_sensitive */)) {
country_code->clear();
}
}
// The region might be different from what we started with.
......
components/autofill/core/browser/phone_number_i18n_unittest.cc
View file @ 663fafcf
......@@ -57,59 +57,68 @@ TEST(PhoneNumberI18NTest, ParsePhoneNumber) {
std::string deduced_region;
} test_cases[] = {
// Test for empty string. Should give back empty strings.
{ false, "", "US" },
// Test for string with less than 7 digits. Should give back empty strings.
{ false, "1234", "US" },
{false, "", "US"},
// Test for string with less than 7 digits. Should give back empty
// strings.
{false, "1234", "US"},
// Test for string with exactly 7 digits.
// Not a valid number - starts with 1
{ false, "17134567", "US" },
{false, "17134567", "US"},
// Not a valid number - does not have area code.
{ false, "7134567", "US" },
{false, "7134567", "US"},
// Valid Canadian toll-free number.
{ true, "3101234", "US", "3101234", "", "", "CA" },
{true, "3101234", "US", "3101234", "", "", "CA"},
// Test for string with greater than 7 digits but less than 10 digits.
// Should fail parsing in US.
{ false, "123456789", "US" },
// Test for string with greater than 7 digits but less than 10 digits and
{false, "123456789", "US"},
// Test for string with greater than 7 digits but less than 10 digits
// and
// separators.
// Should fail parsing in US.
{ false, "12.345-6789", "US" },
{false, "12.345-6789", "US"},
// Test for string with exactly 10 digits.
// Should give back phone number and city code.
// This one going to fail because of the incorrect area code.
{ false, "1234567890", "US" },
// This one going to fail because of the incorrect number (starts with 1).
{ false, "6501567890", "US" },
{ true, "6504567890", "US", "4567890", "650", "", "US" },
{false, "1234567890", "US"},
// This one going to fail because of the incorrect number (starts with
// 1).
{false, "6501567890", "US"},
{true, "6504567890", "US", "4567890", "650", "", "US"},
// Test for string with exactly 10 digits and separators.
// Should give back phone number and city code.
{ true, "(650) 456-7890", "US", "4567890", "650", "", "US" },
{true, "(650) 456-7890", "US", "4567890", "650", "", "US"},
// Tests for string with over 10 digits.
// 01 is incorrect prefix in the USA, and if we interpret 011 as prefix, the
// 01 is incorrect prefix in the USA, and if we interpret 011 as prefix,
// the
// rest is too short for international number - the parsing should fail.
{ false, "0116504567890", "US" },
{false, "0116504567890", "US"},
// 011 is a correct "dial out" prefix in the USA - the parsing should
// succeed.
{ true, "01116504567890", "US", "4567890", "650", "1", "US" },
// 011 is a correct "dial out" prefix in the USA but the rest of the number
{true, "01116504567890", "US", "4567890", "650", "1", "US"},
// 011 is a correct "dial out" prefix in the USA but the rest of the
// number
// can't parse as a US number.
{ true, "01178124567890", "US", "4567890", "812", "7", "RU" },
{true, "01178124567890", "US", "4567890", "812", "7", "RU"},
// Test for string with over 10 digits with separator characters.
// Should give back phone number, city code, and country code. "011" is
// US "dial out" code, which is discarded.
{ true, "(0111) 650-456.7890", "US", "4567890", "650", "1" , "US" },
// Now try phone from Czech republic - it has 00 dial out code, 420 country
{true, "(0111) 650-456.7890", "US", "4567890", "650", "1", "US"},
// Now try phone from Czech republic - it has 00 dial out code, 420
// country
// code and variable length area codes.
{ true, "+420 27-89.10.112", "US", "910112", "278", "420", "CZ" },
{ false, "27-89.10.112", "US" },
{ true, "27-89.10.112", "CZ", "910112", "278", "", "CZ" },
{ false, "420 57-89.10.112", "US" },
{ true, "420 57-89.10.112", "CZ", "910112", "578", "420", "CZ" },
{true, "+420 27-89.10.112", "US", "910112", "278", "420", "CZ"},
{false, "27-89.10.112", "US"},
{true, "27-89.10.112", "CZ", "910112", "278", "", "CZ"},
{false, "420 57-89.10.112", "US"},
{true, "420 57-89.10.112", "CZ", "910112", "578", "420", "CZ"},
// Parses vanity numbers.
{ true, "1-650-FLOWERS", "US", "3569377", "650", "1", "US" },
// 800 is not an area code, but the destination code. In our library these
{true, "1-650-FLOWERS", "US", "3569377", "650", "1", "US"},
// 800 is not an area code, but the destination code. In our library
// these
// codes should be treated the same as area codes.
{ true, "1-800-FLOWERS", "US", "3569377", "800", "1", "US" },
{true, "1-800-FLOWERS", "US", "3569377", "800", "1", "US"},
// Don't add a country code where there was none.
{true, "(08) 450 777 7777", "DE", "7777777", "8450", "", "DE"},
};
for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_cases); ++i) {
......
components/autofill/core/browser/phone_number_unittest.cc
View file @ 663fafcf
......@@ -104,11 +104,21 @@ TEST(PhoneNumberTest, SetInfo) {
ASCIIToUTF16("8887776666"), "US"));
EXPECT_EQ(ASCIIToUTF16("(888) 777-6666"),
phone.GetRawInfo(PHONE_HOME_WHOLE_NUMBER));
EXPECT_TRUE(phone.SetInfo(AutofillType(PHONE_HOME_WHOLE_NUMBER),
ASCIIToUTF16("+18887776666"),
"US"));
EXPECT_EQ(ASCIIToUTF16("+1 888-777-6666"),
phone.GetRawInfo(PHONE_HOME_WHOLE_NUMBER));
// Differently formatted numbers should be re-formatted.
// Differently formatted numbers should be left alone.
EXPECT_TRUE(phone.SetInfo(AutofillType(PHONE_HOME_WHOLE_NUMBER),
ASCIIToUTF16("800-432-8765"), "US"));
EXPECT_EQ(ASCIIToUTF16("(800) 432-8765"),
EXPECT_EQ(ASCIIToUTF16("800-432-8765"),
phone.GetRawInfo(PHONE_HOME_WHOLE_NUMBER));
// SetRawInfo should not try to format.
phone.SetRawInfo(PHONE_HOME_WHOLE_NUMBER, ASCIIToUTF16("8004328765"));
EXPECT_EQ(ASCIIToUTF16("8004328765"),
phone.GetRawInfo(PHONE_HOME_WHOLE_NUMBER));
// Invalid numbers should not be stored. In the US, phone numbers cannot
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment