1039955 - [Messages] circular activity issue: "Call" is not functioning when tapping the number on sms which is brought up by sending sms from call log

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

Description

[Eric Chang [:ericcc] [:echang]]

### STR
1. Have a entry in Call log
2. (number not in Contacts)Long tap the entry and choose "Send Message" in option menu.
2. (number in Contacts)Long tap the entry and choose "Send Message" in contact details.
3. Send an SMS.
4. Tap the number or name in the upper banner.
5. Tap Call -> nothing happened
https://www.youtube.com/watch?v=QuPGQ6_SBlU

### Actual
Nothing happened

### Expect
Call made or option not availabe

### Version
Flame central v2.1
Gaia      d29773d2a011825fd77d1c0915a96eb0911417b6
Gecko     https://hg.mozilla.org/mozilla-central/rev/4024d8019701
BuildID   20140716160202
Version   33.0a1
ro.build.version.incremental=109
ro.build.date=Mon Jun 16 16:51:29 CST 2014
B1TC00011220

Flame aurora v2.0
Gaia      aa4f795b81c6147d67c4f06009e166debcf8856e
Gecko     https://hg.mozilla.org/releases/mozilla-aurora/rev/0ec0b9ac39f0
BuildID   20140716160201
Version   32.0a2
ro.build.version.incremental=109
ro.build.date=Mon Jun 16 16:51:29 CST 2014
B1TC00011220

Comment 1

[Salvador de la Puente González [:salva]]

See bug 1051719 comment 3 for clarifications on this bug.

Comment 2

[Salvador de la Puente González [:salva]]

Sorry, I'm opening this again as the `circular activity` issues is not being resolved in bug 1051719 but we could use the same strategy there to solve this problem or another UX workaround.

Indeed, the problem is not related with Dialer app but with the SMS application and we could apply the same strategy to solve this issue or wait until `circular activity` is fixed.

Jenny, could we close the SMS activity when tapping on a number in the same way we did for bug 1051719? Or may be is not the same case?

I'm asking to Steve for confirmation about this being the same problem as described in bug 1051719 too.

Thank you.

Comment 3

[bhavana bajaj [:bajaj]]

BLocking as the STR described are definitely functional regression. Even if this turns out to a DUP we have the DUP blocking as well, so it should be fine here.

Comment 4

[Anthony Ricaud (:rik)]

I'm not sure this is a regression.

Comment 5

[Anthony Ricaud (:rik)]

And moving to SMS per comment 2.

Comment 6

[Jenny Lee]

(In reply to Salvador de la Puente González [:salva] from comment #2)

> Jenny, could we close the SMS activity when tapping on a number in the same
> way we did for bug 1051719? Or may be is not the same case?
> 

I think the two cases are similar. Let's apply the same fix as bug 1051719, close SMS activity and go back to dialer. Thanks!

Comment 7

[Steve Chung [:steveck]]

(In reply to Salvador de la Puente González [:salva] from comment #2)
> Sorry, I'm opening this again as the `circular activity` issues is not being
> resolved in bug 1051719 but we could use the same strategy there to solve
> this problem or another UX workaround.
> 
> Indeed, the problem is not related with Dialer app but with the SMS
> application and we could apply the same strategy to solve this issue or wait
> until `circular activity` is fixed.
> 
> Jenny, could we close the SMS activity when tapping on a number in the same
> way we did for bug 1051719? Or may be is not the same case?
> 
> I'm asking to Steve for confirmation about this being the same problem as
> described in bug 1051719 too.
> 
> Thank you.

Thanks for the clarification, it is circular activity issues indeed. But bug 1051719 only addressed the issue only on browser part. Maybe we should solve it with a general solution.

Comment 8

[Salvador de la Puente González [:salva]]

As per comment 6, let's implement a solution based on bug 1051719. Anyway, we need the bug about circular activity to block both (this and bug 1051719).

Alive, could you point us what is the bug where circular activities are being tracked?

Comment 9

[(inactive after 6/18) Alive Kuo [:alive]]

I think it's https://bugzilla.mozilla.org/show_bug.cgi?id=1033944 though now I have different idea to go.

What we could do is:
if we see the activity callee is just under the bottom of the caller(window A -> inline B -> window A) and the caller is an inline activity, we could close the caller. Not sure if gecko will fail when we remove the caller during activity. (Does postResult/postError still works?) But for more complex circular issues we may only have to kill or block them. For example, window A -> inline B -> window C -> inline D -> window A.

Comment 10

[Steve Chung [:steveck]]

Hi Jenny, could you please confirm the behavior here? Now we could have 2 solutions here:

a) Always leave activity when it need to open a new window no matter it's same to the callee app or not.
 app owners should avoid the possible circular issue by themselves. 
b) Let system app to handle and force to close the activity if it trying to open the callee app. So apps don't need to care about the circular issue.

So which one do you prefer?

Comment 11

[(inactive after 6/18) Alive Kuo [:alive]]

WIP: Kill all front windows if the callee is the same as current active app.
https://github.com/mozilla-b2g/gaia/pull/23362

I don't think we should close activity when it launches a window activity. There might be something to do with the new window and it may pass the result to the activity caller.

Comment 12

[Salvador de la Puente González [:salva]]

This is a 2.0+ blocker so my recommendation is to go further with a UX compliant solution like suggested in comment 6 while trying to provide the ultimate solution in further releases.

What do you think? I'm going to prepare a patch for this and ask for your review, Steve.

Comment 13

[Salvador de la Puente González [:salva]]

Attachment: Leaving the activity after call activity success.

Comment 14

[Jenny Lee]

(In reply to Steve Chung [:steveck] from comment #10)
> Hi Jenny, could you please confirm the behavior here? Now we could have 2
> solutions here:
> 
> a) Always leave activity when it need to open a new window no matter it's
> same to the callee app or not.
>  app owners should avoid the possible circular issue by themselves. 
> b) Let system app to handle and force to close the activity if it trying to
> open the callee app. So apps don't need to care about the circular issue.
> 
> So which one do you prefer?

Hi Steve, let's do solution (b) for now. We will take some time to review other related cases and see if a long term solution is needed. Thanks!

Comment 15

[Steve Chung [:steveck]]

Comment on attachment 8479928 [details] [review]
Leaving the activity after call activity success.

Hey salva, after some discussion with UX, we'll propose to let system to handle circular activity issue in comment 11. Thanks for the quick help but sorry about wasting your time on this...

Comment 16

[(inactive after 6/18) Alive Kuo [:alive]]

Attachment: Fix direct circular activity by killing all foreground windows

Kill direct circular activity (caller.getBottomMostWindow ==== callee) by killing the front windows.

Indirect circular is not covered.

Comment 17

[Salvador de la Puente González [:salva]]

(In reply to Steve Chung [:steveck] from comment #15)
> Comment on attachment 8479928 [details] [review]
> Leaving the activity after call activity success.
> 
> Hey salva, after some discussion with UX, we'll propose to let system to
> handle circular activity issue in comment 11. Thanks for the quick help but
> sorry about wasting your time on this...

Mmm. I think Alive point us with bug 1033944 in charge, precisely of tracking "kill circular activity". Maybe we should fix that bug and mark this as blocked by that.

If not, please reword this to ease tracking and mark the other as a dup.

Comment 18

[Etienne Segonzac (:etienne)]

Comment on attachment 8480390 [details] [review]
Fix direct circular activity by killing all foreground windows

r=me with the test renamed (see github)

Comment 19

[(inactive after 6/18) Alive Kuo [:alive]]

It seems there were several test failing which are unrelated but still re-running.

Comment 20

[(inactive after 6/18) Alive Kuo [:alive]]

https://github.com/mozilla-b2g/gaia/commit/3f59dcdb71aed7147b2b9fc3716a0d4120e22a3d

Comment 21

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/b75c7fdae7d9328297fa6363d59f96170a609245

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Reverted for Gaia unit test failures.
v2.0: https://github.com/mozilla-b2g/gaia/commit/e466549e204f2aed4ae56ebf51e4edd0efbabb65

https://tbpl.mozilla.org/php/getParsedLog.php?id=47032350&tree=Mozilla-B2g32-v2.0

Comment 23

[Etienne Segonzac (:etienne)]

Making a new uplifted patch with the missing mock method

Comment 24

[Etienne Segonzac (:etienne)]

Attachment: Gaia PR againt 2.0

Here's a PR against 2.0, I can't merge it because the tree is closed, but feel free to do it :)

Comment 25

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/449d8db9b3ea1f9262db822c37ef2143477172b7

Comment 26

[Yeojin Chung [:YeojinC]]

This issue is verified fixed on Flame 2.0 and 2.1.

Result: Dialer appears with the number already populated, and the user can make a call successfully.

Device: Flame 2.0 (319mb, KK, Shallow Flash)
Build ID: 20141119000207
Gaia: 1ede2666f1e6c1b3fd3b282011caf0cbc59544b0
Gecko: faa64077b0c2
Version: 32.0 (2.0)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Device: Flame 2.1 (319mb, KK, Shallow Flash)
Build ID: 20141119001205
Gaia: 1b231b87aad384842dfc79614b2a9ca68a4b4ff3
Gecko: 95fbd7635152
Version: 34.0 (2.1)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0