1061130 - [STK] 3GPP TS 22.030 6.6.4 An abbreviated dialling code shall be able to be read using the following procedure: N(N)(N)#

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Dialer, defect, P2)

Description

[sync-1]

Firefox OS v1.3
 AU_LINUX_GECKO_B2G_JB_3.2.01.03.00.112.301
 Mozilla build ID:20140422024003
 
 DEFECT DESCRIPTION:
 'The first record of the phonebook and the own numbers can not be verified by pressing 1#.
 
 REPRODUCING PROCEDURES:
 The first record of the phonebook and the own numbers can not be verified by pressing 1#.
 
 Expected Behaviour: 
 
 
 EXPECTED BEHAVIOUR:
 
 ASSOCIATE SPECIFICATION:
 
 TEST PLAN REFERENCE:
 
 TOOLS AND PLATFORMS USED:
 
 USER IMPACT:
 
 REPRODUCING RATE:
 
 For FT PR, Please list reference mobile's behavior:

Comment 1

[sync-1]

Attachment: OK QXDM log

Comment 2

[sync-1]

Attachment: KO ADB log

Comment 3

[sync-1]

Attachment: KO QXDM log

Comment 4

[Lingling Zhao]

Dear Vance,

This PR is similar as 857522. 
we only need to do as follow:
 when user input '1#' on dialer, we should fill the first number on sim card on
dialer screen;
 when user input '2#', we should fill the second number on sim card on dialer
screen;
 ...
if there is no contacts on simcard, we should do nothing;

In v1.1, we use navigator.mozContacts.getSimContacts('ADN'); to get the contact in the sim card.

But in V1.3, this interface is disappear, so I follow the contact app and use navigator.mozIccManager.getIccById(iccManager.iccIds[0]).readContacts('adn'), although it can return the contacts in the sim card, but the order is not correct.

I cannot find a proper interface to realize the function, could you please find someone to help me?

Thanks very much!

Comment 5

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Hi Kevin, Shawn -

Any one can shed some lights about this issue for partner?

Thanks

Vance

Comment 6

[shawn ku [:sku]]

Hi there:
 If I understand the issue correctly, this should be a feature defined by 3GPP 6.6.4 Reading the abbreviated dialling code.
An abbreviated dialling code shall be able to be read using the following procedure:
N(N)(N)#
Alternative additional procedures are also permitted.


This feature is not supported by FxOS yet.
Please raise your request to EPM to have further discussion.

Comment 7

[Lingling Zhao]

We have realized this function in our side in V1.1, but now in v1.3 that interface is disappear, so I have to use other interface, Do you know any this kind of interface?

Thanks.

Comment 8

[shawn ku [:sku]]

(In reply to Lingling Zhao from comment #7)
> We have realized this function in our side in V1.1, but now in v1.3 that
> interface is disappear, so I have to use other interface, Do you know any
> this kind of interface?
> 
> Thanks.

Please check below code to see if is what you need.

https://github.com/mozilla-b2g/gaia/blob/master/shared/js/contacts/import/utilities/import_sim_contacts.js#L119

Comment 9

[Lingling Zhao]

As I said in comment 4, I use navigator.mozIccManager.getIccById(iccManager.iccIds[0]).readContacts('adn') which is the use of L119, but the order in the result is not right.

Comment 10

[Lingling Zhao]

Dears,

I try to reverse the patch so that I can use navigator.mozContacts.getSimContacts('ADN'),:
https://github.com/mozilla/gecko-dev/commit/1ff5ff69ac03f71c66e7fadc736c1620a90937ac#diff-f80af006e8dbfd2a88cf1def0051e7beL27

But when I call this interface, there is a error that mRIL.getICCContacts is not a function.

Can you please help me to resolve it?

Thanks a lot.

Comment 11

[Lingling Zhao]

I sort the result from .readContacts('adn') by id by myself, so the problem is fixed.

Thanks for all your help, please close this PR.

Comment 12

[Gabriele Svelto [:gsvelto]]

Comment 11 suggests to close this but I wonder if we shouldn't be implementing this in master. Vance what do you think?

Comment 13

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

(In reply to Gabriele Svelto [:gsvelto] from comment #12)
> Comment 11 suggests to close this but I wonder if we shouldn't be
> implementing this in master. Vance what do you think?

Hi Garbriele, yes I do think we should put this into the backlog and try to implement it in master. Are you taking this one?

Thanks

Comment 14

[Gabriele Svelto [:gsvelto]]

(In reply to Vance Chen [:vchen][[e-mail]] from comment #13)
> Hi Garbriele, yes I do think we should put this into the backlog and try to
> implement it in master. Are you taking this one?

Yes, I'll be taking it. BTW I think that bug 1092968 is a duplicate of this one.

Comment 15

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

(In reply to Gabriele Svelto [:gsvelto] from comment #14)
> (In reply to Vance Chen [:vchen][[e-mail]] from comment #13)
> > Hi Garbriele, yes I do think we should put this into the backlog and try to
> > implement it in master. Are you taking this one?
> 
> Yes, I'll be taking it. BTW I think that bug 1092968 is a duplicate of this
> one.

Yes it is a duplicate of this one. And since we do need this one for future IOT purpose, i would like to nominate this issue as 2.2+. Please let me know you think otherwise

Thanks

Vance

Comment 16

[shawn ku [:sku]]

(In reply to Gabriele Svelto [:gsvelto] from comment #14)
> (In reply to Vance Chen [:vchen][[e-mail]] from comment #13)
> > Hi Garbriele, yes I do think we should put this into the backlog and try to
> > implement it in master. Are you taking this one?
> 
> Yes, I'll be taking it. BTW I think that bug 1092968 is a duplicate of this
> one.

Yes, Bug 1092968 is dup. to 1061130.
I copy the test case from 3GPP here again (see bug 1092968 comment 10).


Below is the 3GPP spec. to define the test of ADN.
3GPP TS 51.010-1
27.15 Abbreviated Dialling Numbers (ADN)

27.15.4.2 Procedure
a) Retrieve data from SIM entry number 7 using the procedure N(N)(N)#.
b) Retrieve data from SIM entry number 6 using the procedure N(N)(N)#.
c) Retrieve data from SIM entry number 101 using the procedure N(N)(N)#.
d) Retrieve data from SIM entry number 1 using the procedure N(N)(N)#.

27.15.5
Test requirements
1) After step a), the number "[phone]" shall be displayed.
2) After step b), the number "00112233" shall be displayed.
3) After step c), the number "**21*44556677#" (or an equivalent representation) shall be displayed.
4) After step d), the number “123” shall be displayed.

Comment 18

[Francisco Jordano [:arcturus] [:francisco]]

Hi, bug 1092968 was going to be taken by partner, in fact in comment 11 (https://bugzilla.mozilla.org/show_bug.cgi?id=1092968#c11) apparently the partner already has the fix for this.

@vchen, is that correct, should we take the partner patch and review?

Comment 19

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

triage: this is certification blocker so let's block on it.

From the original dup (bug 1092968) I didn't see partner showing interest to take & contribute on this bug.
Very likely we have to pick it up.

Comment 20

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Hi LingLing -

We are wondering if you can contribute your patch back to our master brunch? it can avoid future code fragment between Mozilla and TCL trunk

Thanks

Vance

Comment 21

[Lingling Zhao]

Attachment: patch.patch

Hi all,

Please see the patch for your reference, the user can get the first(second,third ...)record in the sim card after input 1#(2#,3#...).

Comment 22

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

(In reply to Lingling Zhao from comment #21)
> Created attachment 8536269 [details] [diff] [review]
> patch.patch
> 
> Hi all,
> 
> Please see the patch for your reference, the user can get the
> first(second,third ...)record in the sim card after input 1#(2#,3#...).

Thanks for your help LingLing!

Vance

Comment 23

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Hi Francisco -

Partner just attached their patch, please help to check if we can adapt this patch

Thanks

Vance

Comment 24

[Francisco Jordano [:arcturus] [:francisco]]

Hi,

will redirect this to dialer owner, Doug, as he is the responsible for the dialer app.

Comment 25

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8536269 [details] [diff] [review]
patch.patch

Hi Doug, could you take a look (or find someone) to this patch?

Thanks!

Comment 26

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8536269 [details] [diff] [review]
patch.patch

This needs lots of work and some tests.

Comment 27

[Josh Cheng [:josh]]

Modify marking from blocking-b2g to feature-b2g 2.2+. Still in scope of 2.2.

Comment 28

[Kai-Chih Hu [:khu]]

Wesley, can you find a owner for this bug? Thank you.

Comment 29

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Hi Doug,
Global 3-week sprint (S3) just began but we actually get 2 weeks considering holidays. 
If the target milestone is still valid, who will take this bug?

Comment 30

[Hubert Lu[:hlu] <[e-mail]>]

Hi Eric,
    Per talk, please help check if this case should be added in moztrap. Thank you.

Comment 31

[Eric Chang [:ericcc] [:echang]]

Dialer case here for checking USSD/MMI regression.
https://moztrap.mozilla.org/manage/suites/?pagenumber=1&pagesize=20&sortfield=created_on&sortdirection=desc&filter-name=%5Bcomms%5Ddialer

Comment 32

[Josh Cheng [:josh]]

Dear Gabriele,
Do you have any update for this bug? Is 1/9 still reasonable ETA for you? Thanks!

Comment 33

[Josh Cheng [:josh]]

Hi Doug,
Just wondering whether you have any update from Gabriele about this bug. I modify the ETA to S4(Jan 23), please feel free to  change it back if Gabriele can still make it at 1/9.
Thanks!

Comment 34

[Gabriele Svelto [:gsvelto]]

Sorry for the delay here, I was so busy landing bug 1081871 I lost track of this. I'm working on this today.

Comment 35

[Gabriele Svelto [:gsvelto]]

Hi Carrie, I need some additional info here about the UX of this bug.

The basic flow here is the following: when a user taps a short number followed by the # key (e.g. 1#) a contact corresponding to the number is taken from the SIM card and displayed instead. For example if my SIM card contains a single contact with number 0123456 and I type 1# in the dialer, the 1# code should be replaced with the number 0123456 as soon as I tap the # key.

There's a couple of issues to address though:

- First of all to implement this we need to read contacts from the SIM card and this is a slow operation and it might take time effectively freezing the dialer in the meantime. Attachment 8536269 [details] [diff] adds a spinner to cope with this but I'm not sure if that's the best approach. Doug suggested we now use a screen with a description of what's happening instead. What do you suggest?

- The second problem is, on a dual-SIM device, what SIM card should we take the contacts from? My idea was to take them from the default SIM card but I wanted this to be confirmed.

Comment 36

[Gabriele Svelto [:gsvelto]]

Attachment: [PATCH WIP] Add speed dial functionality to the dialer

This is a first iteration of the patch. It does implement all the required functionality but doesn't add a waiting spinner and lacks unit-tests.

As far as the waiting spinner/overlay goes adding it might be slightly complex. The issue is we already have two overlays in the dialer: one is for the suggestion list and the other is the waiting overlay for sending MMI messages. The second could be repurposed for the job here since it's already a waiting overlay but it's rather poorly done IMHO. I'd rather use shared code for it, such as the one used in the contacts app (check overlay.js there) but I fear there might be CSS clashes due to it.

Comment 37

[Gabriele Svelto [:gsvelto]]

Attachment: [PULL REQUEST] Add speed dial functionality to the dialer

Comment 38

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8547032 [details] [diff] [review]
[PATCH WIP] Add speed dial functionality to the dialer

Looks good.

(In reply to Gabriele Svelto [:gsvelto] from comment #36)
> As far as the waiting spinner/overlay goes adding it might be slightly
> complex. The issue is we already have two overlays in the dialer: one is for
> the suggestion list and the other is the waiting overlay for sending MMI
> messages. The second could be repurposed for the job here since it's already
> a waiting overlay but it's rather poorly done IMHO. I'd rather use shared
> code for it, such as the one used in the contacts app (check overlay.js
> there) but I fear there might be CSS clashes due to it.

We could use the gaia-confirm web component. Here's an example:
http://gaia-components.github.io/gaia-confirm/examples/

Comment 39

[Gabriele Svelto [:gsvelto]]

Attachment: [PATCH] Add speed dial functionality to the dialer

Complete patch minus unit-tests. The PR contains the three different incremental pushes for ease of review.

Comment 40

[Doug Sherk (:drs) (inactive)]

Attachment: Add speed dial functionality to the Dialer.

PR: https://github.com/mozilla-b2g/gaia/pull/27329

This is still Gabriele's bug, I'm just taking this through the final steps of review.

Will create followups for issues here.

Comment 41

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8547758 [details] [diff] [review]
Add speed dial functionality to the Dialer.

Going to bake the unit tests into this patch instead.

Comment 43

[Carrie Wang [New account: [e-mail]]]

Hi Gabriele, 

I'd suggest popping up a window which shows the description "Retrieving the contact's numbers" and displaying a progress bar underneath it (just like the importing contacts window). What do you say?

For the DSDS case, I think we shall go with the default SIM, but if it's set with "Always ask", can we take contacts from both SIM? If not, then we can just search it in SIM 1. Thanks!


(In reply to Gabriele Svelto [:gsvelto] from comment #35)
> Hi Carrie, I need some additional info here about the UX of this bug.
> 
> The basic flow here is the following: when a user taps a short number
> followed by the # key (e.g. 1#) a contact corresponding to the number is
> taken from the SIM card and displayed instead. For example if my SIM card
> contains a single contact with number 0123456 and I type 1# in the dialer,
> the 1# code should be replaced with the number 0123456 as soon as I tap the
> # key.
> 
> There's a couple of issues to address though:
> 
> - First of all to implement this we need to read contacts from the SIM card
> and this is a slow operation and it might take time effectively freezing the
> dialer in the meantime. Attachment 8536269 [details] [diff] adds a spinner
> to cope with this but I'm not sure if that's the best approach. Doug
> suggested we now use a screen with a description of what's happening
> instead. What do you suggest?
> 
> - The second problem is, on a dual-SIM device, what SIM card should we take
> the contacts from? My idea was to take them from the default SIM card but I
> wanted this to be confirmed.

Comment 44

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8547033 [details] [review]
[PULL REQUEST] Add speed dial functionality to the dialer

Un-obsoleting since I've resumed pushing stuff to it.

Comment 45

[Gabriele Svelto [:gsvelto]]

Attachment: [PATCH] Add speed dial functionality to the dialer

This corrects a small fix for when the speed dial index doesn't correspond to any contact, unit-tests as well as a small fix to the IccManager mock to make the contacts request behave more like a real DOMRequest.

Tamara, I've pushed my changes on top of Doug's modified patch you already reviewed. You can check them as a separate patch on the PR so it's simpler to spot what changed.

Comment 46

[Tamara Hills [:thills]]

Hi Gabriele,

I finished reviewing and testing and had a few comments in the PR.  If you can just take a look and see if you think any of these are issue (or need follow ups), that would be great.  Otherwise, this looks good to go.

-tamara

Comment 47

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8548247 [details] [diff] [review]
[PATCH] Add speed dial functionality to the dialer

I did some backseat reviewing on this and left comments on the PR. You don't need to request my review on the next iteration -- I'll let Tamara take it to completion.

Comment 48

[Gabriele Svelto [:gsvelto]]

Attachment: [PATCH v3] Add speed dial functionality to the dialer

I've added all the missing tests (including the SIM picker one as well as the 0# speed dialing code) and ensured we don't try to use negative indexes when picking SIM contacts. I had to change the MockMozIccManager mock to provide asynchronous behavior. The mock behaved synchronously by default and this made it impossible to observe what happened while the contacts request was pending.

The incremental changes have been pushed on top of the PR for ease of review.

Comment 49

[Tamara Hills [:thills]]

Comment on attachment 8548937 [details] [diff] [review]
[PATCH v3] Add speed dial functionality to the dialer

Hi Gabriele,

Looks good!  Thanks for adding all those tests.  Just a reminder that we should probably file a follow up just so we have clarity on what the requirement is for the emergency case.

Thanks,

-tamara

Comment 50

[Gabriele Svelto [:gsvelto]]

(In reply to Carrie Wang [:carrie] from comment #43)
> I'd suggest popping up a window which shows the description "Retrieving the
> contact's numbers" and displaying a progress bar underneath it (just like
> the importing contacts window). What do you say?

Thanks for the feedback Carrie, I've implemented something like that - an overlay with the standard "loading contacts" message - but without the progress bar for simplicity. I'll file a follow up bug to add the progress bar.

> For the DSDS case, I think we shall go with the default SIM, but if it's set
> with "Always ask", can we take contacts from both SIM? If not, then we can
> just search it in SIM 1. Thanks!

I think that taking contacts from both SIMs would be very confusing because we need to sort the contacts before choosing which one to pick and this means that the contacts would all be mixed up. I've implemented the standard SIM picker in this situation instead, do you think it's acceptable?

 Gabriele

Comment 51

[Carrie Wang [New account: [e-mail]]]

Hi  Gabriele, 

So, in this "Always ask" case, after entering the code, does user need to tap the call button to bring up the SIM picker or it will pop up right away? 
In addition, after choosing the SIM picker for the numbers, this numbers will be populated on the screen and then the user needs to tap the call button to bring up the SIM picker again for dialing out? 

Looks like the SIM picker will pop up two times in this case, one for retrieving numbers and the other for calling out. I think it will be fine, but still want to understand the whole flow. :)
Thanks!

Comment 52

[Gabriele Svelto [:gsvelto]]

(In reply to Carrie Wang [:carrie] from comment #51)
> So, in this "Always ask" case, after entering the code, does user need to
> tap the call button to bring up the SIM picker or it will pop up right away? 

Yes.

> In addition, after choosing the SIM picker for the numbers, this numbers
> will be populated on the screen and then the user needs to tap the call
> button to bring up the SIM picker again for dialing out?

Yes, that's going to be the case.

> Looks like the SIM picker will pop up two times in this case, one for
> retrieving numbers and the other for calling out. I think it will be fine,
> but still want to understand the whole flow. :)

Yes, it would work this way because the speed dial functionality only gets the number on the screen but doesn't dial out. So dialing out will pop up the SIM picker again.

Comment 53

[Gabriele Svelto [:gsvelto]]

OK, here's my green try run: https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=dbf11817f3cf

I had to retrigger a couple of suites due to intermittent issues. Landed on gaia/master 67b424672a69d73b4bc94b354ba088b50758d482

https://github.com/mozilla-b2g/gaia/commit/67b424672a69d73b4bc94b354ba088b50758d482

Comment 54

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8548937 [details] [diff] [review]
[PATCH v3] Add speed dial functionality to the dialer

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): New feature
[User impact] if declined: Speed dial as described in 3GPP TS 22.030 6.6.4 is not available in the dialer
[Testing completed]: Tested on a device with both one and two SIMs and covered all code paths with unit-tests
[Risk to taking this patch] (and alternatives if risky): None, this is a new feature
[String changes made]: None

Comment 55

[Doug Sherk (:drs) (inactive)]

checkin-needed is not needed as the sheriffs know to uplift this based on tracking flags.

Comment 56

[Ryan VanderMeulen [:RyanVM]]

We rock like that.

v2.2: https://github.com/mozilla-b2g/gaia/commit/5feeecad9d1215bcc61b99a7dfe828c102bdf7d7