Closed Bug 1016885 Opened 6 years ago Closed 6 years ago

[Tarako][Dolphin][LMK][Youtube][USSD] Pop-up USSD message causes Youtube being killed at background due to LMK while using Youtube app

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:1.3T+, b2g-v1.3T fixed, b2g-v1.4 fixed, b2g-v2.0 verified, b2g-v2.1 verified)

RESOLVED FIXED
2.0 S5 (4july)
blocking-b2g 1.3T+
Tracking Status
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- verified
b2g-v2.1 --- verified

People

(Reporter: bli, Assigned: gsvelto)

References

Details

(Whiteboard: [partner-blocker][sprd317445][planned-sprint])

Attachments

(10 files, 2 obsolete files)

500.92 KB, application/zip
Details
16.46 KB, patch
etienne
: review+
Details | Diff | Splinter Review
46 bytes, text/x-github-pull-request
Details | Review
10.76 KB, patch
etienne
: review+
Details | Diff | Splinter Review
46 bytes, text/x-github-pull-request
Details | Review
46 bytes, text/x-github-pull-request
etienne
: review+
Details | Review
46 bytes, text/x-github-pull-request
Details | Review
46 bytes, text/x-github-pull-request
Details | Review
659 bytes, patch
Details | Diff | Splinter Review
2.19 MB, video/3gpp
Details
Attached file Youtube_slog.zip
Steps to reproduce:
-----------------------------------------
1. Install Youtube app from Market Place.
2. Launch Youtube and play a video
3. USSD message pops up while playing the video


Actual result:
-----------------------------------------
YouTube is pushed to background and gets killed due to LMK
Whiteboard: [sprd298584 ]
Whiteboard: [sprd298584 ]
it's hard to view YouTube video using prepaid SIM, since the USSD msg comes actively and reports how much mins / data left to use. nom to 1.3? is a blocker.
blocking-b2g: --- → 1.3?
Whiteboard: [sprd298584 ]
Whiteboard: [sprd298584 ]
I'm preparing a fix for bug 989713 that might help with this too.
blocking-b2g: 1.3? → 1.3T?
(In reply to Marvin Khoo [:Marvin_Khoo] from comment #1)
> it's hard to view YouTube video using prepaid SIM, since the USSD msg comes
> actively and reports how much mins / data left to use. nom to 1.3? is a
> blocker.

I'm assuming you meant to nominate this for Tarako.
blocking-b2g: 1.3T? → 1.3T+
Bing, can you confirm if this is a recent regression or have we had this issue forever on the youtube app ?
Flags: needinfo?(bli)
Thanks Jason, yes, is 1.3T?.. my bad. :(

also, after tapping the X on upper left and close the USSD dialog, the next UI we see is dialer pad. i think we shouldn't show dialer pad unless the USSD command is typed from the pad.

may i know what's the behavior or UI display in US / China if user receives "remaining minutes / data" USSD info when using pre-paid SIM?
(In reply to bhavana bajaj [:bajaj] from comment #4)
> Bing, can you confirm if this is a recent regression or have we had this
> issue forever on the youtube app ?
I'm truly sorry that we are unable to reproduce it at our site, need partner to confirm this.

Hi Marvin and James, could you please help with this?
Flags: needinfo?(mkhoo)
Flags: needinfo?(james.zhang)
Flags: needinfo?(bli)
Whiteboard: [partner-blocker]
Flags: needinfo?(james.zhang)
Whiteboard: [partner-blocker] → [partner-blocker][sprd317445]
Youtube killed by LMK when USSD app popup.
You can also try other app popup case.
per discussion with partner, partner will see if they can workaround this 
ni? James
Flags: needinfo?(james.zhang)
Loop Siiaceon.
Assignee: nobody → siiaceon.cao
Flags: needinfo?(siiaceon.cao)
Flags: needinfo?(james.zhang)
Flags: needinfo?(arvin.zhang)
Component: General → Gaia
attached a flow for better understanding. You may found this issue when using whatever tools to send device an USSD while playback a YouTube content.
Flags: needinfo?(mkhoo)
try to group ussd, and need check if can make ussd not prompt but only one tone(ussd notify unhide while exit video). (Many handset use this solution for such as MMS/SMS case)
loop arvin also.
Flags: needinfo?(siiaceon.cao)
Youtobe need background while ussd coming. So just make USSD group also can make Youtobe crash . So solution is
Need moz cooperator change the design, which need seperate the dialer and USSD. At least cannot launch dailer when recieved notify USSD (ussd which no need dailer)

anyone else solution?
Flags: needinfo?(james.zhang)
Group USSD notify to system app?
Flags: needinfo?(james.zhang)
just make USSD group also can make Youtobe crash except group Youtobe(not a good solution).
need seperate the dialer and USSD by mozilla designed (no need dailer when ussd just notify). 

agree ?
Flags: needinfo?(james.zhang)
Find USSD owner and discuss with him.
Flags: needinfo?(james.zhang)
But I don't think change design is a good idea at this time.
 dailer/USSD owner to aprove, pls
change designed or not launch dailer in this case?
Assignee: siiaceon.cao → nobody
Component: Gaia → Gaia::Dialer
Flags: needinfo?(gsvelto)
I've done some investigation on this and the solution looks more like a design change / window management issue rather than an LMK problem, let me describe why: I've tried running both the YouTube app and the dialer at the same time and both work fine. I can switch between the two w/o either one getting killed and more importantly I can simulate a USSD interaction w/o the YouTube app getting killed. For example this sequence works fine:

- Launch the YouTube app
- Start playing a video
- Go back to the homescreen
- Launch the dialer
- Type an MMI code and send it (such as *#30#)
- Wait for the USSD panel to show up and close it
- Go back to the homescreen
- Go back to the YouTube app

Once we're back in the YouTube app it's still running fine.

Now solving this is not trivial however as two different changes are required:

- We need to close the dialer after the USSD interaction has finished and the user has closed the USSD screen. This shouldn't be really hard if we add some tracking to the app to distinguish between solicited and unsolicited USSD messages.

- We need a way to bring back the YouTube app to the foreground when the dialer is closed. This should behave somewhat similarly to what we do when we close the callscreen and the former background app comes back into the foreground.
Flags: needinfo?(gsvelto)
Thanks Gabriele for your comment 18
What do you think of the risk and estimate of effort on this bug? this will help to evaluate whether this bug should stay as 1.3T+. Thanks
Flags: needinfo?(gsvelto)
Flags: needinfo?(arvin.zhang)
(In reply to Joe Cheng [:jcheng] from comment #19)
> What do you think of the risk and estimate of effort on this bug? this will
> help to evaluate whether this bug should stay as 1.3T+. Thanks

I've studied the code a bit more and the required change is to move the USSD UI into an attention screen and change the related code accordingly. This has both risks and drawbacks:

- The affected code is relatively isolated so the change is unlikely to affect the rest of the application however the change is fairly radical so it might cause breakage the existing MMI/USSD functionality.

- Additionally moving the USSD display into an attention screen changes it's behavior. Currently when we display an USSD message we do it in the context of the dialer app. Tapping the home button when viewing an USSD message will send the dialer to the background and bringing it back will still display the current USSD message. In an attention screen this will work differently: tapping the home button will shrink the attention screen and show a reduced version of it in the top bar and the only way to dismiss it will be to explicitly close it using the close button. Would this be acceptable from a UX POV? What should appear in the minimized attention screen then? What happens if a new USSD message arrives while the attention screen is shrunk? All this probably requires some additional UX.

- Last but not least the upside is that we don't need to move the USSD code anywhere else (like in the system app). It can still live in the dialer and this shouldn't cause problems with a foreground app such as YouTube.
Flags: needinfo?(gsvelto)
Assignee: nobody → anthony
Target Milestone: --- → 2.0 S4 (20june)
Carrie, please review the proposed UX change for USSD. Thank you very much.
Flags: needinfo?(mtsai)
Flags: needinfo?(cawang)
Hi  Gabriele, 

I don't really understand the "Shrunk UI" part. Is it like something from Dialer that if users tap Home while they are in a call and they can tap the bar again to get back to the call screen (in this case, that will be USSD page)?
However, I'd prefer to keep this USSD info in Notification. In this way, users tap Home and they can still access the page whenever they need (by dragging down the notification tray and tapping it to access the page), but if they tap "x" to close the page, it will be cleared from notification tray. Is this doable? Thanks!
Flags: needinfo?(cawang) → needinfo?(gsvelto)
Flags: needinfo?(mtsai)
also Ni? Rik for comment 22 as he took the bug
Flags: needinfo?(anthony)
We may be killing the Youtube app because we request a high-priority wake lock when we receive a MMI. This was introduced to fix bug 999356. I need to try some code to verify this. If I do verify it, I'm not sure how we can fix it though.

For the UI part, moving the code outside of the Dialer app or moving it to an attention screen will introduce a lot of regressions in behaviour. What about this solution: If the dialer is not the front app, we show a notification. In the dialer, we store all the mmi received and when a user taps a notification, we display the ussd window for that ussd. That may help us solve bug 861754 at the same time.
Flags: needinfo?(anthony)
I'd go with the "notification" proposal as what Rik mentioned here.
Our Indian QA sain dolphin also has 30% reproduce rate.
Summary: [Tarako][LMK][Youtube][USSD] Pop-up USSD message causes Youtube being killed at background due to LMK while using Youtube app → [Tarako][Dolphin][LMK][Youtube][USSD] Pop-up USSD message causes Youtube being killed at background due to LMK while using Youtube app
1025457 related issue
I am wondering that is this issue reproducible on other devices as well?
(In reply to Anthony Ricaud (:rik) from comment #24)
> We may be killing the Youtube app because we request a high-priority wake
> lock when we receive a MMI. This was introduced to fix bug 999356. I need to
> try some code to verify this. If I do verify it, I'm not sure how we can fix
> it though.

Some clearing on this: we're not killing the YouTube app, we're just sending it in the background as the USSD display is part of the dialer so once that is closed you'll find yourself in the dialer instead of the YouTube app.

> For the UI part, moving the code outside of the Dialer app or moving it to
> an attention screen will introduce a lot of regressions in behaviour. What
> about this solution: If the dialer is not the front app, we show a
> notification. In the dialer, we store all the mmi received and when a user
> taps a notification, we display the ussd window for that ussd. That may help
> us solve bug 861754 at the same time.

+1

The MMI code is already tracking the state of a single MMI request and expanding it to handle more should be doable w/o too many changes.
Flags: needinfo?(gsvelto)
While working on this, I'm sometimes getting this error:
[JavaScript Error: "requestWindow is undefined" {file: "jar:file:///system/b2g/omni.ja!/components/RILContentHelper.js" line: 2105}]

Hsin-Yi: I'm not yet sure if it's a problem but could you take a look in case we need this fixed for the solution to work?

Here is the patch I use to receive USSD messages while developing. Pressing volume down sends a USSD request.
diff --git a/apps/system/js/hardware_buttons.js b/apps/system/js/hardware_buttons.js
index 9c77db3..c96de57 100644
--- a/apps/system/js/hardware_buttons.js
+++ b/apps/system/js/hardware_buttons.js
@@ -493,6 +493,9 @@
       case 'volume-down-button-release':
         if (this.direction === 'volume-down-button-press') {
           if (!this.repeating) {
+            console.log('XXX: MMI sent');
+            navigator.mozMobileConnections[0].sendMMI('#123#');
+
             this.hardwareButtons.publish('volumedown');
           }
           this.hardwareButtons.setState('base', type);
Flags: needinfo?(htsai)
Flags: needinfo?(janjongboom)
I'm running into bug 1028039 while working on this.
Depends on: 1028039
I am looking at this. Will keep you posted!
Here is my current state: https://github.com/Rik/gaia/tree/youtube-ussd-1.3t-1016885

Although we'd like to land this on master, I had to work on 1.3t because of bug 1028039. The current patch works but it is not ready for review yet. I'll need someone else to continue the work while I am out (unassigning myself to reflect that).
Assignee: anthony → nobody
Thanks for the sharp eye, :rik. File a gecko bug 1028062.
Flags: needinfo?(htsai)
Assignee: nobody → gsvelto
Status: NEW → ASSIGNED
Depends on: 1028062
Target Milestone: 2.0 S4 (20june) → 2.0 S5 (4july)
Bug 1028062 doesn't block this issue IMO. It is caught when Rik is working on this. Bug 1028082 makes firing request error for mobileConnection.cancelMMI().
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #35)
> Bug 1028062 doesn't block this issue IMO. It is caught when Rik is working
> on this. Bug 1028082 makes firing request error for
> mobileConnection.cancelMMI().

Agreed.
Flags: needinfo?(janjongboom)
No longer depends on: 1028062
Whiteboard: [partner-blocker][sprd317445] → [partner-blocker][sprd317445][planned-sprint]
Here's a more complete version of Anthony's patch now that we've got the document.hidden issue fixed. The only major change is that I've converted both the handleMMIReceived() and sendNotification() functions to use promises so that we can properly release the high-priority wakelock when we're finished. I've given this patch only light testing and I'm not 100% happy with it yet so I'm asking for feedback first. BTW it also lacks tests so this is barely above a WIP.
Attachment #8432888 - Attachment is obsolete: true
Attachment #8447745 - Flags: feedback?(etienne)
Hi Gabriele,

Thank you for supporting here, can you suggest a comfortable ETA for this and the risk assessment on this change?
Flags: needinfo?(gsvelto)
(In reply to Wayne Chang [:wchang] from comment #38)
> Thank you for supporting here, can you suggest a comfortable ETA for this
> and the risk assessment on this change?

A comfortable ETA including reviews and a couple of extra iterations would be sometimes later this week like Thursday or Friday at the latest. The risk for this change should be modest as it affects only unsolicited USSD messages making them pop-up notifications when the dialer is not visible. The associated logic received only minor changes and only for the unsollicited case.
Flags: needinfo?(gsvelto)
Comment on attachment 8447745 [details] [diff] [review]
[PATCH] Send a notification instead of displaying the dialer when receiving an unsolicited USSD notification

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

Let me know if my comments are unclear, had a hard time formulating them.

::: apps/communications/dialer/js/dialer.js
@@ +413,5 @@
>            }
>  
>            MmiManager.handleMMIReceived(evt.message, evt.sessionEnded,
> +                                       evt.serviceId)
> +                    .then(releaseWakeLock);

I'm afraid in the cases where we're doing a getSelf.launch(), this might release the wake lock _before_ the visibility change. (since handleMMIReceived resolves outside of the getSelf block)

::: apps/communications/dialer/js/mmi.js
@@ +356,5 @@
> +          };
> +          var notification =
> +            new Notification('USSD', {body: message, icon: iconURL});
> +          notification.addEventListener('click', clickCB);
> +          resolve();

Since we're resolving as soon as the notification is sent (which is a good thing), we're releasing the wake lock and the dialer could get killed.
Should we add support for those notifications in dialer.js/handleNotification? (called via system message when the app gets killed taking the clickCB with it).
Attachment #8447745 - Flags: feedback?(etienne)
(In reply to Etienne Segonzac (:etienne) from comment #40)
> I'm afraid in the cases where we're doing a getSelf.launch(), this might
> release the wake lock _before_ the visibility change. (since
> handleMMIReceived resolves outside of the getSelf block)

Good point. We should release the wakelock immediately only after having posted the notification. I'm splitting that case of handleMMIReceived() to make the code cleaner so I'll use that to handle the unlocking properly too.

> Since we're resolving as soon as the notification is sent (which is a good
> thing), we're releasing the wake lock and the dialer could get killed.
> Should we add support for those notifications in
> dialer.js/handleNotification? (called via system message when the app gets
> killed taking the clickCB with it).

Yes, I forgot about that. That's definitely needed otherwise we might lose some messages and that would be especially likely so on the Tarako.
This is a fully working patch for gaia/master with the issues you had highlighted addressed plus a little test coverage. A few notes:

- I'm passing information using the iconURL field of a notification; the tag would be better but we need to uplift this to the v1.3t branch and we don't have tags there so I didn't want to make my life more complicated than it already is :-p

- On the topic of notifications, the dialer is a bit of a mess in this regard still abusing iconURL to distinguish between the origin of notifications and closing all notifications when one is tapped. I've made it play nice with my changes but we should open a nice follow-up bug to clean up the mess.

- I've had to modify the fake origin of certain mock messages from 'app://communications.gaiamobile.org' to 'http://communications.gaiamobile.org:8080' because the test failed otherwise.
Attachment #8447745 - Attachment is obsolete: true
Attachment #8448690 - Flags: review?(etienne)
Comment on attachment 8448690 [details] [diff] [review]
[PATCH] Send a notification instead of displaying the dialer when receiving an unsolicited USSD notification

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

(In reply to Gabriele Svelto [:gsvelto] from comment #42)
> Created attachment 8448690 [details] [diff] [review]
> [PATCH] Send a notification instead of displaying the dialer when receiving
> an unsolicited USSD notification
> 
> This is a fully working patch for gaia/master with the issues you had
> highlighted addressed plus a little test coverage. A few notes:
> 
> - I'm passing information using the iconURL field of a notification; the tag
> would be better but we need to uplift this to the v1.3t branch and we don't
> have tags there so I didn't want to make my life more complicated than it
> already is :-p

Make sense, let's make sure we have a follow up filed (and referenced in the code).

> 
> - On the topic of notifications, the dialer is a bit of a mess in this
> regard still abusing iconURL to distinguish between the origin of
> notifications and closing all notifications when one is tapped. I've made it
> play nice with my changes but we should open a nice follow-up bug to clean
> up the mess.

The cost of being a pioneer :)

> 
> - I've had to modify the fake origin of certain mock messages from
> 'app://communications.gaiamobile.org' to
> 'http://communications.gaiamobile.org:8080' because the test failed
> otherwise.

Ok. let's keep an eye open to make sure this doesn't fail on some test infrastructure.
Attachment #8448690 - Flags: review?(etienne) → review+
See Also: → 1032784
Blocks: 1033254
Thanks for the quick review!

(In reply to Etienne Segonzac (:etienne) from comment #44)
> Make sense, let's make sure we have a follow up filed (and referenced in the
> code).

I've filed bug 1033254 and referenced it in the code; let's get this code cleaned up ASAP.

> Ok. let's keep an eye open to make sure this doesn't fail on some test
> infrastructure.

My first Travis run contained some changes to the fake app origin but did fail, now I've reverted those changes again and the tests fail locally. I'll be waiting to see what happens on Travis; annoying :-|
This is the Travis run: https://travis-ci.org/mozilla-b2g/gaia/builds/28946591

It's red but the failing tests are completely unrelated so I'll just go ahead and push it.
Pushed to gaia/master ddbf0e11b494fd9665545141485d0b70cab6d514

https://github.com/mozilla-b2g/gaia/commit/ddbf0e11b494fd9665545141485d0b70cab6d514

I'll post the v1.3t patch shortly.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
While testing the v1.3t patch on the Tarako I noticed that a mistake had slipped in my original patch that makes the changes work only when one SIM is used. When multiple SIMs are used localization fails and the notification is not delivered when receiving an unsolicited message so I'm reopening the bug. Fix pending.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Hi Gabriel,

When you have a patch good to go, please also request for 1.4 approval... partner is also encountering the same problem on Dolphin.
This patch initializes the MmiManager before sending a notification, not doing so made notifications fail on multi-SIM devices as the message couldn't be localized properly. The change is conceptually small but the patch is large because I've added test coverage for this specific case. BTW I should really file a follow up to clean up the MMI unit-tests as they're in dire shape (and a bunch of them are disabled to make things worse).
Attachment #8450069 - Flags: review?(etienne)
Comment on attachment 8450069 [details] [diff] [review]
[PATCH] Always initialize the MmiManager before sending a notification

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

Good catch!
Attachment #8450069 - Flags: review?(etienne) → review+
Pushed to gaia/master c664153cd702ae1db12a485e539bae88b9bdcdea

https://github.com/mozilla-b2g/gaia/commit/c664153cd702ae1db12a485e539bae88b9bdcdea

The Travis run was partially red but the failures were unrelated:

https://travis-ci.org/mozilla-b2g/gaia/builds/29041867

I'll be uplifting the patches right away.
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Depends on: 1034279
Uplifting this patch has been an uphill battle against a race in the LazyL10n which I finally managed to fix. I'm asking for review again before landing because the patches had to undergo some important changes, namely:

- New style notifications have been replaced with old style ones, this had been planned ahead and was thus relatively painless.

- The LazyL10n object was suffering from the following race: initialization would go through two asynchronous steps, one to wait for the 'localized' event and one to lazy load further code. If another caller would invoke LazyL10n.get() in between the two calls its callback would be lost as it would never receive the localized event nor would it be able to proceed further. I've modified the code to mark the beginning and end of the initialization procedure and allow only one caller to go through it. Successive callers arriving in the middle of the initialization phase will have their callbacks delayed until the end the object is fully initialized by stacking them into an array.

Besides doing some testing of the scenarios at hand I also tested the other users of LazyL10n to ensure this change didn't break anything but I want to stress that this hasn't been a simple, straightforward uplift and thus we have to proceed with some caution before merging.
Attachment #8450584 - Flags: review?(etienne)
I've updated the PR because the new unit-tests were failing; hopefully they should be fixed now.
Depends on: 1002327
Good news, Travis is green for the uplift too: https://travis-ci.org/mozilla-b2g/gaia/builds/29123015
Comment on attachment 8450584 [details] [review]
[PULL REQUEST] v1.3t uplift

r=me with the test added (see github).

Stupid question but better safe than sorry, we do have Promise support in the gecko we're releasing with 1.3t right?
Attachment #8450584 - Flags: review?(etienne) → review+
(In reply to Etienne Segonzac (:etienne) from comment #57)
> r=me with the test added (see github).

Good point, I'm going to add that.

> Stupid question but better safe than sorry, we do have Promise support in
> the gecko we're releasing with 1.3t right?

Mmm... now that you mention it the documentation says we've got promises enabled by default in Gecko 29 and hidden behind a flag starting with Gecko 25. v1.3t is Gecko 28 so it's a matter of figuring out if they've been enabled or not. I'll double-check, worst case I'll turn that specific instance into a callback (ugly but functional).
I've updated the PR once more:

- Now we use a callback instead of a promise to release the wakelock after having sent the notification, I checked that it's released correctly.

- I've adjusted the tests to cope with the callback.

- I've slightly modified the LazyL10n code: I push the queued up callbacks and then shift them so they're called in order of arrival. Before I popped them out of the array causing them to be called in inverse order of arrival. I think this is cleaner and less "surprising".

- There's an additional test for the LazyL10n code that verifies that all the queued callbacks are invoked properly.

If Travis turns green I'm ready to merge: https://travis-ci.org/mozilla-b2g/gaia/builds/29150394
Please also merge to v1.4. Dolphin has the same issue.
(In reply to James Zhang (Spreadtrum) from comment #60)
> Please also merge to v1.4. Dolphin has the same issue.

I will as soon as I get approval for this and its dependency.
Hi Gabriele,

   Can you also merge this patch to v1.4? We met this issue on dolphin and 1.4 is dolphin branch now.
Gabriele,

Is the 1.3t patch good for 1.4 or does it need rebasing?

Thanks
Flags: needinfo?(gsvelto)
(In reply to Wayne Chang [:wchang] from comment #64)
> Is the 1.3t patch good for 1.4 or does it need rebasing?

Some rebasing is needed, I'm almost done with. Will post the PR for approval soon.
Flags: needinfo?(gsvelto)
Here's a PR for the v1.4 branch. It required some light modifications, mostly to the unit-tests. I'm not asking for approval just yet because I haven't had the time to test it on a device.
Comment on attachment 8452163 [details] [review]
[PULL REQUEST] v1.4 uplift

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): -
[User impact] if declined: Unsolicited USSD messages will bring the dialer application to the front hiding whatever the user was doing in the meantime (e.g. watching a video on YouTube). According to our partners this is quite detrimental in the target markets where unsolicited USSD messages are sent to inform the user of its remaining credit / data cap.
[Testing completed]: Tested on a Flame & covered with unit-tests
[Risk to taking this patch] (and alternatives if risky): USSD/MMI code may be adversely affected. A minor issue that cropped up during testing is bug 1034279.
[String changes made]: None
Attachment #8452163 - Flags: approval-gaia-v1.4?(release-mgmt)
Comment on attachment 8452163 [details] [review]
[PULL REQUEST] v1.4 uplift

Approved for 1.4.
Attachment #8452163 - Flags: approval-gaia-v1.4?(release-mgmt) → approval-gaia-v1.4+
Thanks for the approval, landed attachment 8452163 [details] [review] on gaia/v1.4 b0e9b4bdb39c5eb93a6783a34624ffc84f62b126

https://github.com/mozilla-b2g/gaia/commit/b0e9b4bdb39c5eb93a6783a34624ffc84f62b126

Now let's move on to v2.0.
See comment 67, the patches here are unmodified and can be cherry-picked cleanly from the master branch.
Attachment #8453028 - Flags: approval-gaia-v2.0?
Attachment #8453028 - Flags: approval-gaia-v2.0? → approval-gaia-v2.0?(release-mgmt)
Duplicate of this bug: 1025457
Attachment #8453028 - Flags: approval-gaia-v2.0?(release-mgmt) → approval-gaia-v2.0+
Hi Gabriele,

In v1.4, there's a little problem occurred that it won't show title info(sim index and operator name) on USSD message received. I filed a new bug to track this, let's check it in Bug 1051841.

Thanks.
Dear Gabriele

Our QA in India found a new bug. When he clicked STK menu which meaned sending *121# in settings, there was neither ussd page nor ussd notification shows.
After I checked the log, I found the ussd had been received, but the ussd page was on background.

I think that is because "sessionEnded" flag, After clicking STK menu to send *121#, "sessionEnded" flag in received ussd message is false. So there is no ussd notification.

https://github.com/mozilla-b2g/gaia/blob/v1.4/apps/communications/dialer/js/dialer.js#L299
I think this patch can resolve this issue.
Could you give some advice?
Thanks.
Flags: needinfo?(gsvelto)
(In reply to Wei Gao (Spreadtrum) from comment #75)
> Created attachment 8484789 [details] [diff] [review]
> ussd_sessionEnd.patch
> 
> I think this patch can resolve this issue.
> Could you give some advice?
> Thanks.

Maybe we can also launch('dialer') instead of showing notification in "MmiManager.handleMMIReceived" to resolve this issue.
https://github.com/mozilla-b2g/gaia/blob/v1.4/apps/communications/dialer/js/dialer.js#L307
(In reply to Wei Gao (Spreadtrum) from comment #74)
> Our QA in India found a new bug. When he clicked STK menu which meaned
> sending *121# in settings, there was neither ussd page nor ussd notification
> shows.
> After I checked the log, I found the ussd had been received, but the ussd
> page was on background.
> 
> I think that is because "sessionEnded" flag, After clicking STK menu to send
> *121#, "sessionEnded" flag in received ussd message is false. So there is no
> ussd notification.

Yes, but that was done on purpose to solve the issue in this bug. We need to find a better way. Let's file another bug for this.
Flags: needinfo?(gsvelto)
(In reply to Gabriele Svelto [:gsvelto] from comment #77)
> Yes, but that was done on purpose to solve the issue in this bug. We need to
> find a better way. Let's file another bug for this.

Ok, thanks, I will file a new bug.
Duplicate of this bug: 877084
Attached video verify.3gp
This issue has been successfully verified on Flame 2.1, 2.0
See attachment:verify.3gp
Reproducing rate: 0/5

Flame 2.1 new versions:
Gaia-Rev        dbaf3e31c9ba9c3436e074381744f2971e15c7bf
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/ebce587d2194
Build-ID        20141203001205
Version         34.0

Flame 2.0 new versions:
Gaia-Rev        8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/29222e215db8
Build-ID        20141203000201
Version         32.0
You need to log in before you can comment on or make changes to this bug.