Closed
Bug 842460
Opened 11 years ago
Closed 11 years ago
B2G RIL: Support reading ANR from Phonebook
Categories
(Core :: DOM: Device Interfaces, defect)
Tracking
()
RESOLVED
FIXED
mozilla22
People
(Reporter: allstars.chh, Assigned: allstars.chh)
References
Details
Attachments
(4 files, 6 obsolete files)
1.45 KB,
patch
|
hsinyi
:
review+
|
Details | Diff | Splinter Review |
1.15 KB,
patch
|
gwagner
:
review+
|
Details | Diff | Splinter Review |
4.46 KB,
patch
|
hsinyi
:
review+
|
Details | Diff | Splinter Review |
11.49 KB,
patch
|
Details | Diff | Splinter Review |
In EF_PBR there's also an EF called "ANR" (Additional Number), we should also read this EF when we read Phonebook.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → allstars.chh
Assignee | ||
Comment 1•11 years ago
|
||
Assignee | ||
Comment 2•11 years ago
|
||
Assignee | ||
Comment 3•11 years ago
|
||
Assignee | ||
Comment 4•11 years ago
|
||
Assignee | ||
Comment 5•11 years ago
|
||
Attachment #717039 -
Attachment is obsolete: true
Assignee | ||
Comment 6•11 years ago
|
||
Attachment #717045 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Attachment #717046 -
Flags: review?(htsai)
Assignee | ||
Updated•11 years ago
|
Attachment #717040 -
Flags: review?(htsai)
Assignee | ||
Updated•11 years ago
|
Attachment #717042 -
Flags: review?(htsai)
Assignee | ||
Updated•11 years ago
|
Attachment #717042 -
Flags: review?(htsai) → review?(anygregor)
Assignee | ||
Updated•11 years ago
|
Attachment #717051 -
Flags: review?(htsai)
Comment 7•11 years ago
|
||
Comment on attachment 717042 [details] [diff] [review] Part 3: Update ContactManager. Review of attachment 717042 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/contacts/ContactManager.js @@ +394,5 @@ > + // ANR - Additional Number > + if (c.anr) { > + for (let i = 0; i < c.anr.length; i++) { > + prop.tel.push(c.anr[i]); > + } You have to create a new object with the value property set. look above: tel: [ { value: c.number } ]}
Assignee | ||
Comment 8•11 years ago
|
||
Comment on attachment 717042 [details] [diff] [review] Part 3: Update ContactManager. Review of attachment 717042 [details] [diff] [review]: ----------------------------------------------------------------- Got it, thank you,
Attachment #717042 -
Flags: review?(anygregor)
Assignee | ||
Comment 9•11 years ago
|
||
Attachment #717040 -
Attachment is obsolete: true
Attachment #717040 -
Flags: review?(htsai)
Attachment #718273 -
Flags: review?(htsai)
Assignee | ||
Comment 10•11 years ago
|
||
Addressed to Gregor's comment, but I didn't send r? to Gregor because Bug 838092 is still under discussion.
Attachment #717042 -
Attachment is obsolete: true
Assignee | ||
Comment 11•11 years ago
|
||
Attachment #717051 -
Attachment is obsolete: true
Attachment #717051 -
Flags: review?(htsai)
Assignee | ||
Updated•11 years ago
|
Attachment #718276 -
Flags: review?(htsai)
Updated•11 years ago
|
Attachment #717046 -
Flags: review?(htsai) → review+
Comment 12•11 years ago
|
||
Comment on attachment 718273 [details] [diff] [review] Part 2: Read ANR in RIL. v2 Review of attachment 718273 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/ril_worker.js @@ +9169,5 @@ > > /** > + * Read USIM Phonebook EF_ANR. > + * > + * @see TS 131.102, clause 4.4.2.13 Should be clause 4.4.2.9? @@ +9171,5 @@ > + * Read USIM Phonebook EF_ANR. > + * > + * @see TS 131.102, clause 4.4.2.13 > + * > + * @param fileId EF id of the EMAIL. EF id should be ANR? @@ +9172,5 @@ > + * > + * @see TS 131.102, clause 4.4.2.13 > + * > + * @param fileId EF id of the EMAIL. > + * @param fileType The type of the EMAIL, one of the ICC_USIM_TYPE* constants. ditto. @@ +10063,5 @@ > > /** > + * Read Phonebook fields. > + * > + * @param pbr PBR file I prefer the full name to the abbreviation PBR. @@ +10069,5 @@ > + * @param onsuccess Callback to be called when success. > + * @param onerror Callback to be called when error. > + */ > + readPhonebookFields: function readPhonebookFields(pbr, contacts, onsuccess, onerror) { > + const fields = ["email", "anr0"]; Please leave a comment here, saying something that if we want to read more anr in the future, we just need to modify this const. @@ +10093,5 @@ > + * @param field Phonebook field to be retrieved. > + * @param onsuccess Callback to be called when success. > + * @param onerror Callback to be called when error. > + */ > + readPhonebookField: function readPhonebookField(pbr, contacts, field, onsuccess, onerror) { readPhonebookField and readPhonebookFields are too similar. Though I understand why you use these names, I also think it would be good to adopt more distinguishable names, or at least leave some comment. @@ +10103,5 @@ > + } > + > + (function doReadContactField(n) { > + if (n >= contacts.length) { > + // All contact's fields are readed. nit: past participle of 'read' is read, not readed. @@ +10122,3 @@ > * > * @param pbr The phonebook reference file. > * @param contact The contact needs to get email. to get email? revise this comment?
Attachment #718273 -
Flags: review?(htsai) → review+
Comment 13•11 years ago
|
||
Comment on attachment 718276 [details] [diff] [review] Part 4: xpcshell for read Anr v3 Review of attachment 718276 [details] [diff] [review]: ----------------------------------------------------------------- Great, thanks!
Attachment #718276 -
Flags: review?(htsai) → review+
Assignee | ||
Comment 14•11 years ago
|
||
Addressed Hsinyi's comments: 1. update comments (remove email-related comments, add full name for comments of 'pbr', and for current supported fields to be read) 2. rename readPhonebookFields to readSupportedPBRFields
Attachment #718273 -
Attachment is obsolete: true
Comment 15•11 years ago
|
||
Try run for 28edeb408ceb is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=28edeb408ceb Results (out of 4 total builds): success: 4 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/yhuang@mozilla.com-28edeb408ceb
Assignee | ||
Updated•11 years ago
|
Attachment #718275 -
Flags: review?(anygregor)
Updated•11 years ago
|
Attachment #718275 -
Flags: review?(anygregor) → review+
Assignee | ||
Comment 16•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/3b1aad4d18a9 https://hg.mozilla.org/integration/mozilla-inbound/rev/37185db4b9fe https://hg.mozilla.org/integration/mozilla-inbound/rev/5a37e14650b5 https://hg.mozilla.org/integration/mozilla-inbound/rev/d811830b3c44
Comment 17•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3b1aad4d18a9 https://hg.mozilla.org/mozilla-central/rev/37185db4b9fe https://hg.mozilla.org/mozilla-central/rev/5a37e14650b5 https://hg.mozilla.org/mozilla-central/rev/d811830b3c44
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in
before you can comment on or make changes to this bug.
Description
•