catamphetamine / libphonenumber-js

A simpler (and smaller) rewrite of Google Android's libphonenumber library in javascript
https://catamphetamine.gitlab.io/libphonenumber-js/
MIT License
2.77k stars 217 forks source link

Unexpected AsYouType result with certain country codes (VI, BS) #318

Closed mcgraphix closed 4 years ago

mcgraphix commented 5 years ago

I believe I have found an issue with the AsYouType feature. With the country code 'VI' (US Virgin Islands) or 'BS' (Bahamas), the input method returns unexpected values with partial phone numbers:

var asYouType = new libphonenumber.AsYouType('VI');

console.log(asYouType.input('(340) 693')); // (340) 693
asYouType.reset();

//***********These don't format correctly*************
console.log(asYouType.input('(340) 6934')); // 1 (340) 340-6934
asYouType.reset();

console.log(asYouType.input('3406934')); // 1 (340) 340-6934
asYouType.reset();
//*****************************************************

console.log(asYouType.input('(340) 69348')); // (340) 693-48
asYouType.reset();

console.log(asYouType.input('(340) 693480')); // (340) 693-48
asYouType.reset();

console.log(asYouType.input('(340) 6934800')); // (340) 693-4800
asYouType.reset();

You can see it happening in this fiddle: https://jsfiddle.net/1vptu0ad/2/

Interestingly, the issue does not happen in the demo here: https://catamphetamine.github.io/libphonenumber-js/

catamphetamine commented 5 years ago
    it('should parse VI "as you type" numbers', () => {
        // https://github.com/catamphetamine/libphonenumber-js/issues/318
        new AsYouType('VI').input('3406934').should.equal('(340) 6934')
    })

@mcgraphix Well, it seems to be a confusing-enough case. It's funny how the demo is actually a bit outdated (Jan 14, 2019) and if I compile it with the latest code + metadata it showcases the bug. This can be of potential assistance to nail it down. So, if you look at Google's demo: https://libphonenumber.appspot.com/phonenumberparser?number=%28340%29+6934&country=VI It says: National format | (340) 340-6934. I guess it means that no one in "Virgin Islands" inputs their number as (340) 6934 and instead they all input it right at 6934. Now, you can also see that Google's "as you type" doesn't prepend 340 but that I think is a bug because it's not consistent with the "National number" behaviour it exibits. I mean, you either prepend it or you don't: make up your mind. This library's code is not a 1:1 copy-paste so its behaviour differs in some aspects. I guess this is one of them. So, in short, I think it's not a bug because Google's demo exibits the same behaviour for the "National number". I'll update the demo to the latest one.

mcgraphix commented 5 years ago

That's really weird. In our data-entry case the person entering the phone number may not be familiar with the fact that you don't need to enter the 340 part. They would just be reading it off of something else.

It also seems like older versions of libphonenumer-js didn't exhibit this behavior. I only noticed it when upgrading to the latest.

On Thu, May 16, 2019, 4:18 AM Nikolay notifications@github.com wrote:

@mcgraphix https://github.com/mcgraphix Well, it seems to be a confusing-enough case. It's funny how the demo is actually a bit outdated and if I compile it with the latest code + metadata it showcases the bug. This can be of potential assistance to nail it down. So, if you look at Google's demo:

https://libphonenumber.appspot.com/phonenumberparser?number=%28340%29+6934&country=VI It says: National format | (340) 340-6934. I guess it means that no one in "Virgin Islands" inputs their number as (340) 6934 and instead they all input it right at 6934. Now, you can also see that Google's "as you type" doesn't prepend 340 but that I think is a bug because it's not consistend with the "National number" behaviour it exibits. I mean, you either prepend it or you don't: make up your mind. This library's code is not a 1:1 copy-paste so its behaviour differs in some aspects. I guess it's one of them. So, in short, I think it's not a bug because Google's demo exibits the same behaviour for the "National number". I'll update the demo to the latest one.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/catamphetamine/libphonenumber-js/issues/318?email_source=notifications&email_token=AANU7MRLTY22PJGLKDVIYITPVUKGTA5CNFSM4HNG3CCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVRBQOI#issuecomment-492967993, or mute the thread https://github.com/notifications/unsubscribe-auth/AANU7MS2X4UILFN2OOIVUZLPVUKGTANCNFSM4HNG3CCA .

catamphetamine commented 5 years ago

@mcgraphix Well, I guess I'll look into it.

mcgraphix commented 5 years ago

Thanks, I also filed a bug with google: https://issuetracker.google.com/issues/132928391

Although, while the formatted National Number does seem incorrect, the results of the As You Type on google's demo seems correct: https://libphonenumber.appspot.com/phonenumberparser?number=3406934&country=VI

AsYouTypeFormatter Results
Char entered: '3' Output: 3
Char entered: '4' Output: 34
Char entered: '0' Output: 340
Char entered: '6' Output: 340-6
Char entered: '9' Output: 340-69
Char entered: '3' Output: 340-693
Char entered: '4' Output: 340-6934
catamphetamine commented 5 years ago

To whoever comes here: I still haven't looked into the issue and currently I'm having a bit of busy days + it's Summer. But eventually I'll look into this issue and compare this library's code with Google's code to see where's the difference.

dmitry-salnikov commented 4 years ago

Same issue. Just compare VI and BS behavior in console:

new AsYouType('BS').input('2421111')
"(242) 111-1"
new AsYouType('VI').input('3401111')
"1 (340) 340-1111"

Any news on that?

catamphetamine commented 4 years ago

@dmitry-salnikov

Any news on that?

I guess I'll prioritize this one. It has really been a long time. I'll see if I can start this one before Christmas.

catamphetamine commented 4 years ago

Finally, fixed in libphonenumber-js@1.7.32. Refactored the code, sorted out some stuff, understood some things I didn't understand about how Google's libphonenumber worked.