Closed
Bug 900850
Opened 10 years ago
Closed 10 years ago
B2G CDMA: Support accessing contacts of CDMA RUIM
Categories
(Core :: DOM: Device Interfaces, defect)
Tracking
()
People
(Reporter: edgar, Assigned: edgar)
References
Details
Attachments
(2 files, 4 obsolete files)
9.15 KB,
patch
|
edgar
:
review+
|
Details | Diff | Splinter Review |
13.25 KB,
patch
|
edgar
:
review+
|
Details | Diff | Splinter Review |
We support access contacts of SIM and USIM. We should support access contacts in RUIM for CDMA.
Assignee | ||
Comment 1•10 years ago
|
||
Please see 3GPP2 C.S0023-D session 2.7
Assignee | ||
Comment 2•10 years ago
|
||
* FDN: Same as 3GPP TS 51.011 session 10.5.2 * ADN: Optionally provide enhanced phonebook in DF_PHONEBOOK. - If support enhanced phonebook: Same as 3GPP TS 31.105 session 4.4.2 - If not: Same as 3GPP TS 51.011 session 10.5.1
Assignee | ||
Comment 3•10 years ago
|
||
Assignee | ||
Comment 4•10 years ago
|
||
Attachment #784924 -
Attachment is obsolete: true
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 786168 [details] [diff] [review] Part 1: Support accessing contacts of CDMA RUIM, v2 (In reply to Edgar Chen [:edgar][:echen] from comment #2) > * FDN: Same as 3GPP TS 51.011 session 10.5.2 > * ADN: Optionally provide enhanced phonebook in DF_PHONEBOOK. > - If support enhanced phonebook: Same as 3GPP TS 31.105 session 4.4.2 > - If not: Same as 3GPP TS 51.011 session 10.5.1 Depended on Service n6 in EF_CST. (Enhanced Phone Book) Please see C.S.0023-D session 3.4.18 If service is allocated and actived, same as 3GPP TS 31.105 session 4.4.2. (USIM) Otherwise, same as 3GPP TS 51.011 session 10.5.1. (SIM) I have tested this patch with n970 + 亞太 RUIM, it works well. I am trying to write a xpcshell tests now. Thanks
Attachment #786168 -
Flags: review?(allstars.chh)
Comment on attachment 786168 [details] [diff] [review] Part 1: Support accessing contacts of CDMA RUIM, v2 Review of attachment 786168 [details] [diff] [review]: ----------------------------------------------------------------- This patch is okay, but I'd like to make it simpler. ::: dom/system/gonk/ril_consts.js @@ +1164,5 @@ > OPL: 46, > SPDI: 51 > }, > ruim: { > + ENHANCED_PHONE_BOOK: 6, Is it a seperated word? Phonebook or Phone book? ::: dom/system/gonk/ril_worker.js @@ +12192,5 @@ > + case CARD_APPTYPE_RUIM: > + if (ICCUtilsHelper.isICCServiceAvailable("ENHANCED_PHONE_BOOK")) { > + return true; > + } > + return false; return ICCUtilsHelper.isICCServiceAvailable("ENHANCED_PHONE_BOOK"); @@ +12194,5 @@ > + return true; > + } > + return false; > + default: > + throw new Error("Unknown icc type"); This function should simply return true or false, why do you trying to make it more complicated? @@ +12209,5 @@ > */ > readICCContacts: function readICCContacts(appType, contactType, onsuccess, onerror) { > switch (contactType) { > case "adn": > + try { If we remove the throwing above, then we could remove this try-catch block as well to make it simpler. @@ +12224,5 @@ > break; > case "fdn": > ICCRecordHelper.readADNLike(ICC_EF_FDN, onsuccess, onerror); > break; > + default: How about let error = onerorr || debug; error(...) with more detail error message. Same as below.
Attachment #786168 -
Flags: review?(allstars.chh)
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #6) > Comment on attachment 786168 [details] [diff] [review] > Part 1: Support accessing contacts of CDMA RUIM, v2 > > Review of attachment 786168 [details] [diff] [review]: > ----------------------------------------------------------------- > > This patch is okay, > but I'd like to make it simpler. > > ::: dom/system/gonk/ril_consts.js > @@ +1164,5 @@ > > OPL: 46, > > SPDI: 51 > > }, > > ruim: { > > + ENHANCED_PHONE_BOOK: 6, > > Is it a seperated word? > Phonebook or Phone book? It should be Phonebook, I will correct this. > > ::: dom/system/gonk/ril_worker.js > @@ +12192,5 @@ > > + case CARD_APPTYPE_RUIM: > > + if (ICCUtilsHelper.isICCServiceAvailable("ENHANCED_PHONE_BOOK")) { > > + return true; > > + } > > + return false; > > return ICCUtilsHelper.isICCServiceAvailable("ENHANCED_PHONE_BOOK"); > > @@ +12194,5 @@ > > + return true; > > + } > > + return false; > > + default: > > + throw new Error("Unknown icc type"); > > This function should simply return true or false, > why do you trying to make it more complicated? It is because I am not sure what value should be returned if someone pass a unknown SIM type. Maybe we could just return false in this case, instead of throwing an error. > > @@ +12209,5 @@ > > */ > > readICCContacts: function readICCContacts(appType, contactType, onsuccess, onerror) { > > switch (contactType) { > > case "adn": > > + try { > > If we remove the throwing above, then we could remove this try-catch block > as well to make it simpler. Okay, I will remove throwing and try-catch to make it simpler. > > @@ +12224,5 @@ > > break; > > case "fdn": > > ICCRecordHelper.readADNLike(ICC_EF_FDN, onsuccess, onerror); > > break; > > + default: > > How about > let error = onerorr || debug; > error(...) > with more detail error message. > > Same as below. Okay, I will use this. Thanks
Assignee | ||
Comment 8•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #786168 -
Attachment is obsolete: true
Assignee | ||
Comment 9•10 years ago
|
||
Comment on attachment 786709 [details] [diff] [review] Part 1: Support accessing contacts of CDMA RUIM, v3 Address review comment #6
Attachment #786709 -
Attachment description: bug_900850_part1_support_accessing_contacts_of_cdam_ruim_v3.patch → Part 1: Support accessing contacts of CDMA RUIM, v3
Assignee | ||
Comment 10•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #786709 -
Flags: review?(allstars.chh)
Assignee | ||
Updated•10 years ago
|
Attachment #786772 -
Flags: review?(allstars.chh)
Comment on attachment 786709 [details] [diff] [review] Part 1: Support accessing contacts of CDMA RUIM, v3 Review of attachment 786709 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/ril_worker.js @@ +12299,5 @@ > > /** > * Helper function to update ICC contact. > * > + * @param appType CARD_APPTYPE_SIM or CARD_APPTYPE_USIM or CARD_APPTYPE_RUIM. nit, One of ...
Attachment #786709 -
Flags: review?(allstars.chh) → review+
Comment on attachment 786772 [details] [diff] [review] Part 2: Xpcshell tests for accessing contacts of CDMA RUIM, v1 Review of attachment 786772 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/tests/test_ril_worker_icc.js @@ +2444,5 @@ > + let onsuccess = function onsuccess(contacts) { > + let contact = contacts[0]; > + for (key in contact) { > + do_print("check " + key); > + if (contact[key] instanceof Array) { Array.isArray See http://web.mit.edu/jwalden/www/isArray.html
Attachment #786772 -
Flags: review?(allstars.chh) → review+
Assignee | ||
Comment 13•10 years ago
|
||
Address review comment #11.
Attachment #786709 -
Attachment is obsolete: true
Attachment #786858 -
Flags: review+
Assignee | ||
Comment 14•10 years ago
|
||
Address review comment #12.
Attachment #786772 -
Attachment is obsolete: true
Attachment #786863 -
Flags: review+
Assignee | ||
Comment 15•10 years ago
|
||
try server: https://tbpl.mozilla.org/?tree=Try&rev=c7286c9ccfd7
Assignee | ||
Updated•10 years ago
|
blocking-b2g: --- → koi?
Assignee | ||
Comment 16•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/bee83df3c47d https://hg.mozilla.org/integration/b2g-inbound/rev/733697c789e8
Comment 17•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/bee83df3c47d https://hg.mozilla.org/mozilla-central/rev/733697c789e8
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Updated•10 years ago
|
blocking-b2g: koi? → koi+
Updated•10 years ago
|
status-b2g-v1.2:
--- → fixed
status-firefox26:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•