950037 - [Email][V1.3] Tap "<" icon cannot get any response

Status: VERIFIED FIXED

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

Description

[William Hsu [:whsu]]

Attachment: WP_20131213_005.mp4

* Description:
  Try to input a incorrect account/password.
  After the warning message pops up, tapping the "<" icon cannot get any response.
  Attaching the video (WP_20131213_005.mp4)

* Reproduction steps:
  1. Add an email account into email app
  2. Add the second email account on email app but input the incorrect account/password
  3. After email app pops up the warning message, please tap "<" to go to previous page

* Expected result:
  User can go to previous page

* Actual result:
  Tap "<" icon cannot get any response

* Test build:(V1.3)
 - Gaia:     588a3e02c4ace3b3341ba1f6bb7274120b53b2b3
 - Gecko:    http://hg.mozilla.org/releases/mozilla-aurora/rev/031270be3702
 - BuildID   20131212004003
 - Version   28.0a2

Comment 1

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

logcat, please: https://wiki.mozilla.org/Gaia/Email/RequiredBugInfo

Comment 2

[Tony Chung [:tchung]]

1.3+.  william, please provide a logcat for devs when reproducing.  Thanks!

Comment 3

[William Hsu [:whsu]]

Attachment: EmailApp_SignInLog_20130108.log

Comment 4

[William Hsu [:whsu]]

As attachment (EmailApp_SignInLog_20130108.log).
Thanks!

Comment 5

[James Burke [:jrburke]]

Attachment: GitHub pull request

I believe this was caused by the changeset for bug 869131 because it was assuming bug 940132 was going to land imminently with some transition changes. Since that change will not land now until 1.4, need to add back in the 'right' navigation.

Asking :asuth for review only because he was involved with bug 869131.

Comment 6

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

Comment on attachment 8358179 [details] [review]
GitHub pull request

r=asuth conditional on pulling the logging enhancement commit at https://github.com/asutherland/gaia/commit/a5de11072f6b3326ff9d89e70e321a043145e61a into your pull request.

I sorta grokked what was going on without logging, but I added some logging and I think this helps make it more clear what is/was happening.  It is admittedly fairly chatty, but there's complicated stuff going on.  Feel free to make enhancements to the logging.

===== Log:
== Andrew clicks 'settings' from folder picker:
LOG: pushCard: settings_main   mode: default   placement: left   index: 0   count: 3
LOG: showCard: 0   method: animate   dir: forward   active: 1   before fixup: 1
LOG:   applied zIndex: 10
LOG: onTransitionEnd: dead count to reap: 0
LOG: Preloading cards: setup_account_info,settings_account

== Andrew clicks 'add account':
LOG: pushCard: setup_account_info   mode: default   placement: right   index: 1   count: 4
LOG: showCard: 1   method: animate   dir: forward   active: 0   before fixup: 0
LOG:   applied zIndex: 10
LOG: onTransitionEnd: dead count to reap: 0

== Andrew types in bad credentials and hits 'next' and we show progress
LOG: deferring card push for type: setup_progress
LOG: pushCard: setup_progress   mode: default   placement: undefined   index: 4   count: 5
WLOG: Attempting to get autoconfiguration for clicky.visophyte.org
WLOG:   Looking in GELAM
WLOG:   Looking in local file store
LOG: showCard: 4   method: animate   dir: forward   active: 1   before fixup: 1
LOG:   applied zIndex: 20
WLOG:   Looking at domain (Thunderbird autoconfig standard)
LOG: onTransitionEnd: dead count to reap: 0
WLOG: SUCCESS
WLOG: PROBE:IMAP attempting to connect to clicky.visophyte.org
WLOG: PROBE:SMTP attempting to connect to clicky.visophyte.org

== The server timeout completes and we get our failure, progress card goes away:
WWAR: PROBE:SMTP sad. error: | bad-user-or-pass |  |
WWAR: PROBE:IMAP sad. Error | NO | Error while executing request: [AUTHENTICATIONFAILED] Authentication failed. | [AUTHENTICATIONFAILED] Authentication failed.
LOG: removeCardAndSuccessors: firstIndex: 4   domNodeWasNull?: false   numCards: 1
LOG:   picked next card index: 3 from min(3, 4)
LOG: showCard: 3   method: animate   dir: back   active: 4   before fixup: 4
LOG:   rcas: new activeCard: 3   old activeCard: 3
LOG:   rcas: waiting on animation to remove node
LOG: onTransitionEnd: dead count to reap: 1

== I hit '<':
LOG: removeCardAndSuccessors: firstIndex: 1   domNodeWasNull?: false   numCards: 1
LOG:   picked next card index: 0 from min(0, 3)
LOG: showCard: 0   method: animate   dir: back   active: 3   before fixup: 3
LOG:   rcas: new activeCard: 0   old activeCard: 0
LOG:   rcas: waiting on animation to remove node


===== Analysis:
(You'd also get this from code reading and an understanding of the system, but it doesn't jump out at you as much.)

- When we push the settings card on, we push it at index 0, leaving all our folder picker/message cards above it, numerically.
- When we push the setup_account_info card on, we push it to the right of the main settings card at index 1.
- When we push the progress card on, we don't specify "right", so it ends up way at the end of the list, index 4.
- When it pops, we end up trying to show card #3, not card #1!  But because of the zIndex magic, we don't see that card.  This happens because our card removal always assumes we want the card to the left of ourselves if we don't specify a specific card to return to.  Architecturally and conceptually, it seems like we really want is to be pushing a new stacking context when we enter settings.  This also does suggest we might also consider doing more "visibility: hidden" or other means of putting cards on ice that don't cause reflows when we bring them back.  (Or maybe display: none is right if we are somehow causing reflows on those hidden cards.)
- Then when we hit '<' we do try and remove the correct card, but the animation state machine goes off the rails and we wait for a transition that never happens so we never remove the node, etc. etc. etc.

James' fix is right since it resolves the dumb 4->3 transition by making it a 2->1 transition.

My apologies for inducing this bug in the first place.

Comment 7

[James Burke [:jrburke]]

Great analysis. In my ideal world, I do like the idea of creating different card stacks/contexts/deck instances, for card sets that go "on top of" other sets, and had played with that idea here before:

https://github.com/jrburke/carmino/blob/master/www/lib/Deck/Deck.js

The existing zIndex-stack solution was done in the old pre-1.0 days to avoid code churn, but I would like to revisit this at some point. I would likely use that Deck.js as a basis, but the email code had some other nuances around `mode` in particular that I had not accounted for in the Deck experiment.

Comment 8

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

Yeah, there's definitely some technical debt to pay down, most of which was due to inability to make large changes to the code-base.  (And this was even during the v1.1 time-frame and some of v1.2).  Including getting rid of that 'mode' stuff.  The usage in the folder/account picker was an outgrowth of a very specific UX spec that is now dead.  And the usage in the message list (folder display versus search mode) is something that should be dealt with by using web components or other means of factoring.

Comment 9

[James Burke [:jrburke]]

Merged in Gaia master:
https://github.com/mozilla-b2g/gaia/commit/9189c61e4cb2137ccae2ca8f78b648b8f8e0d94f

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

Comment 10

[William Hsu [:whsu]]

Thanks for your help!
I cannot reproduce this bug on latest master build.
Verified it.

By the way, this is V1.3+ bug. Please help merge into V1.3 branch while you have free time.
Thanks. Have a nice day!

* Verified Build:
 - Gaia      3a41d9c2c6dce6a5a248495a1847f8eaa703ef83
 - Gecko     http://hg.mozilla.org/mozilla-central/rev/171857e7e334
 - BuildID   20140112040202
 - Version   29.0a1

* Result:
 - Cannot reproduce

Comment 11

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

Uplifted 9189c61e4cb2137ccae2ca8f78b648b8f8e0d94f to:
v1.3: 17417a8a5fde43a4c55d95bf0a9651a4f06f93eb

Comment 12

[William Hsu [:whsu]]

Thanks.
I also cannot reproduce this bug on latest V1.3 build.
Mark as "VERIFIED"


*Build Information:
 - Gaia      26e8a950f3ac703310ee8efcb741ff2418e9d190
 - Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/ecdc275e9741
 - BuildID   20140217004003
 - Version   28.0

* Test Result:
 - Cannot reproduce