786694 - B2G RIL: Import 2G SIM Contacts

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Currently B2G cannot import 2G SIM(i.e. APP_TYPE_SIM) contacts,
we need to fix this.

Comment 1

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: WIP - Part 1: Get 2G SIM contact

Comment 2

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: WIP - Part 2: Error callback

Comment 3

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Upload current WIP Patches,
but cannot test on the phone because I met problems in Bug 786686

Comment 4

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Part 1: Patch to get sim contact

Comment 5

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Part 2: Error callback

Hi, philikon
This patch deals with error, i.e. when sim is not inserted
So ContactService still get get callback.

Thanks

Comment 6

[Philipp von Weitershausen [:philikon]]

Comment on attachment 657302 [details] [diff] [review]
Part 1: Patch to get sim contact

Review of attachment 657302 [details] [diff] [review]:
-----------------------------------------------------------------

I liek.

::: dom/system/gonk/ril_worker.js
@@ +1420,5 @@
> +   *
> +   * @params type
> +   *         "ADN" or "FDN".
> +   * @params requestId
> +   *         Request id from RadioInterfaceLayer.

Don't need to mention `requestId` since the ril_worker is oblivious to it.

@@ +1428,5 @@
> +    switch (type) {
> +      case "ADN":
> +        switch (this.appType) {
> +          case CARD_APPTYPE_SIM:
> +            options.fileId = ICC_EF_ADN; 

Nit: trailing whitespace (also below)

Comment 7

[Philipp von Weitershausen [:philikon]]

Comment on attachment 657304 [details] [diff] [review]
Part 2: Error callback

Review of attachment 657304 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/system/gonk/ril_worker.js
@@ +1360,5 @@
>        type:      EF_TYPE_LINEAR_FIXED,
>        callback:  callback,
>        loadAll:   true,
> +      requestId: options.requestId,
> +      error:     error

Can we call this `onerror` and put it next to the `callback` property, to signify that they're both functions.

(I'm not also wondering whether we should rename `callback` to `onsuccess` to match, but that's more work and risk than I'd like to introduce right now.)

@@ +1477,5 @@
> +    function error(options) {
> +      this.sendDOMMessage({rilMessageType: "icccontacts",
> +                           contactType: "ADN",
> +                           contacts: [],
> +                           requestId: options.requestId});

Reuse `options`? (Also above.)

Comment 8

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Part 1: Patch to get sim contact v2.

Address to philikon's comments
- remove requestId from comments
- remove trailing whitespace.

Comment 9

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Part 2: Error callback v2

Address to philikon's comments
- error -> onerror and move it next to callback
- reuse options

Comment 10

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Part 1: Patch to get sim contact v2.

Rebase

Comment 11

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/164b65db56b6
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ceee266e44f

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/164b65db56b6
https://hg.mozilla.org/mozilla-central/rev/0ceee266e44f