1203886 - The back button in sms conversation view does not always work

Status: RESOLVED FIXED

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

Description

[Nicolas Silva [:nical]]

I have had this bug a handful of time the last month, but I don't have deterministic steps to reproduce. I think that it always happens as follows (but if I try to reproduce the steps it does not always happen):
 - happily send texts to someone (let's call her Charlie).
 - do other thing (but leave sms in the conversation with Charlie)
 - Tango (obviously someone that is not Charlie) sends you a text, and you see the notification.
 - tap the notification to bring the sms app to the front.

 ---> sms is still in Charlie's conversation view, although it should be Tango's instead. I can send messages to Charlie, etc, but the back button on the top left corner does not bring me back to the conversations list.

This is on the z3 with the default foxfooding channel and all proposed updates applied.

Comment 1

[Julien Wajsberg [:julienw]]

I've seen the issue as well but like you I couldn't pinpoint a specific STR.

This is likely a fallout of bug 1162030. I think we likely don't end properly a navigation -- and if a navigation is not ended, we can't start a new one.

I had a look already but couldn't find anything obvious.
Let's see if QA can find steps around this.

Comment 2

[Martin Shuman [:Marty]]

I am unable to reproduce this issue on the latest Spark Aries 2.5 Master build.
I attempted leaving the Messages app running in the background focused on Thread A, then focusing on homescreen, gallery, browser, in a fullscreen youtube video, and in a packaged app (Cut the Rope) when receiving a message from Contact B. Tapping on the Contact B notification always successfully brought me to view Thread B, with app navigation functioning properly.  I tried to include tests with multiple apps running at once, using memory intensive processes, having initially opened Messages from the Contacts and Dialer app, and having landscape orientation transitions. I also attempted to interrupt the initial messages transition in Thread A by tapping the home button and switching apps quickly.

Environmental Variables:
Device: Aries 2.5
BuildID: 20150911152752
Gaia: 758c75ee087ea3722213ea2c185cca1d952c8a29
Gecko: 12b6c9a7fe539c9da25a5682a6b8f2dac0678bad
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 43.0a1 (2.5)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

Leaving Steps-Wanted for another tester to attempt

Comment 3

[Francisco Jordano [:arcturus] [:francisco]]

Removing nomination as we could not reproduce.

Not closing it to keep it track. If it appears again please renominate for 2.5?

Comment 4

[Francisco Jordano [:arcturus] [:francisco]]

After a conversation with Julien we have decided to block on this.

Apparently is happening to more than 1 user, so let's block, we could reevaluate later on if we cannot repro.

Comment 5

[Pi Wei Cheng [:piwei] (inactive)]

I cannot reproduce this issue. I tried with having 10 conversation threads on RC4 build, ota to dogfood test, tried the original str, and other variations of the str. I was always brought to the correct thread, and back button always worked.

Build after OTA (no repro):
Device: Aries 2.5
BuildID: 20150915152315
Gaia: d2e5c49440bf8410ae747b15c0dd11c54053ef3e
Gecko: 112ed0121136e9e642e34382256ec73d33bb1b86
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 43.0a1 (2.5) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

Comment 6

[Julien Wajsberg [:julienw]]

Just want to say I encountered the issue yesterday. I still have no clue what could trigger it though. But it's frequent enough we want to investigate.

Comment 7

[Sxean Lee-David [:sxean]]

Not able to repro: 
Started initial SMS with phone 1 and replied with Aries latest build (BuildID: 20150917135859)
then started a second SMS with phone 2 and replied after notification on Aries. Continued texting 
between all 3 phones. The Aries Phone kept all messages contained withing proper message windows. 
Not able to Repro per Nicolas Silva steps or otherwise.

Comment 8

[Sxean Lee-David [:sxean]]

Here are the Build Environment Variables for the above comment, also re-posted below: 
Not able to repro: 
Started initial SMS with phone 1 and replied with Aries latest build (BuildID: 20150917135859)
then started a second SMS with phone 2 and replied after notification on Aries. Continued texting 
between all 3 phones. The Aries Phone kept all messages contained withing proper message windows. 
Not able to Repro per Nicolas Silva steps or otherwise.

Environmental Variables:
Device: Aries 2.5
Build ID: 20150917135859
Gaia: aede8622d780ec71f766a3ecccbff74c04aaba4e
Gecko: de0e763b521062d9a9da0ce029792be026886223
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 43.0a1 (2.5)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

Comment 9

[Oleg Zasypkin [:azasypkin]]

Investigating bug 1205988 seems I found more or less stable STR that break our back button:

* Open Inbox view;
* Quickly tap on any conversation as many times as you can;
* Once you enter Conversation view, tap on back button.

Expected result: user should go back to Inbox;
Actual result: back button doesn't work

Repro rate: 5/5

Julien, could it happen that you tapped conversation several times?

I think it may happen when we have a delay in Inbox->Conversation navigation (conversation with a _big_ number of messages or Inbox view with a big number of conversations or something other that can introduce delay so that user tries to tap once again).

Comment 10

[Oleg Zasypkin [:azasypkin]]

The good thing is that we can guard ourselves in the future and cover the case from comment 9 with automated Marionette JS tests :)

Just checked that integration tests that involve back button fail if we tap on conversation node twice like:

var conversationNode = messagesApp.Inbox.firstConversation;
conversationNode.tap();
conversationNode.tap();

Comment 11

[Julien Wajsberg [:julienw]]

> Julien, could it happen that you tapped conversation several times?

Yeah, definitely possible; as you said, when a user tap on a conversation, we can wait for some time (that the app is initialized) before entering the conversation, and I definitely may have tapped the conversation again.

Good finding Oleg ! With this I'm sure we can fix the issue.

Comment 12

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Let's get branch checks using the steps from comment 9.

Comment 13

[Pi Wei Cheng [:piwei] (inactive)]

I can reproduce this issue using steps at comment 9. Once the bug happens, if I keep my view in this thread (A), and receive an SMS notification from thread B, tapping on notification takes me to thread A again instead of going to thread B. I believe the reporter was already in this state at step 1 of comment 0 STR.

Issue is reproducible on:
Device: Aries 2.5
BuildID: 20150918122511
Gaia: 4f22dfecdc046fe5223ee858dd06c11b75884740
Gecko: 37c7812ce0e6d10c7e7182f12e752832835e1d67
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 43.0a1 (2.5) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

Device: Flame 2.5
BuildID: 20150917030226
Gaia: db6664f0e07e9966283d30cfc7006151fe7103ff
Gecko: e7d613b3bcfe1e865378bfac37de64560d1234ec
Gonk: c4779d6da0f85894b1f78f0351b43f2949e8decd
Version: 43.0a1 (2.5) 
Firmware Version: v18Dv4
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

-------

This issue does NOT occur on Flame 2.2. Following comment 9 STR, back button works as expected.

Device: Flame 2.2
BuildID: 20150917032502
Gaia: 047c85de3fa1633bd0a319e40bffd2d5afcdae1d
Gecko: 1db31b873e29
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 37.0 (2.2) 
Firmware Version: v18Dv4
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Comment 14

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Let's get a window on this issue then.

Comment 15

[Julien Wajsberg [:julienw]]

not needed, pretty sure it's bug 1162030.

Comment 16

[Pi Wei Cheng [:piwei] (inactive)]

1162030 is indeed in my central pushlog.

Comment 17

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Thanks Julien.

Comment 18

[GitHub Autolander Bot]

Attachment: [gaia] steveck-chung:bug-1203886-navigation-bug > mozilla-b2g:master

Comment 19

[Steve Chung [:steveck]]

Comment on attachment 8665304 [details] [review]
[gaia] steveck-chung:bug-1203886-navigation-bug > mozilla-b2g:master

After some investigation, it seems some startNavigation promise was deferred by the navigationTransition that never finished. The reason of the unfinished navigationTransition is because the later toPanel action was executed and was waiting for the hash change to complete the navigation, but the hash the changed while in first call and there's no hash change event fired again.

Originally it should be block in the toPanel because it will check the panel and return. But if the later toPanel call is triggered before the panel actually switched, the later call which is deferred in startNavigation could not know the panel is switched and it shouldn't execute following actions. I add a panel checking in toPanel(the original checking is still in startNavigation because not all the navigation is started from toPanel), but maybe you have another thought about this.

Comment 20

[Julien Wajsberg [:julienw]]

Comment on attachment 8665304 [details] [review]
[gaia] steveck-chung:bug-1203886-navigation-bug > mozilla-b2g:master

Yes, I think it's the easiest way to fix it.

Thanks for the investigaton !

Comment 21

[Steve Chung [:steveck]]

Comment on attachment 8665304 [details] [review]
[gaia] steveck-chung:bug-1203886-navigation-bug > mozilla-b2g:master

Unit test added for requesting toPanel repeatedly. I was try to add integration for it but it didn't work as expected. The second tap action will always slower than panel actually switched even I call the second tap immediately. The latency between the client and service is much longer than I thought :/.

Comment 22

[Oleg Zasypkin [:azasypkin]]

Hmm, yeah, it may happen in slow environment :/

Have you tried test where we tap on notification several times with opened activity (I think we have all in place for this test case and it should not depend on the environment perf characteristics) like mentioned in bug 1205988?

Comment 23

[Julien Wajsberg [:julienw]]

Comment on attachment 8665304 [details] [review]
[gaia] steveck-chung:bug-1203886-navigation-bug > mozilla-b2g:master

r=me

thanks for the fix !

Comment 24

[Oleg Zasypkin [:azasypkin]]

It was completely out of my head and not directly related to this bug, but I wanted to mention that we have other edge cases where fast tapping on buttons can easily break UI, so that user will have to restart the app. E.g.:

1) Open Inbox with at least one conversation;
2) Quickly tap on "New Message" button as many times as you can;

Expected result: user is forwarded to New Message view;

Actual result: user is forwarded to New Message View, but options dialog from Inbox is opened. So user can choose "Select threads" while being in New Message View that will completely break UI.

1) Open Conversation View;
2) Tap on back button and then quickly on Options button;

Expected result: user is either left in Conversation view and sees Conversation options dialog or forwarded to Inbox view;

Actual result: user is forwarded to Inbox view and sees Conversation options dialog. So user can choose "Add subject" that will break UI as well.

Comment 25

[Julien Wajsberg [:julienw]]

We can check if this is fixed with this patch; if not fixed, file separate bugs.

Comment 26

[Oleg Zasypkin [:azasypkin]]

(In reply to Julien Wajsberg [:julienw] (away Oct 1st, 2nd) from comment #25)
> We can check if this is fixed with this patch; if not fixed, file separate
> bugs.

Steve is on holidays, so I've tried to reproduce the issue with this PR and I still see the issues from comment 24. I also see them on v2.2 - so filed bug 1209137 to fix this.

Comment 27

[Steve Chung [:steveck]]

Thanks,

In master: https://github.com/mozilla-b2g/gaia/commit/f345f6a015709beeb2ca3955cab077fcaa959d3b

(In reply to Oleg Zasypkin [:azasypkin][⏰UTC+1] from comment #22)
> Hmm, yeah, it may happen in slow environment :/
> 
> Have you tried test where we tap on notification several times with opened
> activity (I think we have all in place for this test case and it should not
> depend on the environment perf characteristics) like mentioned in bug
> 1205988?

I think it should solve bug 1205988 but I didn't have a proper sim to verify this right now(still not in the office because of typhoon :/). Will check this later.

Comment 28

[Steve Chung [:steveck]]

Hi Oleg, I think bug 1205988 is addressed after this patch merged, will reply in bug 1205988.