Jason Smith [:jsmith] Reporter
	Description • 12 years ago

Build: B2G 18 1/14/2013
Device: Unagi

Steps:

1. Go to http://www.se.rit.edu/~jds2501/persona_observer_test.html
2. Start the login process
3. After your submit your creds, hit the home button
4. Wait a few seconds

Expected:

The homescreen should remain functional. When I decide to return to that app, I should be logged in with no trusted UI present.

Actual:

The homescreen completely disappears. If I then lock and unlock the phone, I can get the homescreen to reappear. If I decide to return to the browser app, the trusted UI is still shown, but remains unfunctional with white content. The only work-around at this point would be to kill the app running the trusted UI.

Additional Notes:

Bad regression from bug 825806. We need to block - one app shouldn't be able to take down the homescreen temporarily and lock itself up.

	Jason Smith [:jsmith] Reporter
	Comment 1 • 12 years ago

Hmm...very easy to reproduce too.

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 2 • 12 years ago

828182 is the problem.

Currently gaia doesn't know the closed browserid iframe is owned by which dialog. It just 'guess'es. Exactly this is not a regression because bug 825806 was hit beyond this bug.
I have to say, currently the implementation of trustedui and identifier needs to be improved -- otherwise we will encounter more issues like this.

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 3 • 12 years ago

I just took a look again. Maybe we could use chromeEventId in as the unique identifier...
But this is still not very reliable.

Jason, do you have a test to setTimeout to identifier login?
I mean if you delay the call to navigator.id.login to 5 sec and then switch to another app, would the trustedUI show
?

	Jason Smith [:jsmith] Reporter
	Comment 4 • 12 years ago

(In reply to Alive Kuo [:alive] from comment #3)
> I just took a look again. Maybe we could use chromeEventId in as the unique
> identifier...
> But this is still not very reliable.
> 
> Jason, do you have a test to setTimeout to identifier login?
> I mean if you delay the call to navigator.id.login to 5 sec and then switch
> to another app, would the trustedUI show
> ?

In my case, I wasn't necessarily testing that. I actually tested the flow of submitting my creds and hitting the home button immediately and waiting a few seconds.

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 5 • 12 years ago

(In reply to Alive Kuo [:alive] from comment #3)
> I just took a look again. Maybe we could use chromeEventId in as the unique
> identifier...
OK I am wrong, the id is different every time gaia gets a mozChromeEvent. Uh.
So it's impossible to make a reasonable multiple trustedUI implementation now unless bug 828182 gets fixed.

Another way:
Make trustedUI to be system wide. That means only one app is allowed to open trustedUI at the same time.

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

Seems like bug 828182 patch ensures that chromeEventId of open/close event of the same dialog is the same.
This could be fixed if 828182 is landed and check the id/frameOrigin of dialog when closing it.

Ideally I would like to move restoreCurrentApp/hideCurrentApp out of Window Manager..

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 7 • 12 years ago

Patch v1:
1. Move TrustedUI related logic out of Window Manager
2. TrustedUI cleanup

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

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

Thanks Alive. I didn't look deep at the code, but I am afraid that this patch would break some of the functionality that we currently support. Specifically, we can't have two trusted UIs owned by to different apps at the same time.

You can test this scenario by calling mozPay() from two different apps. Unfortunately, the mock payment provider preferences were removed from Gaia, so you would have to add them yourself to 'build/payment-prefs.js':

pref("dom.payment.skipHTTPSCheck", true);
pref("dom.payment.provider.0.name", "mockpayprovider");
pref("dom.payment.provider.0.description", "Mock Payment Provider");
pref("dom.payment.provider.0.type", "mock/payments/inapp/v1");
pref("dom.payment.provider.0.uri", "http://people.mozilla.com/~kmcmillan/pay-provider.html?req=");
pref("dom.payment.provider.0.requestMethod", "GET");

You can use the mozPay test from the UITests app and the tests at http://people.mozilla.com/~kmcmillan/gaia-pay.html.

STR:
- Open the browser and load http://people.mozilla.com/~kmcmillan/gaia-pay.html
- Click on 'Pay PMPP'.
- Once trusted UI is shown with the pay flow, close the dialog by clicking on the HOME button.
- Open the UITests app and choose the 'navigator.mozPay' tests.
- Click on 'Pay PMPP'.
- Once trusted UI is shown with the pay flow, close the dialog by clicking on the HOME button.
- Open the browser app again, which should show the trusted UI again.
- Click on 'Success'.

Expected:
- The trusted UI should close and show the browser app.

Current: 
- The payment flow opened from the UITests is shown and the trusted UI remains unusable.

FTR, the payment callbacks are properly triggered.

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

Alive or Ferjm - Is it possible to fix this bug is a low risk way?

The comments above concern me that we're trying to do a rewrite here, which will put us at risk for blocking regressions.

If it's not possible to find a low risk way, should we continue to block on this?

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 10 • 12 years ago

Jason,
I will look into what should be done to trusted ui with payment according to comment 8 and give feedback soon.

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 11 • 12 years ago

When testing with payment, there is another bug:

STR:
- Open the browser and load http://people.mozilla.com/~kmcmillan/gaia-pay.html
- Click on 'Pay PMPP'.
- Once trusted UI is shown with the pay flow, close the dialog by clicking on the HOME button.
- Open the UITests app and choose the 'navigator.mozPay' tests.
- Click on 'Pay PMPP'.
- Once trusted UI is shown with the pay flow, close the dialog by clicking on the HOME button.
- Open the browser app again, which should show the trusted UI again.
- Click on 'Success'.
- Click on 'Pay PMPP' again in the browser page.

Expected:
Trusted UI shows on homescreen

Actual:
Trusted UI shows on browser app

Log:

E/GeckoConsole(  108): Content JS LOG at app://system.gaiamobile.org/js/trusted_ui.js:80 in trui_open: open-payment-flow-dialog-{3cab09cf-758f-4986-8549-8e992893f503} app://browser.gaiamobile.org =======
I/Gecko   (  108): ======================= payment.js ======================= 
E/GeckoConsole(  108): Content JS LOG at app://system.gaiamobile.org/js/trusted_ui.js:80 in trui_open: open-payment-flow-dialog-{c2169b1a-b3df-4ef2-be02-264224a93cad} app://uitest.gaiamobile.org =======
I/Gecko   (  108): ======================= payment.js ======================= 
E/GeckoConsole(  108): Content JS LOG at app://system.gaiamobile.org/js/trusted_ui.js:98 in trui_close: open-payment-flow-dialog-{c2169b1a-b3df-4ef2-be02-264224a93cad} app://browser.gaiamobile.org =======

Note that the id returned by close payment in browser just gives the chrome id of ui-test got...this is totally wrong. This is related to Bug 828182 I think.

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

I agree not to block on this unless we have much time to redesign and re-test.

Bug 818292 doesn't solve the problem...chrome event id is NOT related to payment one by one. Or there is something wrong in platform....

	Jason Smith [:jsmith] Reporter
	Comment 13 • 12 years ago

Sounds fine to not block given the risk. Let's get this on tracking radar for a future release though - Jed indicated concern that some cleanup of the trusted UI code for a bug like this is necessary eventually (I believe the reason was fragility being the concern), although we'll have to be careful when doing the cleanup to avoid regressions.

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

(In reply to Alive Kuo [:alive] from comment #11)
> When testing with payment, there is another bug:
> 
> STR:
> - Open the browser and load
> http://people.mozilla.com/~kmcmillan/gaia-pay.html
> - Click on 'Pay PMPP'.
> - Once trusted UI is shown with the pay flow, close the dialog by clicking
> on the HOME button.
> - Open the UITests app and choose the 'navigator.mozPay' tests.
> - Click on 'Pay PMPP'.
> - Once trusted UI is shown with the pay flow, close the dialog by clicking
> on the HOME button.
> - Open the browser app again, which should show the trusted UI again.
> - Click on 'Success'.
> - Click on 'Pay PMPP' again in the browser page.
> 
> Expected:
> Trusted UI shows on homescreen
> 
> Actual:
> Trusted UI shows on browser app
> 
> Log:
> 
> E/GeckoConsole(  108): Content JS LOG at
> app://system.gaiamobile.org/js/trusted_ui.js:80 in trui_open:
> open-payment-flow-dialog-{3cab09cf-758f-4986-8549-8e992893f503}
> app://browser.gaiamobile.org =======
> I/Gecko   (  108): ======================= payment.js
> ======================= 
> E/GeckoConsole(  108): Content JS LOG at
> app://system.gaiamobile.org/js/trusted_ui.js:80 in trui_open:
> open-payment-flow-dialog-{c2169b1a-b3df-4ef2-be02-264224a93cad}
> app://uitest.gaiamobile.org =======
> I/Gecko   (  108): ======================= payment.js
> ======================= 
> E/GeckoConsole(  108): Content JS LOG at
> app://system.gaiamobile.org/js/trusted_ui.js:98 in trui_close:
> open-payment-flow-dialog-{c2169b1a-b3df-4ef2-be02-264224a93cad}
> app://browser.gaiamobile.org =======
> 
> Note that the id returned by close payment in browser just gives the chrome
> id of ui-test got...this is totally wrong. This is related to Bug 828182 I
> think.

You are right Alive. Thanks for catching this!

I failed reviewing this code, we always take the latest chrome event Id when calling the TrustedUIManager.close() function from the payment implementation.

I am afraid that we might need to match the trusted dialogs with DOMRequest ids, instead of chrome event ids :\... I'll file a bug for this.

	Jed Parsons (use needinfo, please) [:jedp, :jparsons]
	Comment 15 • 12 years ago

Thanks, Fernando and Alive.  That was my bug in the way chromeEventIds were being used.

I like the association with the DOM request ID very much.  I think it makes the logic of the flow much more transparent.  It also makes me wonder whether we need the chromeEventIds at all.  Seems like updating payment and identity callers to use the requestId for messaging and clean-up functions would make things easier to understand and maintain.

	Alex Keybl [:akeybl]
	Comment 16 • 11 years ago

Tracking for uplift consideration to v1.x if a low risk fix is found.

	BMO Automation
	Comment 17 • 6 years ago

Firefox OS is not being worked on