Closed Bug 1119024 Opened 9 years ago Closed 9 years ago

[Settins][SIM Managment] SIM preference for Outgoing Calls / Messages resets to SIM 1 after setting them to 'Always Ask', closing the Settings app and then returning to Settings > SIM Manager.

Categories

(Firefox OS Graveyard :: Gaia::Settings, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.2+, b2g-v2.1 unaffected, b2g-v2.2 verified, b2g-master verified)

VERIFIED FIXED
2.2 S4 (23jan)
blocking-b2g 2.2+
Tracking Status
b2g-v2.1 --- unaffected
b2g-v2.2 --- verified
b2g-master --- verified

People

(Reporter: jmitchell, Assigned: arthurcc)

References

Details

(Keywords: regression, Whiteboard: [2.2-Daily-Testing])

Attachments

(2 files)

Description:
In Settings > SIM Manager you can set the preference for outgoing calls and outgoing messages to SIM 1, SIM 2, or Always Ask. When you set them to Always Ask, and close the settings app, they will appear as 'SIM1' when you return to the SIM Manager. 

Notes: This bug is for the appearance of the selection only - the actual functionality maintains the 'Always Ask' preference even though it appears to be set to SIM 1. However, this does prevent the user from returning the preference setting to SIM 1 as it 'thinks it is already there' and will continue to stay on 'always ask' functionality. 

Prerequisite: have 2 SIMS

Repro Steps:
1) Update a Flame to 20150107010216
2) Go to Settings > Sim Manager and set Outgoing Call / Message preferences to 'Always Ask'
3) Close the Settings app (Hit home, long press home, and swipe settings card off the screen)
4) Return to Settings > Sim Manager

Actual:
Outgoing Call / Message preferences list 'SIM 1' as the selection

Expected:
Preference will accurately reflect the 'Always Ask' option previously selected

Environmental Variables:
Device: Flame Master (Flame KK - Nightly - Full Flashed)
Build ID: 20150107010216
Gaia: 69ac77cfa938fae2763ac426a80ca6e5feb6ad25
Gecko: 33781a3a5201
Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76
Version: 37.0a1 (Master)
Firmware Version: V18D
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Repro frequency: 5/5
See attached: logcat

--------------------------------------------------------------------------------

This issue also occurs on 2.2 with the 188-1 Base

Device: Flame Master (V188-1 Flame KK - Nightly - Full Flashed)
Build ID: 20150105010205
Gaia: c2bf20d23851d5fda9f8f0ef0267db5f49152376
Gecko: 636498d041b5
Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76
Version: 37.0a1 (Master)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

----------------------------------------------------------------------

This issue does NOT repro on 2.1

Device: Flame 2.1 (Flame KK - Nightly - Full Flashed)
Build ID: 20150105001204
Gaia: 73be51f998031f06db0cd660c0e388fa621c9f4c
Gecko: 05dd053f1d90
Gonk: a814b2e2dfdda7140cb3a357617dc4fbb1435e76
Version: 34.0 (2.1)
Firmware Version: V18D
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
Flags: needinfo?(pbylenga)
Functional regression causing bad UX.

Requesting a window.
blocking-b2g: --- → 2.2?
Flags: needinfo?(pbylenga)
QA Contact: jmercado
Triage: regression, blocking
blocking-b2g: 2.2? → 2.2+
This is a gaia issue but I am blocked from getting a smaller window by two bugs which I didn't find in the database, but likely were written up.  

The first was an issue that prevented me from opening the SIM Manager settings page (the button was unresponsive.  

The second was that I could not confirm the functionality of the SIM choice because changing the default SIM to Ask prevented both the Dialer and Messages apps from sending calls out, even after setting the setting back.  When encountering the second bug, the text on the buttons did indeed revert and that may have been reproing the issue, but without being able to confirm that the setting had successfully changed I can't make that call.

Central Regression Window:

Last Working 
Environmental Variables:
Device: Flame 2.2
BuildID: 20141104041844
Gaia: 3c50520982560ccba301474d1ac43706138fc851
Gecko: cadcd47db610
Gonk: Could not pull gonk.  Did you shallow Flash?
Version: 36.0a1 (2.2) 
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

First Broken 
Environmental Variables:
Device: Flame 2.2
BuildID: 20141106122022
Gaia: 1b974ce130eed3988ff5d012c7bd8431c4aba93b
Gecko: 678dd5860cce
Gonk: Could not pull gonk.  Did you shallow Flash?
Version: 36.0a1 (2.2) 
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Last Working gaia / First Broken gecko - Issue does NOT occur
Gaia: 3c50520982560ccba301474d1ac43706138fc851
Gecko: 678dd5860cce

First Broken gaia / Last Working gecko - Issue DOES occur
Gaia: 1b974ce130eed3988ff5d012c7bd8431c4aba93b
Gecko: cadcd47db610

Gaia Pushlog: https://github.com/mozilla-b2g/gaia/compare/3c50520982560ccba301474d1ac43706138fc851...1b974ce130eed3988ff5d012c7bd8431c4aba93b
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Assignee: nobody → arthur.chen
Status: NEW → ASSIGNED
This was regressed by bug 1090097 where we changed to use strict comparison.

EJ, could you help review this simple patch based on what we've discussed? Thanks.
Attachment #8551053 - Flags: review?(ejchen)
Comment on attachment 8551053 [details]
link to https://github.com/mozilla-b2g/gaia/pull/27479

Arthur, I just quickly scanned all possible places that need to be updated ! Please check my comments on GitHub :)

BTW, for those `===` parts, you can decide whether to update it or not.

Thanks :)
Attachment #8551053 - Flags: review?(ejchen)
Comment on attachment 8551053 [details]
link to https://github.com/mozilla-b2g/gaia/pull/27479

Patch updated, please help review it, thanks!
Attachment #8551053 - Flags: review?(ejchen)
Attachment #8551053 - Flags: review?(drs.bugzilla)
Comment on attachment 8551053 [details]
link to https://github.com/mozilla-b2g/gaia/pull/27479

This would break OTA updates, and I'm also not sure if we can assume that we get integers from the mozSettings API. I think we will have to take a different approach.

I don't know much about the Settings app, but since this was caused by a refactor, I think we should go back and figure out what exactly broke this.
Attachment #8551053 - Flags: review?(drs.bugzilla) → review-
Hi @Doug, why this patch would break OTA updates ? If you check https://github.com/crh0716/gaia/blob/1119024/shared/js/sim_settings_helper.js#L81 here, you would notice that we already casted values to `Number` before storing values into mozSettings. So, I can't understand which part would break OTA (?)

Back to this bug, https://github.com/crh0716/gaia/blob/1119024/apps/settings/js/panels/simcard_manager/simcard_manager.js#L406 is the root cause that makes this bug happen. Based on L40, you can see that we use `Number` type to compare (===) with `String` type and this makes the UI broken.

After discussed with Arthur offline, if we are going to store all related values as `Number`, we should fix all the other places to `Number` type to make them more consistent and you can feel free to use `===` or `==` if you want.

Hope this clarifies something and if there is anything missing (or wrong), please kindly let me know !

THanks :)
Flags: needinfo?(drs.bugzilla)
(In reply to EJ Chen [:eragonj][:小龍哥][ni? if you need me] from comment #8)
> Hi @Doug, why this patch would break OTA updates ? If you check
> https://github.com/crh0716/gaia/blob/1119024/shared/js/sim_settings_helper.
> js#L81 here, you would notice that we already casted values to `Number`
> before storing values into mozSettings. So, I can't understand which part
> would break OTA (?)

I haven't tested this, but I don't think that the cast here prevents us from loading a string from one of the default service ID settings, on a profile that existed prior to this patch being applied. From my reading of this code, it looks like it only prevents us from saving a string from that point onwards. Could you clarify that?
Flags: needinfo?(drs.bugzilla)
Comment on attachment 8551053 [details]
link to https://github.com/mozilla-b2g/gaia/pull/27479

Okay, nevermind, I see. This cast has existed for a long time.
Attachment #8551053 - Flags: review- → review+
Well, if you are talking a profile that existed prior to this patch being applied, I have to say that they should already be `Number` type because related patch was landed https://github.com/mozilla-b2g/gaia/commit/47b70c71b87c7b7328a69a80170c5dc7c7e753c9 on v1.4.

No matter how, we would try OTA to make sure these changes work.

Thanks !!
(In reply to Doug Sherk (:drs) (use needinfo?) from comment #10)
> Comment on attachment 8551053 [details]
> link to https://github.com/mozilla-b2g/gaia/pull/27479
> 
> Okay, nevermind, I see. This cast has existed for a long time.

It seems that we are typing at the same time xDDD !

And yes, this cast was landed since v1.4 ! Thanks Doug :)
Thanks all!

master: 966b3a7a13a7f0d5b86cbc9e64cb78d43ec7dba8
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment on attachment 8551053 [details]
link to https://github.com/mozilla-b2g/gaia/pull/27479

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): bug 1090097
[User impact] if declined: Users are not able to set the primary sim to "always ask"
[Testing completed]: Manual tests were completed
[Risk to taking this patch] (and alternatives if risky): Low - only fix the type of the variable and type casting.
[String changes made]: N/A
Attachment #8551053 - Flags: approval-gaia-v2.2?
Attachment #8551053 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
This issue is verified fixed on the latest Flame 3.0 build.  

Actual Results:
The drop downs continue to show "Always ask" when viewed after reopening settings

Environmental Variables:
Device: Flame 3.0
BuildID: 20150123010227
Gaia: cba2f0bf49b882e0044c3cc583de8fcf83d2ffa4
Gecko: 494632b9afed
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Leaving 2.2 Flame verfication for Monday, as the latest available build is before Ryan's uplift this morning.
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage-]
Flags: needinfo?(ktucker) → needinfo?(jmercado)
This issue is verified fixed for Flame 2.2 as well.

Actual Results:
The drop down continue to show "Always ask" when viewed after reopening settings.

Environmental Variables:
Device: Flame 2.2
BuildID: 20150127002504
Gaia: 80d5b797fd0497a7e3337b7798a21b2e1219681a
Gecko: 01bf1516a65b
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0a2 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
Flags: needinfo?(jmercado) → needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage-] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: