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

RESOLVED FIXED in Firefox OS v1.2

Status

Firefox OS
Gaia::E-Mail
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: asuth, Assigned: jrburke)

Tracking

unspecified
1.3 Sprint 3 - 10/25
x86_64
Linux
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:koi+, b2g18-, b2g18-v1.0.1 affected, b2g-v1.2 fixed)

Details

(Whiteboard: Interaction testrun 5.1 inarirun1 leorun1,inarirun2,leorun3, leorun4, retest_leorun4, burirun1)

Attachments

(4 attachments)

(Reporter)

Description

6 years ago
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.)
Andrew, bug 798445 landed with changes that should allow that. I don't know if the gaia side is done yet.
(Reporter)

Updated

6 years ago
Depends on: 801520
(Reporter)

Updated

6 years ago
No longer depends on: 801520
(Reporter)

Comment 2

6 years ago
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.
Depends on: 799039
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.
Depends on: 802108
No longer depends on: 799039
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.
(Reporter)

Comment 5

6 years ago
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

6 years ago
Hi Andrew RFI to you. what is the current status of this bug? is it still valid?
Flags: needinfo?(bugmail)

Updated

6 years ago
Whiteboard: Interaction [UX-P?]
(Reporter)

Comment 7

6 years ago
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.
Flags: needinfo?(bugmail)
tracking-b2g18: --- → ?
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.
tracking-b2g18: ? → -

Updated

5 years ago
Duplicate of this bug: 816583

Updated

5 years ago
Duplicate of this bug: 828464

Updated

5 years ago
Whiteboard: Interaction [UX-P?] → Interaction [UX-P?], testrun 5.1

Updated

5 years ago
Whiteboard: Interaction [UX-P?], testrun 5.1 → Interaction [UX-P?] testrun 5.1

Comment 11

5 years ago
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.
status-b2g18-v1.0.1: --- → affected
Whiteboard: Interaction [UX-P?] testrun 5.1 → Interaction [UX-P?] testrun 5.1 inarirun1

Comment 12

5 years ago
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.
Whiteboard: Interaction [UX-P?] testrun 5.1 inarirun1 → Interaction testrun 5.1 inarirun1

Updated

5 years ago
Whiteboard: Interaction testrun 5.1 inarirun1 → Interaction testrun 5.1 inarirun1 leorun1
(Reporter)

Updated

5 years ago
Duplicate of this bug: 870953

Comment 14

5 years ago
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
Whiteboard: Interaction testrun 5.1 inarirun1 leorun1 → Interaction testrun 5.1 inarirun1 leorun1,inarirun2

Updated

5 years ago
Blocks: 882421

Comment 15

5 years ago
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
Whiteboard: Interaction testrun 5.1 inarirun1 leorun1,inarirun2 → Interaction testrun 5.1 inarirun1 leorun1,inarirun2,leorun3

Updated

5 years ago
Whiteboard: Interaction testrun 5.1 inarirun1 leorun1,inarirun2,leorun3 → Interaction testrun 5.1 inarirun1 leorun1,inarirun2,leorun3, leorun4
blocking-b2g: --- → koi?

Updated

5 years ago
Whiteboard: Interaction testrun 5.1 inarirun1 leorun1,inarirun2,leorun3, leorun4 → Interaction testrun 5.1 inarirun1 leorun1,inarirun2,leorun3, leorun4, retest_leorun4
blocking-b2g: koi? → koi+
(Reporter)

Updated

5 years ago
Duplicate of this bug: 898985
Triage: This bug has been added to the v1.2 backlog but is not blocking release
blocking-b2g: koi+ → -
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?
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?
Anyway lemme create a bug in system to track this first.
No longer blocks: 909255
Depends on: 909255
Hi Alive,

Yes, the onsuccess and onerror function in MozActivity will be invoked after do "postResult()" and "postError()" in activity handler.
Created attachment 795368 [details]
video of inline mode transition
Created attachment 795369 [details]
video of window mode transition
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?
Flags: needinfo?(rmacdonald)
Adding Jacqueline to NI.
Flags: needinfo?(jsavory)
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. :(
Need UX input on transitions for this scenario.
Flags: needinfo?(firefoxos-ux-bugzilla)

Comment 28

5 years ago
Clearing the general flag since Rob and Jacqueline are already flagged on this.
Flags: needinfo?(firefoxos-ux-bugzilla)
(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
Flags: needinfo?(rmacdonald)
Flags: needinfo?(jsavory)
Blocks: 821519
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?
Hi all,

We could solve this bug when Bug 909255 is landed.
And thanks for Alive's work.
About back to opener transition:
I reuse the same transition as app to app transition to be consistent.
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.)
Assignee: nobody → alive
Created attachment 798646 [details]
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.
Attachment #798646 - Flags: review?(bugmail)
(Reporter)

Comment 35

5 years ago
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.)
Attachment #798646 - Flags: review?(bugmail) → review+
I think I also need to send PR to gelam ?
(Reporter)

Comment 38

5 years ago
(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.
Target Milestone: --- → 1.2 FC (16sep)

Updated

5 years ago
Whiteboard: Interaction testrun 5.1 inarirun1 leorun1,inarirun2,leorun3, leorun4, retest_leorun4 → Interaction testrun 5.1 inarirun1 leorun1,inarirun2,leorun3, leorun4, retest_leorun4, burirun1
(Reporter)

Updated

5 years ago
Duplicate of this bug: 915486
(Reporter)

Updated

5 years ago
Duplicate of this bug: 917637
Alive, can this bug be resolved?
Flags: needinfo?(alive)
(Reporter)

Comment 42

5 years ago
(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.
Flags: needinfo?(alive)
Andrew, would you steal the bug?
(Assignee)

Comment 44

5 years ago
Stealing for now, may reassign to other productivity member if another one becomes available before me.
Assignee: alive → jrburke
Target Milestone: 1.2 FC (16sep) → 1.3 Sprint 3 - 10/25
(Assignee)

Comment 45

5 years ago
Created attachment 820729 [details] [review]
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.
Attachment #820729 - Flags: review?(bugmail)
(Assignee)

Comment 46

5 years ago
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.
blocking-b2g: - → koi?
blocking-b2g: koi? → koi+
triage: koi+ based on comment #46
(Reporter)

Comment 48

5 years ago
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.
Attachment #820729 - Flags: review?(bugmail) → review+
(Assignee)

Comment 49

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

From pull request:
https://github.com/mozilla-b2g/gaia/pull/13014
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Uplifted f1329530a062d220cd70e74af373aa6f0d004b66 to:
v1.2: 08f547bcd094ae8842ad746c100c080d8e9753a8
status-b2g-v1.2: --- → fixed
Depends on: 936729
Depends on: 937400
No longer depends on: 936729
You need to log in before you can comment on or make changes to this bug.