1033921 - [NFC] Shrinking UI incorrectly when using swipe gesture to change apps

Status: VERIFIED FIXED

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

Description

[Alison Shiue]

Gaia      90605754e9bdbe20f3999522f9e1aec600c422a4
Gecko     https://hg.mozilla.org/releases/mozilla-aurora/rev/0bffc7e5d8a2
BuildID   20140702160207
Version   32.0a2

STR:
1. Open multiple apps which can NFC share
2. Using swipe gesture to change apps
3. Tap 2 phones together and check shrinking UI (http://youtu.be/624rOi1xtgU)
4. Open another app which cannot NFC share
5. Using swipe gesture to change to an app which can NFC share
6. Tap 2 phones together and check shrinking UI (http://youtu.be/qpHW_sgbMoA)

Expected result:
1. Step 3 should show shrinking UI correctly
2. Step 6 should show shrinking UI correctly

Actual result:
1. Step 3, shrinking UI would show the final launch app screen
2. Step 6 could not show shrinking UI

Comment 1

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

Should we block swipe in NFC pairing in v2.0 release?
Or we just need to cancel the pairing and clear the state after swiping?

Comment 2

[Stephany Wilkes]

Flagging Gordon on this as I try not overload Francis while Jacqueline, Peter and Casey are out. 

I am not sure this is a blocker: does the state just never clear, making the screen "stuck"?

Comment 3

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

Triage: +'ing, functional breakage.

Comment 4

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

Assign to Greg for investigation.

Comment 5

[[e-mail]]

take it

Comment 6

[[e-mail]]

Can be reproduced.

The shrinking UI always recognize the rightest sheet instead of the current sheet.

Comment 7

[[e-mail]]

shrinking UI maintain it's own app stack. It will call `_switchTo` function while get `appopen` event. So it will always recognize the rightest sheet if edge gesture did not popped this event while swiping between apps.

Comment 8

[[e-mail]]

Attachment: pull request redirect to github

emit 'appopen' event when stack manager did goPrev/goNext operation.

Comment 9

[[e-mail]]

more explaination:

`shrinking UI` maintain it's own app stack and depends on `appopen` event to switch to current app.
`edge gesture` use `stack manager` to handle goPrev/goNext operation, but it does not notify others that the current app is changed, so the module maintain its own app stack will not get update.

Comment 10

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

Comment on attachment 8456607 [details] [review]
pull request redirect to github

1. Does UX confirm to enable swipe navigation during NFC pairing?
2. StackManager should not dispatch any app* event for appWindow.
3. appopen is fired each time the swipe done in _handle_swipein().

Comment 11

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

(In reply to Fred Lin [:gasolin] from comment #9)
> more explaination:
> 
> `shrinking UI` maintain it's own app stack and depends on `appopen` event to
> switch to current app.
> `edge gesture` use `stack manager` to handle goPrev/goNext operation, but it
> does not notify others that the current app is changed, so the module
> maintain its own app stack will not get update.

The broadcast is to enable a nice continuous swipe so the real opened event is delayed after 800ms.
What we could do (if UX wants) is to call appIn.publish('opening') or a new event when it goes in the queue.

Comment 12

[[e-mail]]

The issue is about NFC pairing AFTER doing swipe. So either remain/disable swipe navigation during NFC pairing is not the case. 
(It looks fine that shinking UI won't overlap the screen while swiping)

Comment 13

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

(In reply to Alive Kuo [:alive][NEEDINFO!][Berlin 7/14-7/18] from comment #11)
> (In reply to Fred Lin [:gasolin] from comment #9)
> > more explaination:
> > 
> > `shrinking UI` maintain it's own app stack and depends on `appopen` event to
> > switch to current app.
> > `edge gesture` use `stack manager` to handle goPrev/goNext operation, but it
> > does not notify others that the current app is changed, so the module
> > maintain its own app stack will not get update.
> 
> The broadcast is to enable a nice continuous swipe so the real opened event
> is delayed after 800ms.
> What we could do (if UX wants) is to call appIn.publish('opening') or a new
> event when it goes in the queue.

Fine, so the problem is StackManager is asynchronous with AppWindowManager.
There's 800ms delay that StackManager has different active app info from AppWindowManager after each swipe. This is problematic.

Since queueBroadcast calls queueShow/queueHide in AppWindow, it makes sense to publish 'willbeactive' and 'willbeinactive' inside queueShow/queueHide and then ask AppWindowManager or ShrinkingUI to update active app info.

Comment 14

[[e-mail]]

Comment on attachment 8456607 [details] [review]
pull request redirect to github

published 'willbeactive' and 'willbeinactive' inside queueShow/queueHide and then ask ShrinkingUI to update active app info.

Test with open gallery(NFC), browser(NFC), settings(non NFC) app and works fine

please kindly review it again.

Comment 15

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

Comment on attachment 8456607 [details] [review]
pull request redirect to github

r+ only with tests.

Comment 16

[[e-mail]]

Thanks for review.

add shrink UI test with willbeactive case. wait for TBPL pass to land.

Comment 17

[[e-mail]]

merged to gaia master https://github.com/mozilla-b2g/gaia/commit/dd8a9e53a9e519e8875fd3fe1a25eec5a1ad9486

thanks!

Comment 18

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/14571b8b75b140d15bb96dafabda83508f97b287

Comment 19

[Alison Shiue]

Verified on
Gaia      aa4f795b81c6147d67c4f06009e166debcf8856e
Gecko     https://hg.mozilla.org/releases/mozilla-aurora/rev/0ec0b9ac39f0
BuildID   20140716160201
Version   32.0a2

Comment 20

[Sue]

Attachment: verify_video.MP4

This issue has been verified successfully on Flame v2.1
See attachment: verify_video.MP4
Reproducing rate: 0/5
Flame 2.1 versions:
Gaia-Rev        5655269098c7e82254e56933f1af05b4abe2a2f3
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/86608c9389b5
Build-ID        20141204001201
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141204.034958
FW-Date         Thu Dec  4 03:50:09 EST 2014
Bootloader      L1TC00011880

Comment 21

[Sue]

This issue has been verified successfully on Flame v2.0
See attachment: verify_video.MP4
Reproducing rate: 0/5
Flame 2.0 versions:
Gaia-Rev        8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/ff1100ba2ab8
Build-ID        20141204000228
Version         32.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141204.040425
FW-Date         Thu Dec  4 04:04:36 EST 2014
Bootloader      L1TC00011880