1116431 - [Messages] SIM identifier is displayed inside messages and notifications even if only one SIM inserted

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Oleg Zasypkin [:azasypkin]]

STR:

* Have dual-sim device with only one SIM inserted.

* Send message to any number and once message is sent observe message bubble - SIM identifier is displayed inside message bubble;

OR

* Receive any message and observe notification - notification contains SIM identifier.

Tested on Flame and the latest PVT central build. 

It's not reproduced on week-old PVT build (eg. 2014-12-23-01-02-02).

Comment 1

[Oleg Zasypkin [:azasypkin]]

Hey Hsin-Yi,

Could you please help us to understand the following issue?

To decide if need to display SIM information in SMS app we iterate through "navigator.mozMobileConnections", and check "iccId" property of every connection and if it's not null then we consider it as an active connection/SIM.

Previously if we had only one SIM inserted then "iccId" for another connection was equal to null (eg. in week-old build 2014-12-23-01-02-02), on the most recent builds "iccId" for such connection is empty string.

So should we change our logic to filter out connections with empty and null iccId or Gecko should return to null value for non-active connection?

Thanks!

Comment 2

[Julien Wajsberg [:julienw]]

Also note that other gaia apps use navigator.mozIccManager, I don't know how it behaves now.

Comment 3

[Oleg Zasypkin [:azasypkin]]

(In reply to Julien Wajsberg [:julienw] (PTO 12/25 -> 12/29) from comment #2)
> Also note that other gaia apps use navigator.mozIccManager, I don't know how
> it behaves now.

Yeah, thanks for the note, mozIccManager works correctly now - "mozIccManager.iccIds" contains only one entry. Maybe it's more appropriate for our use case?

Anyway it'd be better to know what happened to mozMobileConnection.iccId :)

Comment 4

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

(In reply to Oleg Zasypkin [:azasypkin] from comment #3)
> (In reply to Julien Wajsberg [:julienw] (PTO 12/25 -> 12/29) from comment #2)
> > Also note that other gaia apps use navigator.mozIccManager, I don't know how
> > it behaves now.
> 
> Yeah, thanks for the note, mozIccManager works correctly now -
> "mozIccManager.iccIds" contains only one entry. Maybe it's more appropriate
> for our use case?
> 
> Anyway it'd be better to know what happened to mozMobileConnection.iccId :)

Hi Oleg,
mozMobileConnection.iccId should be "null" instead of an empty string in this case. I seem know what's going on there. But let me investigate more and confirm. Will keep you updated!

Comment 5

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

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #4)
> (In reply to Oleg Zasypkin [:azasypkin] from comment #3)
> > (In reply to Julien Wajsberg [:julienw] (PTO 12/25 -> 12/29) from comment #2)
> > > Also note that other gaia apps use navigator.mozIccManager, I don't know how
> > > it behaves now.
> > 
> > Yeah, thanks for the note, mozIccManager works correctly now -
> > "mozIccManager.iccIds" contains only one entry. Maybe it's more appropriate
> > for our use case?
> > 
> > Anyway it'd be better to know what happened to mozMobileConnection.iccId :)
> 
> Hi Oleg,
> mozMobileConnection.iccId should be "null" instead of an empty string in
> this case. I seem know what's going on there. But let me investigate more
> and confirm. Will keep you updated!

It's caused by Bug 1087847 that accidentally set 'empty string' as default.

Comment 6

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

Attachment: 1116431.patch

Comment 7

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

Comment on attachment 8542860 [details] [diff] [review]
1116431.patch

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

Hi Edgar,

Bug 1087847 accidentally set 'empty string' as default of mIccId that changes the API behaviour, because per design, mIccId should be null, not empty string, when there's no sim.
We don't have problems if sim card becomes undetected during device run-time, because our code will |.SetIsVoid(true)| in this case. So the fix is easy that we simply change default value to null.

Mind taking a look? Thank you!

Comment 8

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Comment on attachment 8542860 [details] [diff] [review]
1116431.patch

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

Good catch!!! Thank you.

Comment 9

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

https://tbpl.mozilla.org/?tree=Try&rev=9b81fb77240f try green! going to land this.

Comment 10

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

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

Comment 11

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

triage: this is a regression causing unexpected user flow. it shouldn't display the identifier under single-sim case.

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bed9ea907821