849119 - [dom/phonenumberutils] PhoneNumber.Normalize() handles letters incorrectly.

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[David Flanagan [:djf]]

In dom/phonenumberutils/PhoneNumber.js, in the function NormalizeNumber, at line 312, the code for converting from letters to numbers incorrectly converts the letters y and z to "10".  It assumes that the letters are evenly distributed in groups of 3.  The incorrect mathematical formula should be replaced by an object that maps letters to digits, I think.

As far as I can tell, however, gecko may not be using this library anymore (except for one call to isViable() from dom/system/gonk/RadioInterfaceLayer.js, and that may be removed by bug 846499). So can we consider just removing this library completely.  (I don't see it being used in gaia anymore, either, though I don't know why not.)

Comment 1

[David Flanagan [:djf]]

I cited the wrong bug above.  The new bug about NormalizeNumber() handling letters incorrectly is bug 849119

Comment 2

[David Flanagan [:djf]]

And now I'm commenting in the wrong bug! Please ignore the above.

Also, I was wrong about PhoneNumberUtils being unused.  It is used in dom/mobilemessage/. So we do actually need to fix the NormalizeNumber() function.

Comment 3

[David Flanagan [:djf]]

I've proposed a fix upstream in https://github.com/andreasgal/PhoneNumber.js/pull/15

I'm not attaching it here, so I'm just going to needinfo gregor to make sure he sees it. Gregor: I've asked you to review the upstream patch. If you like it we can use this bug to land it in gecko, too.

Comment 4

[Gregor Wagner [:gwagner]]

I just merged it upstream and will update our gecko code!
Thanks for fixing!

Comment 5

[Gregor Wagner [:gwagner]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/40dff07b49f5

Comment 6

[Gregor Wagner [:gwagner]]

Attachment: pointer

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/40dff07b49f5

Comment 8

[Michael Vines [:m1] [:evilmachines]]

(tef+, minor patch + tests, and there is no DTMF for the number 10)

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/9bab1dcf7f72
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/564f6ab9fdcb