Skip to content

GitLab

T
tangled
Commit 4d37e5c3 authored by rsleevi@chromium.org's avatar rsleevi@chromium.org
Browse files
Options

Revert 282761 "Change PhoneNumber::SetInfo to only apply formatt..." 

Possible failure on Android ( http://build.chromium.org/p/chromium.linux/buildstatus?builder=Android%20Tests%20%28dbg%29&number=21495 )

> 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

TBR=estade@chromium.org

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

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