TypeError: evt is null at: app://system.gaiamobile.org/js/modal_dialog.js line: 250

RESOLVED FIXED in Firefox OS v2.1

Status

Firefox OS
Gaia::System
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Anshul, Assigned: gduan)

Tracking

unspecified
2.1 S7 (24Oct)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:2.1+, b2g-v2.1 fixed, b2g-v2.2 fixed)

Details

(Whiteboard: [caf priority: p2][CR 741214])

Attachments

(3 attachments)

(Reporter)

Description

4 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.
Ni :frsela, to get started on investigation here. :frsela, can you please help with this or direct it to the right folks ?
Flags: needinfo?(frsela)
Also adding alive here since he knows best about modal_dialog.js
Flags: needinfo?(alive)
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.
Flags: needinfo?(alive)
(Reporter)

Comment 4

4 years ago
Created attachment 8505664 [details]
log with console.trace()
(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.
Flags: needinfo?(gduan)
Blocking 2.1 CC = 2.1+.
blocking-b2g: 2.1? → 2.1+

Updated

4 years ago
Whiteboard: [CR 741214]

Updated

4 years ago
Whiteboard: [CR 741214] → [caf priority: p2][CR 741214]
Assignee: nobody → gduan
Flags: needinfo?(gduan)
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.
Flags: needinfo?(anshulj)
(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.
Flags: needinfo?(frsela)
(Reporter)

Comment 9

4 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.
Flags: needinfo?(anshulj)
(Reporter)

Comment 10

4 years ago
Created attachment 8508334 [details]
log with George's test patch
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?
Flags: needinfo?(anshulj)
(Reporter)

Comment 12

4 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.
Flags: needinfo?(anshulj)
Created attachment 8509998 [details] [review]
PR to 2.1

Hi Alive,
could you help to review this patch? Thanks.
Attachment #8509998 - Flags: review?(alive)
Comment on attachment 8509998 [details] [review]
PR to 2.1

Could you explain why this fixes it?
Flags: needinfo?(gduan)
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.
Flags: needinfo?(gduan) → needinfo?(alive)
Comment on attachment 8509998 [details] [review]
PR to 2.1

I see, but I think you need to modify browser_test.js
Flags: needinfo?(alive)
Attachment #8509998 - Flags: review?(alive) → feedback+
Comment on attachment 8509998 [details] [review]
PR to 2.1

Hi Alive,
test is updated. could you check again? Thanks.
Attachment #8509998 - Flags: review?(alive)
Comment on attachment 8509998 [details] [review]
PR to 2.1

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

...if needed.
Comment on attachment 8509998 [details] [review]
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]:
Attachment #8509998 - Flags: approval-gaia-v2.1?
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] [review]
> > PR to 2.1
> > 
> > Please have a master patch as well.
> 
> ...if needed.

Updated

4 years ago
Attachment #8509998 - Flags: approval-gaia-v2.1? → approval-gaia-v2.1+
PR to 2.1:
https://github.com/mozilla-b2g/gaia/commit/f8378fe1c7f128a3b679201b469a82fca2371828
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
status-b2g-v2.1: --- → fixed
status-b2g-v2.2: --- → fixed
Target Milestone: --- → 2.1 S7 (24Oct)
Unable to verify as it is a back-end issue.
QA Whiteboard: [QAnalyst-Triage?][QAnalyst-verify-]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?][QAnalyst-verify-] → [QAnalyst-Triage+][QAnalyst-verify-]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.