925174 - [email] webactivity-triggered compose activity does not trigger after creating a new account created because there were no existing accounts

Status: VERIFIED FIXED

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

Description

[Chris Kreinbring [:CKreinbring]]

Attachment: Log of logging in to email from SMS text

Description:
Attempting to send a message to an email address from an SMS text will send the user to the email's Inbox instead of the Compose page if the user is not logged in to an email account beforehand.

Repro Steps:
1) Update Buri to Build ID: 20131009004001
2) Launch the Messaging app.
3) Send an SMS containing an email address.
4) After sending, tap the email address in the text.
5) Once the Email app loads, login with valid credentials, service provider doesn't matter.
6) Advance through the preliminary setup and observe the page that appears after selecting Continue to Mail.

Actual:
The user is taken to the Inbox page.

Expected:
The user is taken to the Compose page.

Environmental Variables
Device: Buri 1.2 mozilla RIL
Build ID: 20131009004001
Gecko: http://hg.mozilla.org/releases/mozilla-aurora/rev/e5edb484e2a9
Gaia: 672c47bf94b69a329e0aacb9228a6aa16ade6226
Platform Version: 26.0a2

Notes:
Repro frequency: 100%
Link to failed test case: https://moztrap.mozilla.org/manage/cases/?filter-id=7166
See attached logcat logs
Works as expected on Leo 1.1 commercial RIL

Comment 1

[Jason Smith [:jsmith]]

Sounds like the compose activity isn't working as expected then.

Comment 2

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #1)
> Sounds like the compose activity isn't working as expected then.

Fix on wording - meant to say the new activity to create a message.

Comment 3

[Preeti Raghunath(:Preeti)]

David,

Please review - seems to be an SMS/ Contacts issue.

Comment 4

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

This looks real from the log and the code.  According to the code, mail_app.js wants to see an 'acctsSlice' emit() call in order for us to trigger the activityCallback.  But that would likely only happen if resetApp() is called, but we only call that if we hit "back" from creating another account.  In the success case, SetupDoneCard just emits 'showLatestAccount'.

We'll want to add an automated integration test for this once we fix up the plumbing which should be fairly minimal.  jrburke, do you think we should do something like emit('addedFirstAccount') and have the activityCallback hang off of that?

Comment 5

[Dylan Oliver [:doliver]]

triage: koi+ for the regression

Comment 6

[Mila Davydova]

Regression range:

Buri Build ID: 20130906040204 – does not reproduce
Gecko: http://hg.mozilla.org/mozilla-central/rev/ab5f29823236
Gaia: 94e5f269874b02ac0ea796b64ab995fce9efa4b3
Platform Version: 26.0a1

Build ID: 20130909114657
Gecko: http://hg.mozilla.org/mozilla-central/rev/218d4334d29e
Gaia: aa4180e9286d385fa6b62d236f30fb24cd8b93e9
Platform Version: 26.0a1

There are no 1.2 builds available for 09/08 and 09/07 to narrow down the regression window.

Comment 7

[Mila Davydova]

in addition to comment 6

 Buri build ID: 20130909114657 - issue started to reproduce

Comment 8

[Julien Wajsberg [:julienw]]

not a comms app issue, clearing David's NI.

Comment 9

[James Burke [:jrburke]]

Attachment: Pointer to pull request 13011

If a web activity opened email, and there was no email accounts configured, the user had option to configure an email account, then continue with the compose web activity. However, what happened was that the compose action was lost once the account was set up.

This change makes sure to continue any web activity action after getting the 'showLatestAccount' event from the card setup. Since that continuation work needed to happen in two places, a new function was created to do the work for each case. the onPushed option for a card is run after the card for the current action completes.

Includes an integration test to make sure this pathway does not regress in the future.

Comment 10

[Gareth Aye [:gaye] (back from PTO)]

Comment on attachment 820673 [details] [review]
Pointer to pull request 13011

I had a few small questions, but mostly this is a great patch. Thanks jrburke :)

Comment 11

[James Burke [:jrburke]]

Addressed questions and merged on Gaia master:
https://github.com/mozilla-b2g/gaia/commit/8560b0128951a609ee3a89047544e0385e4517c8

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

Comment 12

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

Uplifted 8560b0128951a609ee3a89047544e0385e4517c8 to:
v1.2: 8e4900a2c93f80550a5787b54fa107e448470456

Comment 13

[William Hsu [:whsu]]

Many thanks! Verified it.

* Test Build:
 - Gaia:     2ef9bc3c7a6de228b63e6ba3613eb0c0dd639c59
 - Gecko:    http://hg.mozilla.org/releases/mozilla-aurora/rev/4a94d2ea9d37
 - BuildID   20131028004002
 - Version   26.0a2