Anshul Reporter
	Description • 10 years ago

[Blocking Requested - why for this release]: This error is blocking our tests for STK that we run to ensure the quality of the release.

I am seeing the following exception every now and then when running STK related marionette test cases JavascriptException: TypeError: evt is null at: app://system.gaiamobile.org/js/modal_dialog.js line: 250 causing our tests to fail. There is no specific test that is causing this issue so I don't have STR.

	bhavana bajaj [:bajaj]
	Comment 1 • 10 years ago

Ni :frsela, to get started on investigation here. :frsela, can you please help with this or direct it to the right folks ?

	Gregor Wagner [:gwagner]
	Comment 2 • 10 years ago

Also adding alive here since he knows best about modal_dialog.js

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

Could you please add |console.trace()| at https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/modal_dialog.js#L248 and tell me what's the call path?

I cannot help since you didn't specify the STR.

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

(In reply to Anshul from comment #4)
> Created attachment 8505664 [details]
> log with console.trace()

Hmm, so the ModalDialog tries to hide itself "whenever" an appWindow is closing and the app somehow triggers system app to use ModalDialog (evt.detail.origin === this.currentOrigin).

Could you do more investigation and find out how ModalDialog is called? Maybe another logging patch for Anshul.

	Andrew Overholt [:overholt]
	Comment 6 • 10 years ago

Blocking 2.1 CC = 2.1+.

	(Inactive after June) George Duan [:gduan] [:喬智] Assignee
	Comment 7 • 10 years ago

From my understanding, currentOrigin should be system or null, no other possible value. And appwillclose should not called with system ... so I guess, there might be a app that has Null origin. If that so, we should try to filter it in modal_dialog.js. But I still need to verify my assumption.


1. May I take a look for test script?

2. Could you add below commit and provide me the log?
https://github.com/cctuan/gaia/commit/dc16e6ae376890170dc0c1e25cf5d3434479790e

Thanks.

	Fernando R. Sela (no CC, needinfo please) [:frsela]
	Comment 8 • 10 years ago

(In reply to bhavana bajaj [:bajaj](On PTO 10/20) from comment #1)
> Ni :frsela, to get started on investigation here. :frsela, can you please
> help with this or direct it to the right folks ?

From my understanding this is now under investigation, so cleaning NI.
Feel free to ni=? to me again if needed.

	Anshul Reporter
	Comment 9 • 10 years ago

(In reply to George Duan [:gduan] [:喬智] from comment #7)
> From my understanding, currentOrigin should be system or null, no other
> possible value. And appwillclose should not called with system ... so I
> guess, there might be a app that has Null origin. If that so, we should try
> to filter it in modal_dialog.js. But I still need to verify my assumption.
> 
> 
> 1. May I take a look for test script?
> 
> 2. Could you add below commit and provide me the log?
> https://github.com/cctuan/gaia/commit/
> dc16e6ae376890170dc0c1e25cf5d3434479790e
> 
> Thanks.

Please find the log with your gaia commit attached.

	(Inactive after June) George Duan [:gduan] [:喬智] Assignee
	Comment 11 • 10 years ago

Thanks Anshul!

The interesting thing is, the appwillclose event has undefined origin which should not happen.
I try to put my fix in https://github.com/mozilla-b2g/gaia/pull/25357 . 
Could you test it again?

	Anshul Reporter
	Comment 12 • 10 years ago

(In reply to George Duan [:gduan] [:喬智] from comment #11)
> Thanks Anshul!
> 
> The interesting thing is, the appwillclose event has undefined origin which
> should not happen.
> I try to put my fix in https://github.com/mozilla-b2g/gaia/pull/25357 . 
> Could you test it again?
This patch does solve the issue. Thank you.

	(Inactive after June) George Duan [:gduan] [:喬智] Assignee
	Comment 13 • 10 years ago

Hi Alive,
could you help to review this patch? Thanks.

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

Comment on attachment 8509998 [details]
PR to 2.1

Could you explain why this fixes it?

	(Inactive after June) George Duan [:gduan] [:喬智] Assignee
	Comment 15 • 10 years ago

When app is launched through browser.js, the origin may be undefined. And it might cause this bug when ModalDialog.origin is still not defined yet and call hide. I think we should use use openwindow event and let browser_config_helper generate required config for appWindow.

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

Comment on attachment 8509998 [details]
PR to 2.1

I see, but I think you need to modify browser_test.js

	(Inactive after June) George Duan [:gduan] [:喬智] Assignee
	Comment 17 • 10 years ago

Comment on attachment 8509998 [details]
PR to 2.1

Hi Alive,
test is updated. could you check again? Thanks.

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

Comment on attachment 8509998 [details]
PR to 2.1

Please have a master patch as well.

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

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #18)
> Comment on attachment 8509998 [details]
> PR to 2.1
> 
> Please have a master patch as well.

...if needed.

	(Inactive after June) George Duan [:gduan] [:喬智] Assignee
	Comment 20 • 10 years ago

Comment on attachment 8509998 [details]
PR to 2.1

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): No.
[User impact] if declined: This error is blocking STK related test as comment 0.
[Testing completed]: Yes.
[Risk to taking this patch] (and alternatives if risky): None.
[String changes made]:

	(Inactive after June) George Duan [:gduan] [:喬智] Assignee
	Comment 21 • 10 years ago

No need for master, it has different approach to make sure origin not null.

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #19)
> (In reply to Alive Kuo [:alive][NEEDINFO!] from comment #18)
> > Comment on attachment 8509998 [details]
> > PR to 2.1
> > 
> > Please have a master patch as well.
> 
> ...if needed.

	(Inactive after June) George Duan [:gduan] [:喬智] Assignee
	Comment 22 • 10 years ago

PR to 2.1:
https://github.com/mozilla-b2g/gaia/commit/f8378fe1c7f128a3b679201b469a82fca2371828

	Yeojin Chung [:YeojinC]
	Comment 23 • 10 years ago

Unable to verify as it is a back-end issue.