Closed Bug 1039955 Opened 6 years ago Closed 6 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:2.0+, b2g-v2.0 verified, b2g-v2.1 verified)

VERIFIED FIXED
2.1 S3 (29aug)
blocking-b2g 2.0+
Tracking Status
b2g-v2.0 --- verified
b2g-v2.1 --- verified

People

(Reporter: echang, Assigned: alive)

References

Details

Attachments

(3 files)

### 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
blocking-b2g: --- → 2.1?
blocking-b2g: 2.1? → ---
QA Whiteboard: [COM=Gaia::Dialer]
See bug 1051719 comment 3 for clarifications on this bug.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1051719
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.
Status: RESOLVED → REOPENED
blocking-b2g: --- → 2.0?
Flags: needinfo?(jelee)
Resolution: DUPLICATE → ---
Flags: needinfo?(schung)
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.
blocking-b2g: 2.0? → 2.0+
I'm not sure this is a regression.
And moving to SMS per comment 2.
Component: Gaia::Dialer → Gaia::SMS
(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!
Flags: needinfo?(jelee)
(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.
Flags: needinfo?(schung)
Summary: [Dialer] "Call" is not functioning when tapping the number on sms which is brought up by sending sms from call log → [Messages] circular activity issue: "Call" is not functioning when tapping the number on sms which is brought up by sending sms from call log
Assignee: nobody → 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?
Flags: needinfo?(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.
Flags: needinfo?(alive)
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?
Flags: needinfo?(jelee)
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.
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.
(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!
Flags: needinfo?(jelee)
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...
Attachment #8479928 - Flags: review?(schung)
Assignee: salva → alive
Kill direct circular activity (caller.getBottomMostWindow ==== callee) by killing the front windows.

Indirect circular is not covered.
Attachment #8480390 - Flags: review?(etienne)
Component: Gaia::SMS → Gaia::System::Window Mgmt
(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 on attachment 8480390 [details] [review]
Fix direct circular activity by killing all foreground windows

r=me with the test renamed (see github)
Attachment #8480390 - Flags: review?(etienne) → review+
It seems there were several test failing which are unrelated but still re-running.
https://github.com/mozilla-b2g/gaia/commit/3f59dcdb71aed7147b2b9fc3716a0d4120e22a3d
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Making a new uplifted patch with the missing mock method
Flags: needinfo?(alive)
Attached file 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 :)
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
Status: RESOLVED → VERIFIED
QA Whiteboard: [COM=Gaia::Dialer] → [COM=Gaia::Dialer][QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [COM=Gaia::Dialer][QAnalyst-Triage?] → [COM=Gaia::Dialer][QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.