1021658 - Open an YouTube link from Facebook and then go to task manager stuck the phone

Status: VERIFIED FIXED

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

Description

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

STR:
1. Install and logic facebook
2. Click an youtube link in facebook
3. When youtube page is launched, long press home button

Expected: TaskManager is shown

Actual: facebook is opened again and you cannot back to homescreen anymore.

Comment 1

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

The cause shall be: if there are two consecutive window.open request, the first opened window will be killed during it's transitioning which causes the window opener to be opened again, and thus we will have two transition-state="opened" window at the same time.

Comment 2

[Jason Smith [:jsmith]]

QA Wanted to confirm this works on 1.4.

Comment 3

[Cody Roesch [:croesch]]

This bug does NOT occur on v1.4 Flame with the following variables. When tapping on a youtube link in the facebook wall then holding down the home button, the task manager is presented as expected.

Environmental Variables:
Device: Flame 1.4
Build ID: 20140609000201
Gaia: 8b239e41bbd85aa7b6a2c5d388e775ba7de6fb2b
Gecko: e3f85877db29
Version: 30.0 (1.4) 
Firmware Version: v10G-2

Comment 4

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

Should be regressed from bug 916709.

Comment 5

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

Attachment: https://github.com/mozilla-b2g/gaia/pull/20270

It seems facebook is using window.open to open the youtube link,
and there will be two requests to the same link, the second one happens during the first one and caller is transitioning.

I am not sure if we should change window.open to default behavior (open popup instead of sheet) but this patch could fix this issue. What do you think?

Comment 6

[Etienne Segonzac (:etienne)]

Comment on attachment 8437513 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20270

r+ because this is definitely fixing the bug (and is well tested).

But I'd like ux-feedback on what should happen here?

Facebook is doing a window.open on the same origin (m.facebook.com) which then redirects to youtube.
We will open a new sheet for the facebook app (since it's the same origin), and the user can swipe back to the facebook timeline.

But swiping back will be the *only* way to get back to the facebook timeline (since youtube doesn't present chrome for this action).

Alternatively we could blacklist (:/) facebook from opening new sheets, this would cause the window.open redirecting to youtube to open as a separate app. The card view would display facebook and youtube separately, instead of displaying them as one app (with 2 sheets).

Comment 7

[Etienne Segonzac (:etienne)]

Better than the blacklist, suggested by Vivien, we could safely disable child windows for hosted apps in 2.0.

Comment 8

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

(In reply to Etienne Segonzac (:etienne) from comment #7)
> Better than the blacklist, suggested by Vivien, we could safely disable
> child windows for hosted apps in 2.0.

Would you like to open another bug for this solution or wanna me to disable it in this bug?

Comment 9

[Etienne Segonzac (:etienne)]

Let's do the safe no child windows for hosted apps here this way we uplift only once :)

Comment 10

[Stephany Wilkes]

Flagging Jacqueline on this.

Comment 11

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

(In reply to Etienne Segonzac (:etienne) from comment #9)
> Let's do the safe no child windows for hosted apps here this way we uplift
> only once :)

For "hosted" do you mean the app without manifest?

Comment 12

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

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #11)
> (In reply to Etienne Segonzac (:etienne) from comment #9)
> > Let's do the safe no child windows for hosted apps here this way we uplift
> > only once :)
> 
> For "hosted" do you mean the app without manifest?

Well is this case facebook is having manifest. And I guess we cannot distinguish by http or app because in firefox nightly they are all http. So I proposed to disable all inner sheets here and turn on for HAIDA=1.

Comment 13

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

Attachment: https://github.com/mozilla-b2g/gaia/pull/20408

v2: Pref off in app sheets.

I end up doing this because I don't know how to distinguish hosted app and even we only turn it off for hosted app, I think we will have the same problems for packaged app who uses window.open()

Before we have solutions for how to open inner sheet (is window.open the final solution?) and how to deal with location change inner sheet, I'd rather turn it off.

Comment 14

[Jacqueline Savory [:jsavory] UX]

I think this one is more of a sheets/task manager question. Passing ni? to Rob and Francis who have been working on these features.

Comment 15

[Etienne Segonzac (:etienne)]

Comment on attachment 8438959 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/20408

r=me with the tiny comments on github addressed.

Comment 16

[Rob MacDonald [:robmac]]

I just reviewed the patch and, if it is what I think it is, it's a more graceful solution. I've set up a meeting with Etienne to confirm on Monday afternoon Paris time to discuss and will comment on this bug  afterwards. I just want to be sure that I'm aware of all the implications. :)

Thanks!

Comment 18

[Rob MacDonald [:robmac]]

I met with Etienne and Francis this morning and we feel this patch addresses the issues well. Does this patch impact only Facebook?

Comment 19

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

(In reply to Rob MacDonald [:robmac] from comment #18)
> I met with Etienne and Francis this morning and we feel this patch addresses
> the issues well. Does this patch impact only Facebook?

Every app uses window.open will be affected. Exactly the change in bug 961800 is pref-off by this change.

Comment 20

[Francis Djabri [:djabber]]

Clearing my ni as Rob is tracking this

Comment 22

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

master https://github.com/mozilla-b2g/gaia/commit/12cdb71ea937e6baeb277694a2ed3a483af9cc39

Rob we could open another bug to discuss what's the wanted behavior if we have child window in the future. Closing.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/25b907600d3a31ee060f3bbec5a0b0071495f442

Comment 24

[sarsenyev]

Verified the issue is fixed on 2.1 and 2.0 Flame builds

Phone is no longer stuck when switching to task manager from YouTube video on Facebook

Device: Flame 2.1 KK
BuildID: 20141103001220
Gaia: 027a7de0c95320cea0579bfd1a4ceef3e9038f34
Gecko: ffecb2be228b
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 34.0 (2.1)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.0 KK
BuildID: 20141103000201
Gaia: 7b8df9941700c1f6d6d51ff464f0c8ae32008cd2
Gecko: 82a6ed695964
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 32.0 (2.0)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0