1017892 - visibility change not firing for window.open

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

Description

[(not reading bugmail) Nick Desaulniers [:\n]]

Attachment: test_case.zip

See attached apps.

STR:

1. Open index.html in desktop FF.
2. Click link
3. close tab that was just opened.
4. Notice two visibiltychange events fired (one for window.open, one for navigating back).
5. Add app to App manager and push to phone (though hosting locally and navigating to your ip addr should work).
6. Click link
7. Close window.

Expected:
two visibility changes.

Actual:
zero visibility changes.

ignore the rAF code.  FxOS 1.4 git commit info 2014-05-09

Comment 1

[Erin Lancaster [:elan]]

Triage team: Nick has isolated this bug as the cause of 2 instances of unexpected behavior on a game we're working on for 1.4.

Comment 2

[Jason Smith [:jsmith]]

Marking qawanted to check 1.3.

Comment 3

[Lionel Mauritson]

Testing on a 1.3 Buri, the result is the same as on 1.4. 
The two lines of text do not appear when returning to the page with the link from the other page.

However, if the device is left alone on the link page and falls asleep the two lines of text will appear after unlocking the device. (This occurs on both 1.4 and 1.3)

Device: Buri
Build ID: 20140602024006
Gaia: 5bd226b03a2d63dfe9df204f7c0afb9984e8fd42
Gecko: 14b755c65af8
Version: 28.0 (1.3) 
Firmware Version: v1.2device.cfg

Comment 4

[Jason Smith [:jsmith]]

In the partner app in question here, can this problem be worked around?

Comment 5

[(not reading bugmail) Nick Desaulniers [:\n]]

Even if we could stop the game loop upon window.open calls, we still cannot know when the opened window is closed.  So we could figure out when we become "invisible" programmatically, but we cannot programatically detect when we become visible again.

Some idea's for a work around:
```javascript
pauseGameEngine();
var a = window.open();
a.onunload = unpauseGameEngine;
//or
a.document.onvisibilitychange = unpauseGameEngine;
```

I will work on a test case to verify one of these in FxOS.

Comment 6

[(not reading bugmail) Nick Desaulniers [:\n]]

The first idea didn't work: https://bugzilla.mozilla.org/show_bug.cgi?id=1020621

Comment 7

[Preeti Raghunath(:Preeti)]

QA,

Please verify me and lets test this before landing.

Comment 8

[Jason Smith [:jsmith]]

Note - add verifyme when the patch is ready here.

Comment 9

[Preeti Raghunath(:Preeti)]

Erin,

Can we please make sure this gets tested prior to landing on 1.4?

Comment 10

[Erin Lancaster [:elan]]

QA is happy to test a try build once a fix/patch is available but first we need an assignee. Any ideas?

Comment 11

[Preeti Raghunath(:Preeti)]

Tim

Can you please review from Window Management?

Comment 12

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Sadly this is yet another result of we overloading setVisible().

Alive, is this intentional? I suspect we defer or not calling setVisible() for pop-ups because of transition.


(In reply to Preeti Raghunath(:Preeti) from comment #9)
> Erin,
> 
> Can we please make sure this gets tested prior to landing on 1.4?

I would like to revisit blocking 1.4 decision here since we shipped with this behavior on 1.3, or maybe even 1.2. What prompts the block? Can this block 2.0 instead? Are we blocking random releases based on what's currently inflight?

Comment 13

[Tim Guan-tin Chien [:timdream] (please needinfo)]

qawanted to verify 1.3 behavior.

Comment 14

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #12)
> Sadly this is yet another result of we overloading setVisible().
> 
> Alive, is this intentional? I suspect we defer or not calling setVisible()
> for pop-ups because of transition.
> 

For popup we don't want to kill the popup opener so we never send the popup to background.
For the use case described in comment 5, since bug 1020621 thinks this is not a platform bug, I don't think gaia could do anything to move this forward.

Comment 15

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #14)
> For popup we don't want to kill the popup opener so we never send the popup
> to background.

... you mean "we never send the popup 'opener' to background"?

Would you point us the bug that add this behavior?

Comment 16

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #15)
> (In reply to Alive Kuo [:alive][NEEDINFO!] from comment #14)
> > For popup we don't want to kill the popup opener so we never send the popup
> > to background.
> 
> ... you mean "we never send the popup 'opener' to background"?

yes. opener who opens the window.

> 
> Would you point us the bug that add this behavior?

I think this is existing from v1.0 popup.

Comment 17

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Nick Desaulniers [:\n] from comment #5)
> Even if we could stop the game loop upon window.open calls, we still cannot
> know when the opened window is closed.  So we could figure out when we
> become "invisible" programmatically, but we cannot programatically detect
> when we become visible again.
> 
> Some idea's for a work around:
> ```javascript
> pauseGameEngine();
> var a = window.open();
> a.onunload = unpauseGameEngine;
> //or
> a.document.onvisibilitychange = unpauseGameEngine;
> ```
> 
> I will work on a test case to verify one of these in FxOS.

I just found a dirty workaround to know an opened window is closed.
http://jsfiddle.net/zuul/namuj/

Comment 18

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Given the fact, I am more confident to say we shouldn't block 1.4 for this bug. A proper fix would be to decouple visibility state with memory management here.

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #17)
> I just found a dirty workaround to know an opened window is closed.
> http://jsfiddle.net/zuul/namuj/

This is not a solution I am comfortable with, but that's what we have right now :'(

Comment 19

[Erin Lancaster [:elan]]

Nick, does Alive's suggestion look viable to you?

Comment 20

[Jason Smith [:jsmith]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #13)
> qawanted to verify 1.3 behavior.

comment 4 already clarified this is reproducible on 1.3.

Comment 21

[(not reading bugmail) Nick Desaulniers [:\n]]

The workaround does work.

Comment 22

[Erin Lancaster [:elan]]

Un-noming. Thank you, :timdream and :alive. We really appreciate the help with the workaround.

Comment 23

[Preeti Raghunath(:Preeti)]

Moving this to backlog for an better solution.

Comment 24

[(inactive after 6/18) Alive Kuo [:alive]]

The proper solution might be use the new setVisible browser API to tell gecko the opener should go to background with higher priority. It's still in planning so I don't have a platform bug handy.

Comment 25

[BMO Automation]

Firefox OS is not being worked on