Closed Bug 849119 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: DOM: Device Interfaces, defect)

18 Branch
x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla22
blocking-b2g tef+
Tracking Status
firefox20 --- wontfix
firefox21 --- wontfix
firefox22 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- fixed

People

(Reporter: djf, Assigned: gwagner)

Details

Attachments

(1 file)

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.)
I cited the wrong bug above.  The new bug about NormalizeNumber() handling letters incorrectly is bug 849119
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.
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.
Flags: needinfo?(anygregor)
I just merged it upstream and will update our gecko code!
Thanks for fixing!
Flags: needinfo?(anygregor)
Assignee: nobody → anygregor
blocking-b2g: --- → tef?
Attached file pointer
Attachment #722812 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/40dff07b49f5
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
(tef+, minor patch + tests, and there is no DTMF for the number 10)
blocking-b2g: tef? → tef+
You need to log in before you can comment on or make changes to this bug.