1102677 - [Telephony] Support change Supplementary Services password through MMI code

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Szu-Yu Chen [:aknow] (inactive)]

3GPP TS 22.030
6.5.4 Registration of new password

* 03 * ZZ * OLD_PASSWORD * NEW_PASSWORD * NEW_PASSWORD #

The UE shall also support the alternative procedure: 

** 03 * ZZ * OLD_PASSWORD * NEW_PASSWORD * NEW_PASSWORD # 

where, for Barring Services, ZZ = 330; 

for a common password for all appropriate services, delete the ZZ, entering: 

* 03 ** OLD_PASSWORD * NEW_PASSWORD * NEW_PASSWORD # 

The UE shall also support the alternative procedure: 

** 03 ** OLD_PASSWORD * NEW_PASSWORD * NEW_PASSWORD #

Comment 1

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Part 1: Support change barring password

Comment 2

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Part 2: Test change barring password

Comment 3

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 8533628 [details] [diff] [review]
Part 1: Support change barring password

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

Overall it looks good except one question :)

::: dom/system/gonk/ril_worker.js
@@ +6056,5 @@
> +    this.sendChromeMessage(options);
> +    return;
> +  }
> +
> +  options.statusMessage = MMI_SM_KS_PASSWORD_CHANGED;

Do we really want to statusMessage MMI_SM_KS_PASSWORD_CHANGED even when rilRequestError is true?

Comment 4

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 8533628 [details] [diff] [review]
Part 1: Support change barring password

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

::: dom/system/gonk/ril_worker.js
@@ +6056,5 @@
> +    this.sendChromeMessage(options);
> +    return;
> +  }
> +
> +  options.statusMessage = MMI_SM_KS_PASSWORD_CHANGED;

Ignore my comment 3. 
Though we don't really care "statusMessage" in error cases, this is okay :) Thanks!

Comment 5

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 8533629 [details] [diff] [review]
Part 2: Test change barring password

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

Looks good to me! Thank you.

Comment 6

[Szu-Yu Chen [:aknow] (inactive)]

try looks good
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=5bacc7ea38a8

Comment 7

[Szu-Yu Chen [:aknow] (inactive)]

https://hg.mozilla.org/integration/b2g-inbound/rev/88787e45d2e1
https://hg.mozilla.org/integration/b2g-inbound/rev/de099dd00df8

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/88787e45d2e1
https://hg.mozilla.org/mozilla-central/rev/de099dd00df8

Comment 9

[Szu-Yu Chen [:aknow] (inactive)]

ni Josh for 2.0m? because it's the bug for Bug 1098978.

Comment 11

[Szu-Yu Chen [:aknow] (inactive)]

ni myself for 2.0m patch

Comment 12

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: (2.0m) [final] Support change barring password. r=hsinyi

Comment 13

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: (2.1) [final] Support change barring password. r=hsinyi

Comment 14

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Part 3: Add missing semicolon

Comment 15

[Szu-Yu Chen [:aknow] (inactive)]

https://hg.mozilla.org/integration/b2g-inbound/rev/579cfc80a665

This is a follow-up patch which fixes the following issue. I am sorry to cause the problem in previous patches.

TEST-UNEXPECTED-FAIL | test_ril_code_quality.py TestRILCodeQuality.test_ril_consts | AssertionError: See errors below and more information in Bug 880643

Comment 16

[Szu-Yu Chen [:aknow] (inactive)]

Comment on attachment 8541056 [details] [diff] [review]
(2.1) [final] Support change barring password. r=hsinyi

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): new feature not supported before
User impact if declined: user cannnot change call barring password through mmi code
Testing completed: N/A
Risk to taking this patch (and alternatives if risky): low ~ mid
String or UUID changes made by this patch: N/A

Comment 17

[Szu-Yu Chen [:aknow] (inactive)]

check-in needed for 2.0m and 2.1

Comment 18

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/579cfc80a665

Comment 19

[Szu-Yu Chen [:aknow] (inactive)]

check-in needed for 2.0m

Comment 20

[Kai-Zhen Li [:seinlin][:kli]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/08ebfa341006

Comment 21

[Kai-Zhen Li [:seinlin][:kli]]

Backouted on 2.0m: https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/96d4f5da6196

Aknow, Could you help to update the patch? Thanks!

Comment 22

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: (2.0m) [final#2] Support change barring password. r=hsinyi

Comment 23

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: (2.1) [final#2] Support change barring password. r=hsinyi

Comment 24

[Szu-Yu Chen [:aknow] (inactive)]

(In reply to Kai-Zhen Li [:seinlin] from comment #21)
> Backouted on 2.0m:
> https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/96d4f5da6196
> 
> Aknow, Could you help to update the patch? Thanks!

Updated. Sorry for the error.

Comment 25

[Kai-Zhen Li [:seinlin][:kli]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/9804735ff433

Comment 26

[Kai-Zhen Li [:seinlin][:kli]]

Comment on attachment 8542028 [details] [diff] [review]
(2.1) [final#2] Support change barring password. r=hsinyi

As per comment 16.

Comment 27

[bhavana bajaj [:bajaj]]

Comment on attachment 8542028 [details] [diff] [review]
(2.1) [final#2] Support change barring password. r=hsinyi

We cannot be adding new features so late in the game, so minusing for 2.1. This will ride the trains and partner can have it in 2.2.