802643 - [email] Compose/share activities are unable to return to calling app once message is sent

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Description

[Andrew Sutherland [:asuth] (he/him)]

We want to be able to return to the calling app once the user has finished composing a message and hit send.  We can't do this super cleanly right now, here's the situation:

1) If we were an inline activity, this would work, but we can't be an inline activity without some legwork.  As discussed on https://github.com/mozilla-b2g/gaia/issues/5136, the naive approach is not viable for this; we would need to establish a postMessage bridge between the inline context and the main app context. The e-mail app is broadly architected for this, but it does assume the inline context can start up the main daemon window context (which currently also includes the UI) if it's not already up and be able to talk to it.

2) Since we are not an inline activity and not a 'pick' activity, we are unable to cause the UI to return to the previous application when we complete the activity.

3) The workaround of using window.close() which now works could be a problem without some extra hacks.  Namely, if we close ourselves, we obviously can't be sending the message while we are closed.  This requires that we have the user wait to see that the message is sent.  This may be desired for UX reasons, but it could take several seconds, and it's possible it  might fail and then the UI would need to stay up for the retries.  As a hack, we could attempt to use mozAlarms to close ourselves but then immediately re-open ourselves in the background to complete the send.

So the possible solutions are, in order of increasing level-of-effort on the behalf of the e-mail app:
A) Fix the platform so that we can return to the originating app when our activity completes.
B) Just close() ourselves once the send completes successfully.
C) close() with hack
Da) See if we can perform some kind of shell game where we create an inline activity, and when it completes, have it trigger a non-inline e-mail activity to perform the send which will immediately return, which I believe the platform does support.  This assumes various things about the platform.
Db) Do the legwork so we can just postMessage the request to ourselves from the inline activity.  This could also involve splitting ourselves into a background process and the UI process (which is what e-mail was architected to do.)

Comment 1

[[:fabrice] Fabrice Desré]

Andrew, bug 798445 landed with changes that should allow that. I don't know if the gaia side is done yet.

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

The gaia bit was bug 799039 so we can do the right thing now.  Hooray!

So it's option #A and we just need to make sure we are returning the right value.

Comment 3

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Should depend on bug 802108; even if bug 799039 lands and System app knows the handling app should be gone, the calling app will not receive any |onsuccess| yet until bug 802108 is fixed.

Comment 4

[Steve Chung [:steveck]]

Hi Andrew,
The reason why we could not return to activity is here:https://bugzilla.mozilla.org/show_bug.cgi?id=802733#c7.

Hi Fabrice,
The share activity in email app set disposition: window now and Alive told me that window mode activity should not have the ability to return to calling activity. Is that confirmed that window mode activity could not, and will not able to switch back to calling activity? If so, the only way to match this requirement would be changing the email share activity to inline mode first, and make email app runnable under inline mode.

But I changed the share activity to inline mode in today's build(gaia: 20b23efa674ac49f62b191b6d8e5c6e285b10948, gecko: 43b9264e47c614c48a23c3efa2d8adefa7d5a4fb) and it will cause email app crash while photo sharing. I could not find any error in adb log now, so I'm still not sure about the root cause...

Even we could run email app under inline mode, there still has some detail that need to clarify first. For example, we may need to wait for the mail sent status callback before switch back. If we call postResult/error first, it will remove the email frame and it might kill the email thread and cause error if mail is still sending.

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

We *cannot* run the mail app as 'inline' right now.  Even if it looks like it works, it will corrupt the database.

The new plan for mail sending from a discussion on Wednesday with :cyee, :clee, and :doliver is to have it block the UI until it completes.  So while we are sending, we will put up a an overlay that says "sending mail".  Once it succeeds, we can return to the calling activity or to the message list.

Comment 6

[ayman maat :maat]

Hi Andrew RFI to you. what is the current status of this bug? is it still valid?

Comment 7

[Andrew Sutherland [:asuth] (he/him)]

Here's the code situation:
- We try to return to the invoking app via activity if it was a "share" activity.
- We do not try to return if it was a "new" activity.

From testing on an unagi just now:
- When we return from the "share" activity, I just ended up back at the home screen rather than back in the gallery app.  According to the long-press home card view, the gallery app was still running, so it's not clear why we did not return to the gallery.  This seems like a system app or platform issue rather than an e-mail app issue.

So the remaining work appears to be to always try and return from all activities, and see what we can do about us failing to return all the way to the right app.

As noted on other bugs, this is not blocking and is accordingly not being worked.

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

This looks like a significant level of effort and we're not certain this would go into a v1 train, will leave it up to UX team to re-nom if they expect to drive this into a v1.next release for a particular partner requirement.

Comment 11

[Jeni]

Issue repros on the Inari Build ID: 20130416070200
Kernel Date: Feb 21
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/6bac24e14538
Gaia: c883af5ecd0998f78d9aaa4c2337c842e1dbb5a0

When you click on contact information and send an email user is not returned to contact application afterwards.  Instead user is returned to email inbox.

Comment 12

[Stephany Wilkes]

Thanks for reporting this. UX reviewed today and recommends users be returned to the Contacts app. It seems like this may actually be a more serious bug: after the user sends the email, the Contacts app is closed. This means that the Contacts app is no longer there as one of the open apps in the task switcher, which means the user can't return to it at all. 

UX recommends that:
* Contacts app should not close when user is taken over to email to compose.
* Contacts should remain accessible in the task switcher.

Please let us know if this comment is sufficient or if a wireframe or sketch is needed.

Comment 14

[sarsenyev]

The bug still reproduce on Inari device.
A user returns to the email after a email is sent from the "Contact" app

Environmental  Variables:
Inari Build ID: 20130515070208
Kernel Date: Feb 21
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/d06cfe7d67c2
Gaia: 0ddb515f15cbc6b74fc2742b7599d6ae74c6413f

Comment 15

[mdaniloff]

The bug still reproduces on the Leo device.
After an email is sent from the "Contact" app, the user returns to the email inbox

Environmental Variables
Leo Build ID: 20130610070206
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/8e3f39363c54
Gaia: ce3b99781d182ad550a325206990c249b0dbcf0e
Platform Version: 18.0

Comment 17

[Dylan Oliver [:doliver]]

Triage: This bug has been added to the v1.2 backlog but is not blocking release

Comment 18

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

I wonder we need more "dirty" work to enable "back to window-disposition activity requester from activity". Window-disposition activity and activity requester are two different app windows which have no relationship. We don't link these two apps right now so we don't do "return to previous app" in system app.

Some problems needs to know:
* If opened window-position activity is doing |postResult|,
  does activity requester onsuccess is invoked?
* What about oncancel case?
* What would happen if the activity callee is closed/crashed/killed? Shall we still go to "previous app"?
* What's the app transition here?

Comment 19

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

After tracing gecko code,
I think it's possible to have some change in window manage to enable this. But I still want to know:
* Shall we kill the window disposition activity when it does postError/postResult? We do so for inline activity case.
* Shall we go back to activity opener if the opened activity is killed/removed by sudden?

Comment 20

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

Anyway lemme create a bug in system to track this first.

Comment 21

[Evan Tseng [:evanxd]]

Hi Alive,

Yes, the onsuccess and onerror function in MozActivity will be invoked after do "postResult()" and "postError()" in activity handler.

Comment 22

[Evan Tseng [:evanxd]]

Attachment: video of inline mode transition

Comment 23

[Evan Tseng [:evanxd]]

Attachment: video of window mode transition

Comment 24

[Evan Tseng [:evanxd]]

Hi Rob,

For fixing this bug, we need a UX support.

How do you think about the transition effect from Email App to Gallery App after the sharing email is sent?

I think the window mode transition more makes sense, right?
You could see the window mode transition in attachment 795369 [details].
Or the transition in attachment 795368 [details] is better?

How do you think?

Comment 25

[Peter Dolanjski [:pdol]]

Adding Jacqueline to NI.

Comment 26

[Evan Tseng [:evanxd]]

Hi all,

If we open App in onsuccess function in MozActivity in Gallery App, the patch is https://github.com/evanxd/gaia/commit/dafd29c43950754dd4173133627495e6651bc213.

The result is just like this video http://youtu.be/4qVf7JN7mCk.
I think it's NOT good. :(

Comment 27

[Peter Dolanjski [:pdol]]

Need UX input on transitions for this scenario.

Comment 28

[Stephany Wilkes]

Clearing the general flag since Rob and Jacqueline are already flagged on this.

Comment 29

[Rob MacDonald [:robmac]]

(In reply to Evan Tseng [:evanxd] from comment #24)
> Hi Rob,
> 
> For fixing this bug, we need a UX support.
> 
> How do you think about the transition effect from Email App to Gallery App
> after the sharing email is sent?
> 
> I think the window mode transition more makes sense, right?
> You could see the window mode transition in attachment 795369 [details].
> Or the transition in attachment 795368 [details] is better?
> 
> How do you think?

Hi Evan...

Thanks for your patience. The transition spec is located on box.com - https://mozilla.box.com/s/k6mzxv86d8ebjd9uga8a. I just confirmed this file is open so you should be able to access. The transition you describe is defined on page 36 of the document.

Best regards,
Rob

Comment 30

[Evan Tseng [:evanxd]]

Hi Rob,

Thanks for the documentation.

So the UI flow in the video http://youtu.be/4qVf7JN7mCk is what we want if we could skip the homescreen screen, right?

Comment 31

[Evan Tseng [:evanxd]]

Hi all,

We could solve this bug when Bug 909255 is landed.
And thanks for Alive's work.

Comment 32

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

About back to opener transition:
I reuse the same transition as app to app transition to be consistent.

Comment 33

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

Hi, we have to remove window.close() in email app to prevent race condition.
https://github.com/mozilla-b2g/gaia/blob/master/apps/email/js/mail_app.js#L258

I could make a patch if this doesn't take much time. (Don't know if I need to write tests on this change.)

Comment 34

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

Attachment: https://github.com/mozilla-b2g/gaia/pull/11894

bug 909255 is under reviewing, after it's resolved, the workaround in email app would cause trouble because mozbrowserclose event is conflicting with activity-done event and we couldn't display the activity caller since the activity callee closes itself by somewhat reason.

Sorry I don't write a test for this because the real test shall be system wide, not in email app.

Comment 35

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 798646 [details]
https://github.com/mozilla-b2g/gaia/pull/11894

r=asuth on removing the specific hack workaround.  There's other work that still needs to happen on this bug in our specific activity handling, so please don't close the bug when landing this fix.

(Normally I think we'd want to try and land all the patches for this bug together, but I realize that you're trying to fix the platform at the same time, and so this is a good case for you to be able to test, etc.)

Comment 36

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

I think I also need to send PR to gelam ?

Comment 37

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

gaia master
https://github.com/mozilla-b2g/gaia/compare/1398204704f7...f96dc0e852ab

Comment 38

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Alive Kuo [:alive] from comment #36)
> I think I also need to send PR to gelam ?

The code under apps/email/js/ext comes from the gaia-email-libs-and-more repo.  So only if you are changing code that you see in there.

Comment 41

[Dylan Oliver [:doliver]]

Alive, can this bug be resolved?

Comment 42

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Dylan Oliver [:doliver] from comment #41)
> Alive, can this bug be resolved?

No, per comment 35 there is still e-mail specific changes that need to happen.  We need to call postResult/postError to actually cause the transitions to occur.  Alive's patch just removed a particularly egregious hack on one of our code paths.

Comment 43

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

Andrew, would you steal the bug?

Comment 44

[James Burke [:jrburke]]

Stealing for now, may reassign to other productivity member if another one becomes available before me.

Comment 45

[James Burke [:jrburke]]

Attachment: Pointer to pull request 13014

This is the last bit of changes to complete this ticket. The 'new' and 'view' activity registration did not specify "returnValue", so even though the email app did call that activity.postX methods, they did not do anything for those cases.

The "disposition" key was set explicitly to "window" in the "view" case too, just to keep it matched with the other activity config blocks, and to also hopefully indicate our purposeful desire to use "window" instead of "inline".

Finally, since there are now three different types of activities that all trigger the compose flow, the compose.js was changed just to call `activity.postResult('complete')` always if an activity caused the compose flow, instead of just doing it for the "share" activity.

With these changes, the email app returns to the previous app in both the error and success cases. 

One exception is that the Contacts app could get killed for out of memory, and in that case, the postResult() does not do anything as the target app is no longer running, and the user stays in the email app after the email is sent. The browser app seems to do a better job of sticking around for the return from the activity.

Asking :asuth for review since he had context further back for the ticket. However, I can switch the reviewer to :gaye given :gaye's recent work on activities if :asuth is satisfied that this description meets with what he thought needed to be done.

Comment 46

[James Burke [:jrburke]]

Asking for koi+ for the pull request mentioned in in Comment 45 as it is very low impact and it completes the other work associated with this ticket that is already in the 1.2 branch since it was completed before the 1.2 branch point.

Comment 47

[Dylan Oliver [:doliver]]

triage: koi+ based on comment #46

Comment 48

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 820729 [details] [review]
Pointer to pull request 13014

r=asuth by inspection.  I've got device problems for another day or two, and the run-in-firefox-nightly thing isn't triggering web activities for me right now, so I am unable to manually test and don't want to hold things up.

Comment 49

[James Burke [:jrburke]]

Merged to Gaia master:
https://github.com/mozilla-b2g/gaia/commit/f1329530a062d220cd70e74af373aa6f0d004b66

From pull request:
https://github.com/mozilla-b2g/gaia/pull/13014

Comment 50

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

Uplifted f1329530a062d220cd70e74af373aa6f0d004b66 to:
v1.2: 08f547bcd094ae8842ad746c100c080d8e9753a8