1111799 - SHB not working with browser in landscape mode

Status: VERIFIED FIXED

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

Description

[Gregor Wagner [:gwagner]]

STR: Hold device in landscape mode
Open bookmarked link on homescreen
scroll up and down to collapse and extend rocketbar

SHB stops working.

It starts working again when phone is rotated to portrait mode.

Comment 1

[Alberto Pastor [:albertopq]]

I cannot repro with:

Gaia-Rev        4353228b849597408f79e16ed3ade7df02c6b45a
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/8401afdb6e6c
Build-ID        20141216160204
Version         37.0a1
Device-Name     flame

Flagging qawanted to see if they can.

Comment 2

[Duane Dixon [:ddixon]]

Branch Check

Actual Results: 
SHB stops functioning in landscape orientation when user is in Browser app. 

Issue DOES occur in Flame 2.2, 2.1 (shallow flash, engineering, 319 MB memory). 

Device: Flame 2.2
BuildID: 20141217035735
Gaia: d22dfece04fc00457e8369c660c11f945b088d2f
Gecko: cb8ad2251c09
Version: 37.0a1 (2.2)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
--------------------------------------------------
Device: Flame 2.1
BuildID: 20141216151310
Gaia: 14315733e2d265a42f9ab02d1aba191789870f70
Gecko: ddecea83ce6e
Version: 34.0 (2.1) 
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
--------------------------------------------------
--------------------------------------------------
Issue DOES NOT occur in Flame 2.0 (shallow flash, engineering, 319 MB memory). 

Device: Flame 2.0
BuildID: 20141216151108
Gaia: d04710d5d643eeff5a6493aef92a1af672a2769c
Gecko: 4d62570b77e4
Version: 32.0 (2.0) 
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 3

[Alberto Pastor [:albertopq]]

I finally repro it with a slightly different STR:

1.- Go to browser and add a web to the homescreen
2.- Go to homescreen and click on the recently added bookmark (in landscape mode)
3.- Scroll up and down
4.- Click SHB (it works)
5.- Click again on the same bookmark (still landscape)

SHB stops working

Comment 4

[Alberto Pastor [:albertopq]]

Tricky one..

First of all, every time the bug happens, there is an error in the console thrown by:
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/edge_swipe_detector.js#L183

That is happening because StackManager.getCurrent() returns undefined. Comparing the events received in the stack_manager, we are missing the 'appopening' event every time the bug happens.

The root cause of not receiving the 'appopening' event when opening the app in landscape mode is:

https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window_manager.js#L274

I'm sure there is a good reason for opening the app with a 'immediate' transition (which makes lose the appopening event) when we are in landscape mode, but I don't have enough context.

I see different solutions here, but not sure which one is the best as probably there were reasons for doing it in the way it is right now:

1.- Use appopened event on stack_manager instead appopening
2.- Fix the transitions when closing/opening apps in landscape mode. Note that there are not animations currently. If we open/close apps on landscape mode, it just shows/hides the app without animation

My impression is that 1 is the short term workaround to easily fix this, and 2 a more long term solution, but I would like to hear :alive's opinion here.

Thanks!

Comment 5

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

(In reply to Alberto Pastor [:albertopq] from comment #4)
> 
> 1.- Use appopened event on stack_manager instead appopening

There is a good reason to not use appopened, I ran into this trap before but I forgot it.
I will leave this issue to Etienne.

> 2.- Fix the transitions when closing/opening apps in landscape mode. Note
> that there are not animations currently. If we open/close apps on landscape
> mode, it just shows/hides the app without animation

Unfortunately this is big work. Implement portrait to landscape animation is tracking here: bug 985843

> 
> My impression is that 1 is the short term workaround to easily fix this, and
> 2 a more long term solution, but I would like to hear :alive's opinion here.
> 
> Thanks!

Good investigation!

Comment 6

[Etienne Segonzac (:etienne)]

What a mess... Thanks Alberto for working through this!

(In reply to Alberto Pastor [:albertopq] from comment #4)
> 1.- Use appopened event on stack_manager instead appopening
> 2.- Fix the transitions when closing/opening apps in landscape mode. Note
> that there are not animations currently. If we open/close apps on landscape
> mode, it just shows/hides the app without animation
> 
> My impression is that 1 is the short term workaround to easily fix this, and
> 2 a more long term solution, but I would like to hear :alive's opinion here.
> 

Yes we should definitely go with 1, since 2 will probably also require gecko work (currently when going back to the homescreen from a landscape app the compositor is frozen for 1sec...).

Like you said, the easy fix for 1. should be to listen to appopened instead, and it fits perfectly with the change in bug 1100341 where the edge_swipe_detector is going to wait for appopened too.

So my recommendation is to pay attention to the event sequence of launchapp/appcreated/appopen{ing,ed} and try to make the change :)

Comment 7

[GitHub Autolander Bot]

Attachment: [PullReq] albertopq:1111799-SHB-landscape to mozilla-b2g:master

Comment 8

[Etienne Segonzac (:etienne)]

Comment on attachment 8540381 [details] [review]
[PullReq] albertopq:1111799-SHB-landscape to mozilla-b2g:master

Cool, that's definitely the safest solution :)

Comment 9

[Alberto Pastor [:albertopq]]

master:https://github.com/mozilla-b2g/gaia/commit/0c439312ad0c49f11330cfd273474b8a58451281

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Is this something that should be uplifted to v2.1? If so, please nominate it for Gaia v2.1 approval :)

Comment 11

[Alberto Pastor [:albertopq]]

I think we should. Gregor?

Comment 12

[Gregor Wagner [:gwagner]]

Yes, SHB is a 2.1 feature. Please request uplift approval.

Comment 13

[Alberto Pastor [:albertopq]]

Comment on attachment 8540381 [details] [review]
[PullReq] albertopq:1111799-SHB-landscape to mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): SHB feature
[User impact] if declined: SHB stops working when opening apps in landscape mode (so you can't leave the app). It works again when rotating back to portrait.
[Testing completed]: Added unit tests
[Risk to taking this patch] (and alternatives if risky): Is only changing the event we are using (appopened instead of appopening) and it has been a while in 2.2 without regressions. I think is reasonably low risk.
[String changes made]: -

Comment 14

[bhavana bajaj [:bajaj]]

Comment on attachment 8540381 [details] [review]
[PullReq] albertopq:1111799-SHB-landscape to mozilla-b2g:master

Busted common use-case for a new 2.1 feature, approving for uplift.

Requesting QA verification once this lands.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/37a186687b0f18b67c085668cfc36dafced3cfb4

Comment 16

[Elie]

Attachment: VIDEO0239_Compress.MP4

This issue verified successfully on flame 2.2.
Gaia-Rev        7c5b27cad370db377b18a742d3f3fdb0070e899f
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/748b20315f75
Build-ID        20150113162504
Version         37.0a2

Comment 17

[Elie]

This issue dose not exist on Flame 2.1
Gaia-Rev        6957ac8a322234ec99c8abb7cc18dc6a2e0176db
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/6600eba54256
Build-ID        20150114001300
Version         34.0
Reproduce rate   0/3

Comment 18

[Vincent Liu[:vliu]]

v2.1s : https://github.com/mozilla-b2g/gaia/commit/37a186687b0f18b67c085668cfc36dafced3cfb4