(inactive after 6/18) Alive Kuo [:alive] Reporter
	Description • 12 years ago

STR:
1. Open whatever app, e.g. browser app.
2. Return to homescreen. Open [UITest]->[navigator.mozPay tests]
3. Click [Pay PMPP]
4. Homescreen pops, trusted dialog shows.
5. Long press home button to invoke card view.
6. Click browser app card in card view.

Expected:
* Trusted dialog either cancel home/holdhome event or deal with app switch.

Actual
* Trusted dialog shows behind browser app. And every button of trusted dialog seems failed.

	Fernando Jiménez Moreno [:ferjm] Assignee
	Comment 2 • 12 years ago

Pointer to Github pull-request

	Fernando Jiménez Moreno [:ferjm] Assignee
	Comment 3 • 12 years ago

Comment on attachment 673205 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/5900

Thanks Alive!

This patch should allow the trusted UI to handle app switch and also app termination.

	alberto.pastor
	Comment 4 • 12 years ago

Hi guys,

Are animations are going to change (and this patch has differente animation that the one when you open the trustedUI at firts time), would you mind if I fix when the final transitions are released?

	Fernando Jiménez Moreno [:ferjm] Assignee
	Comment 5 • 12 years ago

If I'm not wrong this has nothing to see with animations and needs to be fixed anyway. So I would prefer to merge this as soon as possible, if you agree to :)

	alberto.pastor
	Comment 6 • 12 years ago

Well, it fades out instead hiding in the bottom when coming back from the task switcher. What's the benefit of merging it right now? Is not going to be tested until final transitions are defined..

	Fernando Jiménez Moreno [:ferjm] Assignee
	Comment 7 • 12 years ago

(In reply to Alberto Pastor from comment #6)
> Well, it fades out instead hiding in the bottom when coming back from the
> task switcher. What's the benefit of merging it right now? Is not going to
> be tested until final transitions are defined..

It seems that Jason is going to start testing the trusted UI [1], so IMHO we should merge this asap, if possible, to save him the time to report known issues. Also the identity folks would probably appreciate a working trusted UI, even if it doesn't provide the final animations. Let's focus on the functionality that we know that is gonna be needed first (handling the app switch is gonna be needed independently of the chosen animation). Then we can deal with the final UX :)

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=801561#c4

	alberto.pastor
	Comment 8 • 12 years ago

(In reply to Fernando Jiménez Moreno [:ferjm] from comment #7) 
> It seems that Jason is going to start testing the trusted UI [1], so IMHO we
> should merge this asap, if possible, to save him the time to report known
> issues. Also the identity folks would probably appreciate a working trusted
> UI, even if it doesn't provide the final animations. Let's focus on the
> functionality that we know that is gonna be needed first (handling the app
> switch is gonna be needed independently of the chosen animation). Then we
> can deal with the final UX :)
> 
> [1] https://bugzilla.mozilla.org/show_bug.cgi?id=801561#c4

Transitions are 90% of functionality. I think we should test when the code is finished. Anyway, I'm not going to discuss anymore this, I'm ok with the code, so r+.

	Jason Smith [:jsmith]
	Comment 9 • 12 years ago

(In reply to Alberto Pastor from comment #8)
> (In reply to Fernando Jiménez Moreno [:ferjm] from comment #7) 
> > It seems that Jason is going to start testing the trusted UI [1], so IMHO we
> > should merge this asap, if possible, to save him the time to report known
> > issues. Also the identity folks would probably appreciate a working trusted
> > UI, even if it doesn't provide the final animations. Let's focus on the
> > functionality that we know that is gonna be needed first (handling the app
> > switch is gonna be needed independently of the chosen animation). Then we
> > can deal with the final UX :)
> > 
> > [1] https://bugzilla.mozilla.org/show_bug.cgi?id=801561#c4
> 
> Transitions are 90% of functionality. I think we should test when the code
> is finished. Anyway, I'm not going to discuss anymore this, I'm ok with the
> code, so r+.

Note - I can hold off testing if need be. The comment you saw over there was cause I thought there was a good enough of piece of work landed to look at to start giving feedback (I usually prefer testing in small increments, rather than doing it in one large clump). If that's not the case, please explain why.

	alberto.pastor
	Comment 10 • 12 years ago

(In reply to Jason Smith [:jsmith] from comment #9)
> Note - I can hold off testing if need be. The comment you saw over there was
> cause I thought there was a good enough of piece of work landed to look at
> to start giving feedback (I usually prefer testing in small increments,
> rather than doing it in one large clump). If that's not the case, please
> explain why.

Hi Jason, my only concern is that we are doing things as dangerous as hiding the current application, which could lead the user to a status in which is not able to click anything. This kind of errors could be introduced on the animations implementation. My impression is that if we test it right now, and we implement the animations later, test should be repeated. 
I see the point of testing what we have right now just in case no final decision on the animations is taken, and just leave it as it is right now. I'm ok with that as well, but as I said, we'll need to repeat the tests if another animation is needed.
In my opinion, the behavior when clicking home/task switch is very related to the UX final input, and that's the reason I was suggesting not merging this until that input is final.

Anyway, merging this + testing, and re-testing after the UX input comes in seems like a plan as well. Up to you guys! :)

	Fernando Jiménez Moreno [:ferjm] Assignee
	Comment 11 • 12 years ago

I still can't see how this patch defers from previous patches related to the trusted UI that avoids it to be merged. But I am also ok with holding its merge if Jason holds off testing or if this issue is taken into account while testing or while developing the identity parts affected by the trusted UI.

	alberto.pastor
	Comment 12 • 12 years ago

That's a different argument. Is not different to other patches. All these merges are leaded by "having something", just wanted to stop them at some point. As I said, I don't want to keep talking about this. I'm OK merging this and testing it. We'll create another patch and re-test when the animations are ready. Sounds good?

	David Scravaglieri [:scravag]
	Comment 13 • 12 years ago

Blocking because no workaround for the user

	Fernando Jiménez Moreno [:ferjm] Assignee
	Comment 14 • 12 years ago

Sounds good. Thanks Alberto.

https://github.com/mozilla-b2g/gaia/commit/6f6d4433f64b473f53eb69f2cbb60ccd28c86c7b

	Jason Smith [:jsmith]
	Comment 15 • 12 years ago

Planning on looking into this today.

	Jason Smith [:jsmith]
	Comment 16 • 12 years ago

Verified, although it looks like I hit bug 810449 which if that occurs, can cause this bug to reproduce if the work-around in that bug is executed. I'll track followup work there.