Closed Bug 1057455 Opened 10 years ago Closed 10 years ago

[Dialer] [MMI] session expired screen for code *3282#, yet SMS still is delivered

Categories

(Firefox OS Graveyard :: RIL, defect, P1)

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:2.0M+, firefox33 wontfix, firefox34 wontfix, firefox35 fixed, b2g-v1.3 wontfix, b2g-v1.4 wontfix, b2g-v2.0 wontfix, b2g-v2.0M fixed, b2g-v2.1 fixed, b2g-v2.2 fixed)

RESOLVED FIXED
2.1 S6 (10oct)
blocking-b2g 2.0M+
Tracking Status
firefox33 --- wontfix
firefox34 --- wontfix
firefox35 --- fixed
b2g-v1.3 --- wontfix
b2g-v1.4 --- wontfix
b2g-v2.0 --- wontfix
b2g-v2.0M --- fixed
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed

People

(Reporter: tchung, Assigned: hsinyi)

References

Details

Attachments

(7 files, 2 obsolete files)

A session expired screen appears when entering MMI code *3282# on the AT&T Network, but still successfully returns the correct data via SMS.   The bug here is that we shouldnt be showing any kind of session expired error screen to the user, and perhaps nothing.    

compare to iOS or android, there is no such error that shown that the would confuse the user.

Screenshots and logcat of error and successful SMS message.

Repro:
1) install 2.0 build on Flame.  v123 base image
Gaia      60cedd4aa6ba408174bcd20a7bf774e73f927674
Gecko     https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/45eca5871dd0
BuildID   20140821000201
Version   32.0
ro.build.version.incremental=110
ro.build.date=Fri Jun 27 15:57:58 CST 2014
B1TC00011230


2) launch dialer with AT&T Sim
3) type in *3282# and dial
4) Verify a session expired message appears, but if you want a few seconds, you will successfuly receive the SMS text of your data summary.

Expected:
- dont notify the user the session expired, since it does work

Actual;
- confusing error message displayed.
Attached file logcat
Qawanted, can we check if this is a regression from 1.3?   please check 1.3 flame, as well as 1.3 open C.  Thanks!
Flags: needinfo?(jmitchell)
Keywords: qawanted
Issue DOES occur on Flame 1.4

Actual Results: A "Session expired" message is seen and then the data summary SMS is received.

Device: Flame 1.4
Build ID: 20140821084113
Gaia: f63cdae1a06c808443ed14de3cd13b61311426e0
Gecko: 1cacad481f49
Version: 30.0 (1.4)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0
------------------------------------------------------------------------------------------------------------

Issue does NOT occur on Flame 1.3, Open-C

Actual Result: A message, "Thank you, your request is being processed. A message will be sent to your phone." is received and then you receive a data summary SMS.

Device: Flame 1.3
Build ID: 20140627162151
Gaia: 5c43d012a9351e8aaeacda0b87b0886b7f67d33d
Gecko: e181a36ebafaa24e5390db9f597313406edfc794
Version: 28.0 (1.3)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:28.0) Gecko/28.0 Firefox/28.0

Device: Open_C 1.3
Build ID: 20140527120838
Gaia: Unknown Git commit; build date shown here.
Gecko: Unknown
Version: 28.0 (1.3)
Firmware Version: P821A10V1.0.0B06_LOG_DL
User Agent: Mozilla/5.0 (Mobile; rv:28.0) Gecko/28.0 Firefox/28.0
---------------------------------------------------------------------------------------------------------

Regression from 1.3 to 1.4

A Regression-window will not be possible due to the age of this issue - it occurs in the oldest build we have access to.

Device: Flame Master
Build ID: 20140417000006
Gaia: 7591e9dc782ac2e97d63a96f9deb71c7b3588328
Gecko: e71ed4135461
Version: 31.0a1 (Master)
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell) → needinfo?(pbylenga)
Keywords: qawantedregression
That looks like the 1.3 was a base image check which means you're also testing COM ril vs MOZ ril, can we instead get the earliest gaia/gecko on top of the base image for the 1.4 check?  I know the earliest builds were blocked by a sim card issue.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage-]
Flags: needinfo?(pbylenga) → needinfo?(jmitchell)
Keywords: regressionqawanted
[Blocking Requested - why for this release]:


Thanks Joshua!   nom'inating for 2.1 since i dont feel it should block 2.0, but also reproduces on 2.1.

(In reply to Peter Bylenga [:PBylenga] from comment #5)
> That looks like the 1.3 was a base image check which means you're also
> testing COM ril vs MOZ ril, can we instead get the earliest gaia/gecko on
> top of the base image for the 1.4 check?  I know the earliest builds were
> blocked by a sim card issue.

Yes, having the RIL data on 1.4 might help.  

A regression between 1.3 -> 2.0, tested on same base image for Flame (v123), and 1.3 open C.  

Reproduced also on 2.1, flame v123
Gaia      afcdd36f13e75adcdebe57d842a277fd587faf28
Gecko     https://hg.mozilla.org/mozilla-central/rev/0b9dd32d1e16
BuildID   20140822040202
Version   34.0a1
ro.build.version.incremental=110
ro.build.date=Fri Jun 27 15:57:58 CST 2014
B1TC00011230
blocking-b2g: --- → 2.1?
The earliest build NOT blocked by the sim card issue is 20140513183003 - which DOES reproduce this issue


Device: Flame 1.4
Build ID: 20140513183003
Gaia: b40103dec34a147c9018a1af76eb21c3184f2f93
Gecko: 7788969f70b0
Version: 30.0 (1.4)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0
QA Whiteboard: [QAnalyst-Triage-] → [QAnalyst-Triage?]
Flags: needinfo?(jmitchell) → needinfo?(pbylenga)
Keywords: qawanted
See Also: → 1043817
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(pbylenga)
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][lead-review+]
This looks a lot like bug 1043817 as Hsin-Yi pointed out. We have a RIL log there, can we get one from here too to see if they're the same bug? There's some instructions on how to gather relevant logs in bug 1043817 comment 6 and bug 1043817 comment 8.
(In reply to Gabriele Svelto [:gsvelto] from comment #8)
> This looks a lot like bug 1043817 as Hsin-Yi pointed out. We have a RIL log
> there, can we get one from here too to see if they're the same bug? There's
> some instructions on how to gather relevant logs in bug 1043817 comment 6
> and bug 1043817 comment 8.

qawanted to provide log.  joshua, can you help gather that log when reproducing?
Flags: needinfo?(jmitchell)
Keywords: qawanted
Attached file logcat_20140827_1745.txt (obsolete) —
Flags: needinfo?(jmitchell)
Please Disregard that last post - in my haste to get this done by end-of-day I blew past the details of getting this specific logcat -  NI to myself to re-address this tomorrow
QA Whiteboard: [QAnalyst-Triage+][lead-review+] → [lead-review+]
Flags: needinfo?(jmitchell)
Keywords: qawanted
OK, I've followed all the RIL debugging instructions at https://wiki.mozilla.org/B2G/QA/Tips_And_Tricks#RIL_Debugging as instructed by those referenced comments and am attaching the correct logcat info. Sorry for the noise.
Attachment #8480276 - Attachment is obsolete: true
Flags: needinfo?(jmitchell)
QA Whiteboard: [lead-review+] → [QAnalyst-Triage?][lead-review+]
Flags: needinfo?(pbylenga)
(In reply to Joshua Mitchell [:Joshua_M] from comment #12)
> Created attachment 8480725 [details]
> logcat_20140828_1031.txt
> 
> OK, I've followed all the RIL debugging instructions at
> https://wiki.mozilla.org/B2G/QA/Tips_And_Tricks#RIL_Debugging as instructed
> by those referenced comments and am attaching the correct logcat info. Sorry
> for the noise.

According to the log, this looks like the same as  bug 1043817.
(In reply to Joshua Mitchell [:Joshua_M] from comment #12)
> OK, I've followed all the RIL debugging instructions at
> https://wiki.mozilla.org/B2G/QA/Tips_And_Tricks#RIL_Debugging as instructed
> by those referenced comments and am attaching the correct logcat info. Sorry
> for the noise.

Thanks for your effort, this is appreciated.

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #13)
> According to the log, this looks like the same as  bug 1043817.

That's my feeling too. Since this wasn't happening before shall we try bisecting this issue?
QA Whiteboard: [QAnalyst-Triage?][lead-review+] → [QAnalyst-Triage+][lead-review+]
Flags: needinfo?(pbylenga)
(In reply to Gabriele Svelto [:gsvelto] from comment #14)
> (In reply to Joshua Mitchell [:Joshua_M] from comment #12)
> > OK, I've followed all the RIL debugging instructions at
> > https://wiki.mozilla.org/B2G/QA/Tips_And_Tricks#RIL_Debugging as instructed
> > by those referenced comments and am attaching the correct logcat info. Sorry
> > for the noise.
> 
> Thanks for your effort, this is appreciated.
> 
> (In reply to Hsin-Yi Tsai [:hsinyi] from comment #13)
> > According to the log, this looks like the same as  bug 1043817.
> 
> That's my feeling too. Since this wasn't happening before shall we try
> bisecting this issue?

I cannot reproduce this issue with local carrier. If I keyed in a valid MMI code (I tried many different sets), I saw information displayed as expected. If I keyed in invalid code, I saw "Session expired" which was also as expected IMO as in this case, gecko indeed received a session terminated message from rild.

However, according to the log in comment 12, the behaviour is somehow different in the AT&T operator. Gecko received *two* unsolicited messages from rild: one with balance information and sessionEnded == true and the other with empty message and sessionEnded == true.

Message 1) from rild
08-28 10:31:42.053   290   670 I Gecko   : RIL Worker: [0] Handling parcel as UNSOLICITED_ON_USSD
08-28 10:31:42.053   290   670 I Gecko   : RIL Worker: [0] On USSD. Type Code: 0 Message: Thank you, your request is being processed. A message will be sent to your phone.

Message 2) from rild
08-28 10:31:42.063   290   670 I Gecko   : RIL Worker: [0] Handling parcel as UNSOLICITED_ON_USSD
08-28 10:31:42.063   290   670 I Gecko   : RIL Worker: [0] On USSD. Type Code: 0 Message: null

I'll try to hack gecko code to see if I could grab more hints. But, as the logic of handling |UNSOLICITED_ON_USSD| on gecko hasn't changed since 1.3, wondering if you have any idea about gaia changes on displaying ussd-received messages, especially when device gets consecutive ussd messages, Gabriele?
I hacked gecko code to imitate the case that rild sends two messages to gecko: |one with balance information and sessionEnded == true and the other with empty message and sessionEnded == true.| With the hack, the issue is reproducible even on v1.3. So this doesn't look like a regression but a flaw in mozril since v1.3_gaia+comRIL doesn't look like have this problem. Digging...
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #15)
> I'll try to hack gecko code to see if I could grab more hints. But, as the
> logic of handling |UNSOLICITED_ON_USSD| on gecko hasn't changed since 1.3,
> wondering if you have any idea about gaia changes on displaying
> ussd-received messages, especially when device gets consecutive ussd
> messages, Gabriele?

There were some changes but none that might have affected how we deal with this IIRC. Two unsolicited messages were already handled this way because they were both considered separately (since sessionEnded is true in the first message). Could there be some corner case in the RIL where it might generate the second message accidentally? Is there a way to get you more relevant data of what reaches the RIL so that you can reproduce it locally?
(In reply to Gabriele Svelto [:gsvelto] from comment #17)
> (In reply to Hsin-Yi Tsai [:hsinyi] from comment #15)
> > I'll try to hack gecko code to see if I could grab more hints. But, as the
> > logic of handling |UNSOLICITED_ON_USSD| on gecko hasn't changed since 1.3,
> > wondering if you have any idea about gaia changes on displaying
> > ussd-received messages, especially when device gets consecutive ussd
> > messages, Gabriele?
> 
> There were some changes but none that might have affected how we deal with
> this IIRC. Two unsolicited messages were already handled this way because
> they were both considered separately (since sessionEnded is true in the
> first message). Could there be some corner case in the RIL where it might
> generate the second message accidentally? 

The second message is coming from modem/rild, not generated by ril_worker.

I would see how gecko ril could handle this corner case carefully based on comment 16.
Attached patch WIP - 1.3Splinter Review
Discard network-initiated notification without messages
Component: Gaia::Dialer → RIL
Attachment #8482546 - Attachment description: WIP → WIP - 1.3
Attached patch WIP - m-c (obsolete) — Splinter Review
Discard network-initiated notification without messages
Comment on attachment 8482558 [details] [diff] [review]
WIP - m-c

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

I believe this issue is triggered by different operator behaviour. I hacked code to imitate the behaviour and the WIP seems work fine.  But, as I cannot reproduce this issue locally, may I have your feedback, seeing if this resolves your problem? Thanks.
Attachment #8482558 - Flags: feedback?(tchung)
triage: this is not regression and not defect to new feature. I'm putting this into backlog for now
blocking-b2g: 2.1? → backlog
Comment on attachment 8482558 [details] [diff] [review]
WIP - m-c

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

This looks like it should work for USSD codes in north american SIM's.  do we know if international SIMs might break though? im not familiar with international USSD codes and if they could issue other messages other than notify or requests.
Attachment #8482558 - Flags: feedback?(tchung) → feedback+
(In reply to Tony Chung [:tchung] from comment #25)
> Comment on attachment 8482558 [details] [diff] [review]
> WIP - m-c
> 
> Review of attachment 8482558 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This looks like it should work for USSD codes in north american SIM's.  do
> we know if international SIMs might break though? im not familiar with
> international USSD codes and if they could issue other messages other than
> notify or requests.

Thanks for the feedback, Tony.
The patch isn't only for north American SIMs but a general solution. So even if any other SIM send messages other than notify or requests, then in the circumstance the message shall be dropped as design. This is also the AOSP design.
Comment on attachment 8482558 [details] [diff] [review]
WIP - m-c

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

We don't dispatch "ussdreceived" if we receive "sessionEnded" with empty message while there's no existing _ussdSession.

Aknow, could you help review this?
Attachment #8482558 - Flags: review?(szchen)
Comment on attachment 8482558 [details] [diff] [review]
WIP - m-c

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

I got this from ril.h

"""
The USSD session is assumed to persist if the type code is "1", otherwise    
the current session (if any) is assumed to have terminated.                  
"""

I personally feel that the original code about ussdSession is not correct. Could you help me confirm it?

::: dom/system/gonk/ril_worker.js
@@ +6938,5 @@
> +    // The network may initiate its own USSD request. Ignore everything
> +    // that isn't a Notify or a Request. Also, discard if there is no message
> +    // to present.
> +    this._ussdSession = (typeCode == "0" || typeCode == "1");
> +    notify = this._ussdSession && message;

You don't have to introduce |notify|
Simply  return here if it hits the condition.
Attachment #8482558 - Flags: review?(szchen) → review-
Thanks for the review, Aknow.

(In reply to Szu-Yu Chen [:aknow] from comment #28)
> Comment on attachment 8482558 [details] [diff] [review]
> WIP - m-c
> 
> Review of attachment 8482558 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I got this from ril.h
> 
> """
> The USSD session is assumed to persist if the type code is "1", otherwise    
> the current session (if any) is assumed to have terminated.                  
> """
> 
> I personally feel that the original code about ussdSession is not correct.
> Could you help me confirm it?
> 

Sure!

I think you are right. Neither the original code nor my patch is correct.
When "type code" is not 1, that means session is terminated per both of ril.h definition and AOSP implementation. Revision will come later, thanks!


> ::: dom/system/gonk/ril_worker.js
> @@ +6938,5 @@
> > +    // The network may initiate its own USSD request. Ignore everything
> > +    // that isn't a Notify or a Request. Also, discard if there is no message
> > +    // to present.
> > +    this._ussdSession = (typeCode == "0" || typeCode == "1");
> > +    notify = this._ussdSession && message;
> 
> You don't have to introduce |notify|
> Simply  return here if it hits the condition.
Assignee: nobody → htsai
Attached patch patch (v2)Splinter Review
Comment 29 addressed!
Attachment #8482558 - Attachment is obsolete: true
Attachment #8500226 - Flags: review?(szchen)
Comment on attachment 8500226 [details] [diff] [review]
patch (v2)

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

Thank you.
Attachment #8500226 - Flags: review?(szchen) → review+
https://hg.mozilla.org/mozilla-central/rev/4948c090087f
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S6 (10oct)
I still can reproduce this bug on latest master build (v2.2).
Did I misunderstand anything?

Attach the log and demo video.
- Log: Dialer_Prepaid_SIM_20141020.log
- Video: http://youtu.be/-qRlL0wdmxE

* Build information:
 - Gaia-Rev        1daf2dadcd0d554c733661a4c0be1b82001e9da0
 - Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/33c0181c4a25
 - Build-ID        20141019160202
 - Version         36.0a1

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

Hi, Joshua,

May I have your help?
Could you please verify this patch to see if it works as expected?
Thanks.
Flags: needinfo?(jmitchell)
Attachment #8507704 - Attachment mime type: text/x-log → text/plain
(In reply to William Hsu [:whsu] from comment #35)
> I still can reproduce this bug on latest master build (v2.2).
> Did I misunderstand anything?
> 
> Attach the log and demo video.
> - Log: Dialer_Prepaid_SIM_20141020.log
> - Video: http://youtu.be/-qRlL0wdmxE
> 
> * Build information:
>  - Gaia-Rev        1daf2dadcd0d554c733661a4c0be1b82001e9da0
>  - Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/33c0181c4a25
>  - Build-ID        20141019160202
>  - Version         36.0a1
> 
> --- -- - --- -- - --- -- - --- -- - --- -- - --- -- - --- -- - 
> 
> Hi, Joshua,
> 
> May I have your help?
> Could you please verify this patch to see if it works as expected?
> Thanks.

Hi William,
Were you using a roaming SIM to verify this? Eric and I tested this before, and we cannot reproduce the scenario with a roaming SIM in Taiwan. We need someone in the U.S. help verify it.

Also, according to Dialer_Prepaid_SIM_20141020.log, the device didn't receive any information from the carrier. Therefore, the "session expired" message was displayed as expected in this case.
Thanks Hsin-Yi.

Yes, I were using roaming SIM (Chinaunicom) to verify this patch.
Okay. Let's wait for QAnalyst's result to see if their still can reproduce this problem.

Have a nice day. :D
This issue does NOT repro in the latest 2.2 build using an AT&T Sim

Actual Result - A "Thank you, your request is being processed. A message will be sent to your phone." message is given followed by a text message with billing data shortly after. 

Marking 2.2 tracking flag as fixed

Environmental Variables:
Device: Flame 2.2
Build ID: 20141020055012
Gaia: dc496d04907dd314f9736ff78bab3bd27156f79a
Gecko: f2d7d694aae5
Version: 36.0a1 (2.2)
Firmware Version: v184
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0
Flags: needinfo?(jmitchell)
(In reply to Joshua Mitchell [:Joshua_M] from comment #39)
> This issue does NOT repro in the latest 2.2 build using an AT&T Sim
> 
> Actual Result - A "Thank you, your request is being processed. A message
> will be sent to your phone." message is given followed by a text message
> with billing data shortly after. 
> 
> Marking 2.2 tracking flag as fixed
> 
> Environmental Variables:
> Device: Flame 2.2
> Build ID: 20141020055012
> Gaia: dc496d04907dd314f9736ff78bab3bd27156f79a
> Gecko: f2d7d694aae5
> Version: 36.0a1 (2.2)
> Firmware Version: v184
> User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Thanks Joshua! :D
Hi Kai-Zhen,
Could you please try land this on 2.0M? Thanks!
Flags: needinfo?(kli)
Blocks: Woodduck
blocking-b2g: backlog → 2.0M+
If this is necessary to land on 2.1, please request the approval for 2.1.
Flags: needinfo?(htsai)
Comment on attachment 8500226 [details] [diff] [review]
patch (v2)

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 #): a flaw in fxos implementation since day 1
User impact if declined: Users cannot see certain ussd messages in some carriers
Testing completed: yes, QA verified
Risk to taking this patch (and alternatives if risky): very low, this patch is very simple
String or UUID changes made by this patch: no
Attachment #8500226 - Flags: approval-mozilla-b2g34?
Flags: needinfo?(htsai)
Attachment #8500226 - Flags: approval-mozilla-b2g34? → approval-mozilla-b2g34+
Blocks: Woodduck_P2
Priority: -- → P1
No longer blocks: Woodduck_P2
You need to log in before you can comment on or make changes to this bug.