853798 - [Buri][Dialer]The screen display abnormal after using USSD

Status: VERIFIED FIXED

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

Description

[sync-1]

Firefox os  v1.0.1
 Mozilla build ID: 20130310070203.
 
 +++ This bug was initially created as a clone of Bug #427992 +++
 
 DEFECT DESCRIPTION:
 (1)Launch Dialer function, press *#06# and dial key
 (2)It will display the IMEI string
 (3)Press home key to return to homescreen
 (4)Launch Dialer again, and you will see the problem, it will display the dialer keypad screen firstly, then go to IMEI string screen, then back to keypad, then go to IMEI string and stay there  --- NOK
 
 REPRODUCING PROCEDURES:
 
 EXPECTED BEHAVIOUR:
 It should go to the USSD screen directely, Or just stay in dialer keypad screen.
 No strang switching forth and back.
 
 ASSOCIATE SPECIFICATION:
 
 TEST PLAN REFERENCE:
 
 TOOLS AND PLATFORMS USED:
 
 USER IMPACT:
 Medium, but Dialer is an important function.
 
 REPRODUCING RATE:
 100%
 
 For FT PR, Please list reference mobile's behavior:
 
 ++++++++++ end of initial bug #427992 description ++++++++++

Comment 1

[Germán Toro del Valle (:gtorodelvalle)]

I would say this bug is a duplicate of bug 831807 but since it is this one the one which has the tef+, I would mark the previous one as duplicate of this one.

Amelie, do you agree with the "duplication"? :-) Thanks!

Comment 3

[Joe Cheng [:jcheng] (please needinfo)]

etienne, can you take this? thanks

Comment 4

[Etienne Segonzac (:etienne)]

As I originally said in bug 831807, the only way to fix this more is to stop using a attention screen for the USSD.

This is non-trivial (basically re-doing the whole USSD UI) and will be a pretty big patch, but I'm happy to work on it if we get the various green lights.

Comment 5

[Kai-Chih Hu [:khu]]

Thanks, Etienne. 
So, let's assign you as the owner, if you don't mind. If you're not working on it, please tell us and we can try to see if there has anyone who could help this case. Thanks!

Comment 6

[Anthony Ricaud (:rik)]

I'll take this.

Comment 7

[Amelie Kong]

I agree. :)

(In reply to gtorodelvalle from comment #1)
> I would say this bug is a duplicate of bug 831807 but since it is this one
> the one which has the tef+, I would mark the previous one as duplicate of
> this one.
> 
> Amelie, do you agree with the "duplication"? :-) Thanks!

Comment 8

[Anthony Ricaud (:rik)]

I have a working refactoring for displaying the IMEI.

Unfortunately, my mobile carrier doesn't have USSD services so I need to do some testing with another provider this weekend.

Comment 9

[Fernando Jiménez Moreno [:ferjm]]

(In reply to Etienne Segonzac (:etienne) from comment #4)
> As I originally said in bug 831807, the only way to fix this more is to stop
> using a attention screen for the USSD.
> 
> This is non-trivial (basically re-doing the whole USSD UI) and will be a
> pretty big patch, but I'm happy to work on it if we get the various green
> lights.

\o/

(In reply to Anthony Ricaud (:rik) from comment #8)
> I have a working refactoring for displaying the IMEI.
> 
> Unfortunately, my mobile carrier doesn't have USSD services so I need to do
> some testing with another provider this weekend.

Let me know if you need any help with that. I am familiar with MMI/USSD services.

Comment 10

[David Palomino [:dpalomino]]

linking for cert meta bug

Comment 12

[Anthony Ricaud (:rik)]

I'm not done yet, a few existing unit tests are still failing.

Here's my progress so far: https://github.com/rik/gaia/tree/ussd-refactoring-853798

Comment 13

[Juan Perez-Bedmar [:juanpbf]]

Adding some colleagues in CC

Comment 14

[Anthony Ricaud (:rik)]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9062

Pointer to Github pull-request

Comment 15

[Germán Toro del Valle (:gtorodelvalle)]

#loveit Couldn't resist to try it ;-)

Comment 16

[Etienne Segonzac (:etienne)]

Comment on attachment 735133 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9062

Comments on github, mainly naming stuffs.

Let's do a joint review with :ferjm for the next version of the patch so that we can land this safely.

Comment 17

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 735133 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9062

Thanks a lot Anthony! This looks really great! We needed this badly.

Just a couple of comments:

1. I am afraid that we are breaking the current concept of network initiated USSD. We are still handling the "ussd-received" system message, but the user is not getting any notification about it.

2. I've noticed that sometimes, the close button does not work. It is hard to reproduce and I am afraid the I couldn't find clear STRs, but I  am able to reproduce it playing with the card view or the home button, jumping between the dialer and other apps while the USSD screen is shown (after receiving replies for an interactive USSD app). I also reproduced it locking the screen or just waiting a few minutes while the USSD view is being shown. I've noticed that each time that I reproduced this issue the following log is shown:

"E/GeckoConsole( 8774): [JavaScript Warning: "HTTP "Content-Type" of "text/html" is not supported. Load of media resource app://communications.gaiamobile.org/dialer/index.html failed." {file: "app://communications.gaiamobile.org/dialer/index.html#keyboard-view" line: 0}]"

Sorry for the lack of clear STR, but it is pretty random.

Comment 18

[Anthony Ricaud (:rik)]

Thanks Fernando.

For 1., would you have a good way to test that?

For 2., I think we've have reliable STR:
1) "Call" a service that lets you respond (#123# on Orange France for example)
2) Let the MMI screen open for a long time till it changes to a "Session expired"
3) Boom.

Comment 19

[Fernando Jiménez Moreno [:ferjm]]

(In reply to Anthony Ricaud (:rik) from comment #18)
> Thanks Fernando.
> 
> For 1., would you have a good way to test that?
> 

Yes. Take a look at the test app attached to bug 820295

Comment 20

[David Palomino [:dpalomino]]

Hi, 

This is reported by certification in Spain, LatAm.

Comment 21

[Anthony Ricaud (:rik)]

Comment on attachment 735133 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9062

And this version should fix the two issues from comment 17.

Comment 22

[Etienne Segonzac (:etienne)]

Comment on attachment 735133 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9062

r=me with the small comment on github addressed

Comment 23

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 735133 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/9062

Awesome work Anthony! Thanks a lot! It looks great!

Comment 24

[Anthony Ricaud (:rik)]

Annnnnnd landed!
https://github.com/mozilla-b2g/gaia/commit/2abd23279885ee8e4e0e5268e01b877433c9135d

Comment 25

[John Ford [:jhford] CET/CEST Berlin Time]

I was not able to uplift this bug to v1-train and v1.0.1.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1-train and v1.0.1, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1-train
  git cherry-pick -x  2abd23279885ee8e4e0e5268e01b877433c9135d
  <RESOLVE MERGE CONFLICTS>
  git commit
  git checkout v1.0.1
  git cherry-pick -x $(git log -n1 v1-train)

Comment 26

[Germán Toro del Valle (:gtorodelvalle)]

Requesting info from Anthony ;-)

Comment 27

[Anthony Ricaud (:rik)]

Uplifted on v1-train:
https://github.com/mozilla-b2g/gaia/commit/f8e8f2be6803b02d6a083b2a144a80e7ebf15951

Comment 28

[Anthony Ricaud (:rik)]

For v1.0.1, I've found at least all those commits that are on v1-train but not on v1.0.1. I need to go through them to see what we can do about it.

afb2e21c5f00cdac589e897976b0bfa1c6ab1a13 - bug 846261
5b9aafa380c14d7176b40323d152327d544caf3f - bug 850170
90eb5a3e3228e7a05777c5dfe526de044b530e72 - bug 839363
f2133bee28657b8ec435d14d820a3684be491a10 - bug 857944
8d4b299144f61d52aa3fc51b11d366beb38b326a - bug 859719
ff1272361763a49a05620ccb63d36f57f043363c - bug 852515
e26e180eb85980bf8af7f289f3bca6c42e312a05 - bug 857795
ce63614fb3271561c09c15f5ee203622baa0ea2e - bug 859879

Comment 29

[Daniel Coloma:dcoloma]

(In reply to Anthony Ricaud (:rik) from comment #28)

Hey!

I think most of those bugs are already uplifted to v1.0.1, except:


> 90eb5a3e3228e7a05777c5dfe526de044b530e72 - bug 839363
> ff1272361763a49a05620ccb63d36f57f043363c - bug 852515
> e26e180eb85980bf8af7f289f3bca6c42e312a05 - bug 857795


Can you please double check and see how difficult would be fixing the patch without uplifting those?

Comment 30

[Anthony Ricaud (:rik)]

I've fixed the merge issues and I'm not sure why those bugs showed up in my git log cause they were actually not a big issue.

But now, we have one issue with the transitions. v1.0.1 doesn't have the transitions. It's part of commit e92d92de1839bd905f710da92cf2260725051837 from bug 816941. Should we uplift this?

Comment 31

[Daniel Coloma:dcoloma]

how risky is uplifting bug 816941 to v1.0.1 is?

Comment 32

[Anthony Ricaud (:rik)]

Let's ask those who wrote the code ;)

Comment 33

[Viga]

I'm from TCL, and wait for uplifting this change to test other related bugs.
Who can tell me when this uplift been done? 
Related bugs 856534 861754.

Comment 34

[Germán Toro del Valle (:gtorodelvalle)]

Having a look at the uplift issue right now ;-)

Comment 35

[Germán Toro del Valle (:gtorodelvalle)]

Hi guys! Apart from other issues that popped up ;-999 I think I have the transition related bugs uplifted to v1.0.1. There were 2 bugs affecting this: bug 816941 and bug 847866.

You can find this uplifting in my personal repository at https://github.com/gtorodelvalle/gaia/tree/v1.0.1-uplifting-bug-816941-and-bug-847866-revolutions (yeah, that was my 3rd uplifting attempt ;-999 )

As you will see, this uplifting is composed of 3 commits:
  1. https://github.com/gtorodelvalle/gaia/commit/b708ab1e69f11f929fa207be418b731ea6f0da1c
  2. https://github.com/gtorodelvalle/gaia/commit/b0b147534abdd07333e90d7aef549dd27ddb6375
  3. https://github.com/gtorodelvalle/gaia/commit/59b3516ec83cae3d06c2f33a7ed5d76b90831762

I guess you guys would want to try it before I uplift them to v1.0.1?

Comment 36

[Anthony Ricaud (:rik)]

German: Thanks for working on this uplift. I believe it is too risky to uplift all of this, especially considering the low visual difference.

So I'll prepare a small modification to my uplift to not rely on transitions.

Comment 37

[Anthony Ricaud (:rik)]

Uplifted to v1.0.1 https://github.com/mozilla-b2g/gaia/commit/8157da8614ff7f3d427f533a392bcb26a54242dd

Comment 38

[Germán Toro del Valle (:gtorodelvalle)]

I think it is the right decision ;-) Thanks!

Comment 39

[John Hammink]

According to khepera_43286, verified fixed on both 4/26 and 5/13 partner builds.