Closed
Bug 796618
Opened 12 years ago
Closed 7 years ago
[META] Phone number normalization utils for comms apps
Categories
(Firefox OS Graveyard :: Gaia, defect, P3)
Firefox OS Graveyard
Gaia
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: ghtobz, Unassigned)
References
Details
(Whiteboard: [label:dialer][label:contacts][label:sms][LOE:M][label:feature][gh-assignee:borjasalguero]dupme)
[GitHub issue by autonome on 2012-09-10T20:56:31Z, https://github.com/mozilla-b2g/gaia/issues/4528]
Not getting a platform fix for V1. Need to implement it in Gaia.
https://bugzilla.mozilla.org/show_bug.cgi?id=743363
@albertopq @etiennesegonzac @arcturus Which comms apps need this, and what should the API look like?
And who can implement it?
[GitHub comment by arcturus on 2012-09-10T21:47:54Z]
@autonome as far as I know will be involved almost all of them (all have contacts integration):
Dialer: has the call log + showing the contact detail when an incoming/outgoing call
Contacts: self explanatory
Sms: we do the threading by contacts as well
I don't have the who yet (@borjasalguero and @steveck-chung have been doing a lot of work around this for SMS), we will try to propose and discuss solutions for this ASAP.
Thanks!
[GitHub comment by fabricedesre on 2012-09-11T03:41:56Z]
I found this other library : https://github.com/floere/phony . We should look if porting this to JS is giving good results with a hopefully smaller codebase.
[GitHub comment by borjasalguero on 2012-09-12T11:49:30Z]
There is a proposal based on @steveck-chung work. https://github.com/mozilla-b2g/gaia/pull/4628
[GitHub comment by autonome on 2012-09-12T21:54:52Z]
Thanks y'all.
Will the work in that pr be usable by other apps? Or is SMS the only app that needs this number disambiguation feature for V1?
[GitHub comment by borjasalguero on 2012-09-13T06:30:11Z]
This library it's only needed in SMS App and Dialer. Once this PR will be approved we are going to add the same functionality to Dialer and the loop will be closed by now. We cant neglect that this is a workaround for 28th September deadline, because everybody agree that this functionality should be part of Gecko.
[GitHub comment by autonome on 2012-09-14T18:55:52Z]
@steveck-chung can you update on where things are at in pr 4628?
[GitHub comment by vingtetun on 2012-09-14T20:28:22Z]
@autonome PR 4628 has been merged. Code needs to be ported to the communications app now.
[GitHub comment by albertopq on 2012-09-17T13:07:38Z]
@gtorodelvalle, are you going to port it to the Dialer?
[GitHub comment by gtorodelvalle on 2012-09-17T13:18:40Z]
Yes ;-) I'll get in touch with Borja and we will organize the best way to do it :-)
Reporter | ||
Comment 10•12 years ago
|
||
[GitHub comment by gtorodelvalle on 2012-09-17T13:19:13Z]
I mean @borjasalguero ;-)
Reporter | ||
Comment 11•12 years ago
|
||
[GitHub comment by borjasalguero on 2012-09-17T13:33:49Z]
Im on it. Im trying to analyze how to implement it without impact the 'on_call.html', trying to minimize the effect of adding the library. I hope to have it in Gecko soon! :'(
Reporter | ||
Comment 12•12 years ago
|
||
[GitHub comment by steveck-chung on 2012-09-17T16:49:36Z]
Hi @borjasalguero , If you have any problem in using these libs, please contact me and I can check if there is anything I anything I can help with.
Reporter | ||
Comment 13•12 years ago
|
||
[GitHub comment by borjasalguero on 2012-09-17T17:06:38Z]
@steveck-chung Could you take a look to this branch https://github.com/borjasalguero/gaia/tree/feature_phonelibnumber_dialer ? You can change whatever you consider if is needed. Thanks again Steve!
Reporter | ||
Comment 14•12 years ago
|
||
[GitHub comment by autonome on 2012-09-26T23:44:16Z]
Been a while. Any update here?
Reporter | ||
Comment 15•12 years ago
|
||
[GitHub comment by borjasalguero on 2012-09-26T23:58:58Z]
We are waiting to take a decision on that. The PR is #5231
Comment 16•12 years ago
|
||
Please reassign to somebody on your team (borjasalguero doesn't have a BZ account).
Assignee: nobody → jmcanterafonseca
Priority: -- → P1
Updated•12 years ago
|
Assignee: jmcanterafonseca → fbsc
Importing a 5MB lib is unacceptable.
My understanding is that what's blocking is a concrete list of markets that need to be supported in v1. needsinfo? to gal to find that out from our friends.
Flags: needinfo?(gal)
Comment 18•12 years ago
|
||
1 0 17 49 2848K 224K 7476K 26M 2389M 47181 35245 sleeping 502 2073 173 38 18
1 0 17 53 6500K 224K 11M 27M 2390M 47186 35245 sleeping 502 3123 172 38 1
The google library is complete insanity. We have to remove it from the code base. It must not spread to any other apps. Above is a snapshot of loading the metadata.js file only. It contains an in-memory object graph representing number formating information. The metadata alone is 4MB in memory. The library also consumes more than 1MB for source code, and an amount I can't easily measure for compiled code.
In other words, this library uses several percent of our total device memory (and I mean total, including kernel, video memory, gecko, what have you not).
At the core of the library is a meta data table, which should go into indexeddb, and about 100 lines of parsing code. We can re-use most of the tests from the library. Lets do that. I am happy to help with the design if needed.
Flags: needinfo?(gal)
You can measure the memory usage more accurately with get-about-memory.py.
Comment 20•12 years ago
|
||
Only for clarifying this issue, because I think that there are a lot of noise and things are simpler than it looks ;).
As you know in our communications app we need a way of distinguish, univocally, a phone number. This issue is not straightforward, ‘numbering’ plans are different and non-standard all over the world, so creating a library for solving this problem it’s not an issue of days.
Everybody agree that this functionality should be part of Gecko, so we should push to have this functionality ready in our backend asap (This bug was reported 7 months ago... -https://github.com/mozilla-b2g/gaia/issues/857-). However, with only few weeks to have all apps working properly, and with no support in the backend we created this workaround for having it in Gaia. That’s why, after searching the most complete & tested one, we decided to use ‘phonenumberlib’ from Google (used in Android, Gmail and Facebook).
This is only a temporary solution, so will be removed once this feature will be in Gecko. And of course if somebody find a cool library for solving this issue better we could use it for sure! Nobody wants to loose RAM or performance :). But as I said, getting it working properly is tough.
Probably we should start now working in this functionality in Backend, because I think we all agree that it is the right solution. So let's work on it! ;)
However, we should also try to find a way to check if we could do a workaround at the Gaia level as it's a key point for our product. Original PR was here https://github.com/mozilla-b2g/gaia/pull/5231, and we could work based on that.
Comment 21•12 years ago
|
||
:borjasalguero, just talked to :gal he has a first version of a super-slimmed down library. He will share with you so you can maintain it.
Updated•12 years ago
|
Priority: P1 → --
Updated•12 years ago
|
Priority: -- → P3
Updated•12 years ago
|
Whiteboard: [label:dialer][label:contacts][label:sms][LOE:M][label:feature][gh-assignee:borjasalguero] → [label:dialer][label:contacts][label:sms][LOE:M][label:feature][gh-assignee:borjasalguero]dupme
Comment 22•12 years ago
|
||
Milestoning for C2 (deadline of 12/10), as this meets the criteria of "known bugs with LOE:M".
Target Milestone: --- → B2G C2 (20nov-10dec)
Comment 24•12 years ago
|
||
To carry my voice from bug 810444 into this one (assuming that duping made sense).
I have a heck of a time being able to figure out if my texts go through or not since I have to choose wisely which of the two numbers to reply to.
Right now I would go to say that my phone is not functional and I can't do texting properly.
If this is what is tried to be fixed in here that is awesome!
What I do know is that texting used to work reasonably well 4 weeks ago but it is now a mess.
Thanks for all your hardwork!
Comment 25•12 years ago
|
||
There are some changes coming, so in a near future more than 'adding' normalization to our comms app, we should delete the PhoneNumberJS because will be part of Gecko (for SMS App the work is on progress). Probably we could split this one into bugs as 'Handling normalization properly in SMS APP', 'Handling normalization properly in Dialer'. What do you think?
Comment 26•12 years ago
|
||
(In reply to Borja Salguero [:borjasalguero] from comment #25)
> There are some changes coming, so in a near future more than 'adding'
> normalization to our comms app, we should delete the PhoneNumberJS because
> will be part of Gecko (for SMS App the work is on progress). Probably we
> could split this one into bugs as 'Handling normalization properly in SMS
> APP', 'Handling normalization properly in Dialer'. What do you think?
Yep makes sense, 2 patches, 2 bugs.
Comment 27•12 years ago
|
||
This bug is, for SMS, is divided into 2 steps:
https://bugzilla.mozilla.org/show_bug.cgi?id=811539
https://bugzilla.mozilla.org/show_bug.cgi?id=811538
Comment 28•12 years ago
|
||
Is this bug a meta bug now?
Comment 29•12 years ago
|
||
[:vingtetun] I think that we could consider it a META. I will create the pending bugs.
Comment 30•12 years ago
|
||
(In reply to Borja Salguero [:borjasalguero] from comment #29)
> [:vingtetun] I think that we could consider it a META. I will create the
> pending bugs.
Nice. Can you then add the dependency on this bugs in the 'depends on' field. And then I will mark this bug as non-blocking (we don't block on meta bug) and the dependency should blocks.
Comment 31•12 years ago
|
||
Hi,
I'm now on the stable build of 11-14.
Things are better than when I reported bug 810444. Thanks!
There is one last thing to notice.
If I start a convo with a number without starting with "+1" I will receive back text messages from the "+1" version of that number and have two entries.
From there on, I keep on replying to the +1 entry.
In fact, I have deleted all of the entries that started with +1.
Let me know if you would like me to file a different bug without having a great severity.
Updated•12 years ago
|
Component: Gaia → Gaia::Dialer
Updated•12 years ago
|
Updated•12 years ago
|
blocking-basecamp: + → ---
Summary: phone number normalization utils for comms apps → [META] Phone number normalization utils for comms apps
Updated•12 years ago
|
Assignee: fbsc → nobody
Component: Gaia::Dialer → Gaia
Comment 32•12 years ago
|
||
[:vingtetun] Updated as a meta and all dependencies added. Thanks!
Comment 33•12 years ago
|
||
Clearing C2 milestone -- not a blocker.
Target Milestone: B2G C2 (20nov-10dec) → ---
Updated•7 years ago
|
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•