From 3ac88c7106e7dcb553bcc794b15f19185928a1c6 Mon Sep 17 00:00:00 2001 From: penmetsaa Date: Thu, 15 Aug 2019 03:04:03 +0000 Subject: [PATCH] Abolish MX's international mobile token (1) (#2376) * Abolish MX's international mobile token * Porting phoneutil changes to JS and AYTF changes to not to swallow parts that are not part of formatting rule. * Make fake change to trigger travis build again --- cpp/src/phonenumbers/asyoutypeformatter.cc | 20 ++++++++++++++-- cpp/src/phonenumbers/phonenumberutil.cc | 1 - cpp/test/phonenumbers/phonenumberutil_test.cc | 4 ++-- .../i18n/phonenumbers/AsYouTypeFormatter.java | 14 ++++++++++- .../i18n/phonenumbers/PhoneNumberUtil.java | 1 - .../phonenumbers/PhoneNumberUtilTest.java | 24 +++++++++---------- .../i18n/phonenumbers/phonenumberutil.js | 1 - .../i18n/phonenumbers/phonenumberutil_test.js | 4 ++-- 8 files changed, 47 insertions(+), 22 deletions(-) diff --git a/cpp/src/phonenumbers/asyoutypeformatter.cc b/cpp/src/phonenumbers/asyoutypeformatter.cc index dbe061d41..f12e18e68 100644 --- a/cpp/src/phonenumbers/asyoutypeformatter.cc +++ b/cpp/src/phonenumbers/asyoutypeformatter.cc @@ -479,8 +479,24 @@ void AsYouTypeFormatter::AttemptToFormatAccruedDigits( DCHECK(status); IGNORE_UNUSED(status); - AppendNationalNumber(formatted_number, formatted_result); - return; + string full_output(*formatted_result); + // Check that we didn't remove nor add any extra digits when we matched + // this formatting pattern. This usually happens after we entered the last + // digit during AYTF. Eg: In case of MX, we swallow mobile token (1) when + // formatted but AYTF should retain all the number entered and not change + // in order to match a format (of same leading digits and length) display + // in that way. + AppendNationalNumber(formatted_number, &full_output); + phone_util_.NormalizeDiallableCharsOnly(&full_output); + string accrued_input_without_formatting_stdstring; + accrued_input_without_formatting_.toUTF8String( + accrued_input_without_formatting_stdstring); + if (full_output == accrued_input_without_formatting_stdstring) { + // If it's the same (i.e entered number and format is same), then it's + // safe to return this in formatted number as nothing is lost / added. + AppendNationalNumber(formatted_number, formatted_result); + return; + } } } } diff --git a/cpp/src/phonenumbers/phonenumberutil.cc b/cpp/src/phonenumbers/phonenumberutil.cc index cc4632816..3b60a40c1 100644 --- a/cpp/src/phonenumbers/phonenumberutil.cc +++ b/cpp/src/phonenumbers/phonenumberutil.cc @@ -536,7 +536,6 @@ class PhoneNumberRegExpsAndMappings { all_plus_number_grouping_symbols_.insert(std::make_pair(c, c)); } - mobile_token_mappings_.insert(std::make_pair(52, '1')); mobile_token_mappings_.insert(std::make_pair(54, '9')); geo_mobile_countries_without_mobile_area_codes_.insert(86); // China geo_mobile_countries_.insert(52); // Mexico diff --git a/cpp/test/phonenumbers/phonenumberutil_test.cc b/cpp/test/phonenumbers/phonenumberutil_test.cc index 1a1fb865e..da6838150 100644 --- a/cpp/test/phonenumbers/phonenumberutil_test.cc +++ b/cpp/test/phonenumbers/phonenumberutil_test.cc @@ -1541,9 +1541,9 @@ TEST_F(PhoneNumberUtilTest, GetCountryMobileToken) { int country_calling_code; string mobile_token; - country_calling_code = phone_util_.GetCountryCodeForRegion(RegionCode::MX()); + country_calling_code = phone_util_.GetCountryCodeForRegion(RegionCode::AR()); phone_util_.GetCountryMobileToken(country_calling_code, &mobile_token); - EXPECT_EQ("1", mobile_token); + EXPECT_EQ("9", mobile_token); // Country calling code for Sweden, which has no mobile token. country_calling_code = phone_util_.GetCountryCodeForRegion(RegionCode::SE()); diff --git a/java/libphonenumber/src/com/google/i18n/phonenumbers/AsYouTypeFormatter.java b/java/libphonenumber/src/com/google/i18n/phonenumbers/AsYouTypeFormatter.java index fe86ca2ec..a0d9957b2 100644 --- a/java/libphonenumber/src/com/google/i18n/phonenumbers/AsYouTypeFormatter.java +++ b/java/libphonenumber/src/com/google/i18n/phonenumbers/AsYouTypeFormatter.java @@ -426,7 +426,19 @@ public class AsYouTypeFormatter { NATIONAL_PREFIX_SEPARATORS_PATTERN.matcher( numberFormat.getNationalPrefixFormattingRule()).find(); String formattedNumber = m.replaceAll(numberFormat.getFormat()); - return appendNationalNumber(formattedNumber); + // Check that we did not remove nor add any extra digits when we matched + // this formatting pattern. This usually happens after we entered the last + // digit during AYTF. Eg: In case of MX, we swallow mobile token (1) when + // formatted but AYTF should retain all the number entered and not change + // in order to match a format (of same leading digits and length) display + // in that way. + String fullOutput = appendNationalNumber(formattedNumber); + String formattedNumberDigitsOnly = PhoneNumberUtil.normalizeDiallableCharsOnly(fullOutput); + if (formattedNumberDigitsOnly.contentEquals(accruedInputWithoutFormatting)) { + // If it's the same (i.e entered number and format is same), then it's + // safe to return this in formatted number as nothing is lost / added. + return fullOutput; + } } } return ""; diff --git a/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java b/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java index bbdc16edb..3b7deb1a6 100644 --- a/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java +++ b/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java @@ -122,7 +122,6 @@ public class PhoneNumberUtil { static { HashMap mobileTokenMap = new HashMap(); - mobileTokenMap.put(52, "1"); mobileTokenMap.put(54, "9"); MOBILE_TOKEN_MAPPINGS = Collections.unmodifiableMap(mobileTokenMap); diff --git a/java/libphonenumber/test/com/google/i18n/phonenumbers/PhoneNumberUtilTest.java b/java/libphonenumber/test/com/google/i18n/phonenumbers/PhoneNumberUtilTest.java index bdce305c0..488ea6348 100644 --- a/java/libphonenumber/test/com/google/i18n/phonenumbers/PhoneNumberUtilTest.java +++ b/java/libphonenumber/test/com/google/i18n/phonenumbers/PhoneNumberUtilTest.java @@ -352,8 +352,8 @@ public class PhoneNumberUtilTest extends TestMetadataTestCase { } public void testGetCountryMobileToken() { - assertEquals("1", PhoneNumberUtil.getCountryMobileToken(phoneUtil.getCountryCodeForRegion( - RegionCode.MX))); + assertEquals("9", PhoneNumberUtil.getCountryMobileToken(phoneUtil.getCountryCodeForRegion( + RegionCode.AR))); // Country calling code for Sweden, which has no mobile token. assertEquals("", PhoneNumberUtil.getCountryMobileToken(phoneUtil.getCountryCodeForRegion( @@ -1804,26 +1804,26 @@ public class PhoneNumberUtilTest extends TestMetadataTestCase { public void testExtractPossibleNumber() { // Removes preceding funky punctuation and letters but leaves the rest untouched. - assertEquals("0800-345-600", PhoneNumberUtil.extractPossibleNumber("Tel:0800-345-600")); - assertEquals("0800 FOR PIZZA", PhoneNumberUtil.extractPossibleNumber("Tel:0800 FOR PIZZA")); + assertEquals("0800-345-600", PhoneNumberUtil.extractPossibleNumber("Tel:0800-345-600").toString()); + assertEquals("0800 FOR PIZZA", PhoneNumberUtil.extractPossibleNumber("Tel:0800 FOR PIZZA").toString()); // Should not remove plus sign - assertEquals("+800-345-600", PhoneNumberUtil.extractPossibleNumber("Tel:+800-345-600")); + assertEquals("+800-345-600", PhoneNumberUtil.extractPossibleNumber("Tel:+800-345-600").toString()); // Should recognise wide digits as possible start values. assertEquals("\uFF10\uFF12\uFF13", - PhoneNumberUtil.extractPossibleNumber("\uFF10\uFF12\uFF13")); + PhoneNumberUtil.extractPossibleNumber("\uFF10\uFF12\uFF13").toString()); // Dashes are not possible start values and should be removed. assertEquals("\uFF11\uFF12\uFF13", - PhoneNumberUtil.extractPossibleNumber("Num-\uFF11\uFF12\uFF13")); + PhoneNumberUtil.extractPossibleNumber("Num-\uFF11\uFF12\uFF13").toString()); // If not possible number present, return empty string. - assertEquals("", PhoneNumberUtil.extractPossibleNumber("Num-....")); + assertEquals("", PhoneNumberUtil.extractPossibleNumber("Num-....").toString()); // Leading brackets are stripped - these are not used when parsing. - assertEquals("650) 253-0000", PhoneNumberUtil.extractPossibleNumber("(650) 253-0000")); + assertEquals("650) 253-0000", PhoneNumberUtil.extractPossibleNumber("(650) 253-0000").toString()); // Trailing non-alpha-numeric characters should be removed. - assertEquals("650) 253-0000", PhoneNumberUtil.extractPossibleNumber("(650) 253-0000..- ..")); - assertEquals("650) 253-0000", PhoneNumberUtil.extractPossibleNumber("(650) 253-0000.")); + assertEquals("650) 253-0000", PhoneNumberUtil.extractPossibleNumber("(650) 253-0000..- ..").toString()); + assertEquals("650) 253-0000", PhoneNumberUtil.extractPossibleNumber("(650) 253-0000.").toString()); // This case has a trailing RTL char. - assertEquals("650) 253-0000", PhoneNumberUtil.extractPossibleNumber("(650) 253-0000\u200F")); + assertEquals("650) 253-0000", PhoneNumberUtil.extractPossibleNumber("(650) 253-0000\u200F").toString()); } public void testMaybeStripNationalPrefix() { diff --git a/javascript/i18n/phonenumbers/phonenumberutil.js b/javascript/i18n/phonenumbers/phonenumberutil.js index 3410fecb8..c52279d6e 100644 --- a/javascript/i18n/phonenumbers/phonenumberutil.js +++ b/javascript/i18n/phonenumbers/phonenumberutil.js @@ -167,7 +167,6 @@ i18n.phonenumbers.PhoneNumberUtil.COLOMBIA_MOBILE_TO_FIXED_LINE_PREFIX_ = '3'; * @private */ i18n.phonenumbers.PhoneNumberUtil.MOBILE_TOKEN_MAPPINGS_ = { - 52: '1', 54: '9' }; diff --git a/javascript/i18n/phonenumbers/phonenumberutil_test.js b/javascript/i18n/phonenumbers/phonenumberutil_test.js index c8ccbaaaa..b0d9e6d86 100644 --- a/javascript/i18n/phonenumbers/phonenumberutil_test.js +++ b/javascript/i18n/phonenumbers/phonenumberutil_test.js @@ -424,8 +424,8 @@ function testGetLengthOfNationalDestinationCode() { } function testGetCountryMobileToken() { - assertEquals('1', i18n.phonenumbers.PhoneNumberUtil.getCountryMobileToken( - phoneUtil.getCountryCodeForRegion(RegionCode.MX))); + assertEquals('9', i18n.phonenumbers.PhoneNumberUtil.getCountryMobileToken( + phoneUtil.getCountryCodeForRegion(RegionCode.AR))); // Country calling code for Sweden, which has no mobile token. assertEquals('', i18n.phonenumbers.PhoneNumberUtil.getCountryMobileToken(