Closed Bug 817529 Opened 10 years ago Closed 10 years ago

B2G RIL: SPN should be shown if device is in PLMN that is listed in EF_SPDI

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-basecamp:+, firefox18 fixed, firefox19 fixed, firefox20 fixed)

RESOLVED FIXED
B2G C2 (20nov-10dec)
blocking-basecamp +
Tracking Status
firefox18 --- fixed
firefox19 --- fixed
firefox20 --- fixed

People

(Reporter: kk1fff, Assigned: kk1fff)

Details

Attachments

(1 file, 4 obsolete files)

This is a follow-up bug of bug 793111.
According to:
* TS 51.011 Sec. 10.3.11 and
* TS 31.102 Sec. 4.2.12
SPN should be shown carrier name if device is in PLMN that is listed in EF_SPDI.
Attached patch Patch (obsolete) — Splinter Review
1. Reading EF_SPDI for PLMN list in both SIM and USIM.
2. Set isDisplaySpnRequired = true when current PLMN is HPLMN or is one of PLMNs that are listed in EF_SPDI.

Vicamo, would you help to review this patch? Thanks.
Attachment #687704 - Flags: review?(vyang)
Try run for bfcaeff4fcb3 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=bfcaeff4fcb3
Results (out of 2 total builds):
    success: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/pwang@mozilla.com-bfcaeff4fcb3
Comment on attachment 687704 [details] [diff] [review]
Patch

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

Nice job!

::: dom/system/gonk/ril_worker.js
@@ +1237,5 @@
>        if (isOnMatchingPlmn) {
>          // The first bit of display condition tells us if we should display
>          // registered PLMN.
>          if (DEBUG) debug("updateDisplayCondition: PLMN is HPLMN or PLMN is in PLMN list");
> +        iccInfo.isDisplaySpnRequired = true;

Since the logic here is so complex and non-trivial that we have had several rounds of discussion/fix on it, I think it deserves some excerpts from the specification to make each line more clear.

@@ +1242,1 @@
>          if (iccSpn.spnDisplayCondition & 0x01) {

Ditto

@@ +1249,5 @@
>          // registered PLMN.
>          if (DEBUG) debug("updateICCDisplayName: PLMN isn't HPLMN and PLMN isn't in PLMN list");
>          if (iccSpn.spnDisplayCondition & 0x02) {
>            iccInfo.isDisplayNetworkNameRequired = false;
>            iccInfo.isDisplaySpnRequired = false;

Per previous discussion, we can't find whether should we turn isDisplaySpnRequired on here. So, some comments here are also appreciated.

@@ +1496,5 @@
>  
>    /**
>     * Read the SPDI (Service Provider Display Information) from the ICC.
>     *
>     * See TS 131.102 section 4.2.66

Please also add: See TS 51.011 section 10.3.50
Attachment #687704 - Flags: review?(vyang) → review+
blocking-basecamp: ? → +
Attached patch Patch (obsolete) — Splinter Review
r+ in comment 3. Add comment in the patch according to reviewer's comment.
Attachment #687704 - Attachment is obsolete: true
Attachment #688109 - Flags: review+
Attached patch Patch (obsolete) — Splinter Review
r+ in comment 3, fix wrong comment in previous version.
Attachment #688109 - Attachment is obsolete: true
Attachment #688110 - Flags: review+
Attached patch Patch (obsolete) — Splinter Review
Comment is still wrong, sorry. Update again.
Attachment #688110 - Attachment is obsolete: true
Attachment #688114 - Flags: review+
Attached patch PatchSplinter Review
Rebase.
Attachment #688114 - Attachment is obsolete: true
Attachment #688117 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/43344ee15385
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
This is marked blocking-basecamp+ but doesn't apply cleanly to aurora/beta. Please get bb+ on whatever this depends on or post a branch-specific patch.
Whoops, wrong bug. Ignore that.
You need to log in before you can comment on or make changes to this bug.