1238887 - [TV][2.5] Browser_contextmenu is not killed correctly when its app window is killed

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::TV::System, defect, P1)

Description

[Josh Cheng [:josh]]

We implemented a kill() function but app_window calls destory() to remove its submodule. Need to figure out which function name is correct.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] luke-chang:1238887_exit_on_contextmenu > mozilla-b2g:master

Comment 2

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8707321 [details] [review]
[gaia] luke-chang:1238887_exit_on_contextmenu > mozilla-b2g:master

Hi Rex,

I noticed the "destroy" function is implemented in BaseUI from which BrowserContextMenu inherits but the "kill" function of BrowserContextMenu seems not to be called anywhere. So we decide to rename "kill" to "_destroy" in BrowserContextMenu (and AppModalDialog as well) and make it be called in the "destroy" function in BaseUI.

After patching like above, the sub modules can be destructed correctly but I encountered a longstanding bug in FocusManager due to some racing conditions. So I have to fix it as well.

Please take a look at this patch. Thanks in advance.

Comment 3

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Comment on attachment 8707321 [details] [review]
[gaia] luke-chang:1238887_exit_on_contextmenu > mozilla-b2g:master

Looks good to me.
AppWindow has kill() but seems submodules don't need that?

Comment 4

[Luke Chang [:lchang] (inactive)]

Rex, Thanks and yes, it affects sub-modules only.


tests passed: https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=8c256c8c73d57b0401de2c90b11dec1ccaff3cf9

landed on master: https://github.com/mozilla-b2g/gaia/commit/87fb9a4c60793fad569894bd888cfbe0d36e294f

Comment 5

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8707321 [details] [review]
[gaia] luke-chang:1238887_exit_on_contextmenu > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): 2.5 feature broken
[User impact] if declined: there's a javascript error that causes context menu broken.
[Testing completed]: yes.
[Risk to taking this patch] (and alternatives if risky): low.
[String changes made]: N/A

Comment 6

[Josh Cheng [:josh]]

Comment on attachment 8707321 [details] [review]
[gaia] luke-chang:1238887_exit_on_contextmenu > mozilla-b2g:master

Approve for TV 2.5

Comment 7

[Carsten Book [:Tomcat]]

for 2.5 https://github.com/mozilla-b2g/gaia/commit/a6df69c4658ee8a32cefcd2362d280751e13a0bc