1155873 - [NFC] 'Swipe to share' screen can reveal homescreen if returning to home from image share (from notification)

Status: VERIFIED FIXED

(Firefox OS Graveyard :: NFC, defect)

Description

[Oliver Nelson [:oliverthor]]

Attachment: logcat_20150417_1416.txt

Description:
If the user is sharing an image (viewed from a notification) through NFC (waiting on the 'Swipe to share screen') and taps the home button, they will observe the UI remaining on screen while the 'image' to share now reflects the homescreen. Sharing this will not produce a share request.

PreReq:
* two devices with NFC enabled
Repro Steps:
1) Update a Flame to 20150417010203
2) phone1: open gallery and select an image
3) phone1: set back to phone2 to initiate a NFC transfer
4) phone2: after receiving image, ensure you are homescreen
5) phone2: slide notification tray down, tap preview for image received
6) phone2: while viewing image from notification, set back to phone1 to initiate an NFC transfer
7) phone2: without moving phone, tap home button


Actual:
'Swipe to share' screen displays sending the homescreen within NFC transfer; does not a transfer a file if shared


Expected:
NFC transfer is canceled after returning to homescreen


Environmental Variables:
Device: Flame 3.0
Build ID: 20150417010203
Gaia: 3cd0a9facce26c2acc7be3755a17131a6358e33f
Gecko: 51e3cb11a258
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0



Repro frequency: 5/5
See attached: 
video- https://youtu.be/ir2nlWlo9xM
screenshot
logcat

Comment 1

[Oliver Nelson [:oliverthor]]

Attachment: swipe_to_share-homescreen.png

Comment 2

[Oliver Nelson [:oliverthor]]

Issue is blocked from appearing in 2.2 as NFC transfer appears to be broken: https://bugzilla.mozilla.org/show_bug.cgi?id=1155887

Comment 3

[Peter Bylenga [:PBylenga]]

Unknown if it's a regression.

NI on qa component owner for nomination decision and assignment.

Comment 4

[Alison Shiue]

Yes, I can reproduce this issue at v2.2.  We need to disable home button function when NFC detect while viewing image from notification.

Hi Fred, could you help check this issue? Thanks.

Comment 5

[Alison Shiue]

v2.1 also has this issue.
Skipped checking v2.0 branch since the function of sharing file via notification is from v2.1. (bug 1033964 comment 17)

Comment 6

[Alison Shiue]

(In reply to Alison Shiue from comment #5)
> v2.1 also has this issue.
> Skipped checking v2.0 branch since the function of sharing file via
> notification is from v2.1. (bug 1033964 comment 17)

Please ignore the previous comment, there is no issue on v2.1.

Comment 7

[[e-mail]]

I can reproduce it on master, it looks like the camera view activity in shrink UI does not block the home button.

I also try UI tests > API/view photo > view image, press home button in shrink UI can back to homescreen, but the home button should be disabled.

Comment 8

[[e-mail]]

After above experiment, I open the gallery app and entering the shrink UI, both home and holdhome works (which is expected not work). 

It looks like a regression?

Comment 9

[Alison Shiue]

Yes, I think this is a regression since 2.1 block both home and holdhome function when at shrinking UI.

Comment 10

[[e-mail]]

set regressionwindow-wanted, the code has not modified for a while,
I suggest check the window with 

5 Jan 2015
https://github.com/mozilla-b2g/gaia/commits/master/apps/system/js/stack_manager.js

or 

8 Oct 2014
https://github.com/mozilla-b2g/gaia/commits/master/shared/js/shrinking_ui.js

thanks for help.

Comment 11

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

triage: regression

Comment 12

[Alison Shiue]

Initial Regression Window: mozilla-central-flame

Last Working
Build ID               20141031061804
Gaia Revision          a07994714f0552f89801d6097982308d8b0a1ee1
Gaia Date              2014-10-30 23:54:58
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/6bd2071b373f
Gecko Version          36.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20141031.094040
Firmware Date          Fri Oct 31 09:40:50 EDT 2014
Bootloader             L1TC100118D0

First Broken
Build ID               20141031160202
Gaia Revision          5964f1339f37e7595aff7de7512b8529bc640b76
Gaia Date              2014-10-31 17:13:50
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/b695d9575654
Gecko Version          36.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20141031.192525
Firmware Date          Fri Oct 31 19:25:36 EDT 2014
Bootloader             L1TC100118D0

First broken Gecko and last working Gaia - issue does NOT reproduce, which means the issue came from Gaia.

Gaia pushlog:
https://github.com/mozilla-b2g/gaia/compare/a07994714f0552f89801d6097982308d8b0a1ee1...5964f1339f37e7595aff7de7512b8529bc640b76

* note: I cannot find more detail since b2g-inbound does not keep these early build.

Comment 13

[[e-mail]]

seems Bug 1050164 moving shrink_ui to shared/ might cause the regression.

George, could you help check if the shrink-ui's home/hold-home event works after moving to shared/.

Comment 14

[(Inactive after June) George Duan [:gduan] [:喬智]]

Yes, this is my regression, previously home eventlistener in shrinkingUI is registered earlier than appWindowManager but now it's not.

My suggestion is to check shrinkingUI.isActive in _handle_home and _handle_holdhome in appWindowManager, return false if it's true. And remove the original listener from shirking_ui.js.

Hi Alive, what do you think?

Comment 15

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

(In reply to George Duan [:gduan] [:喬智] from comment #14)
> Yes, this is my regression, previously home eventlistener in shrinkingUI is
> registered earlier than appWindowManager but now it's not.
> 
> My suggestion is to check shrinkingUI.isActive in _handle_home and
> _handle_holdhome in appWindowManager, return false if it's true. And remove
> the original listener from shirking_ui.js.
> 
> Hi Alive, what do you think?

1. Please remove the event stopImmediatePropagation() stuff in ShrinkingUI. This is not expected to happen after HierarchyManager.
2. "if" block home/holdhome is what we want, make shrinkingUI 'hierarchy-friendly' by passing home/holdhome button to it just like FtuLauncher if ShrinkingUI is managed by AppWindowManager.

Comment 16

[(Inactive after June) George Duan [:gduan] [:喬智]]

Hi Juwei,
What should it happen if user press home or holdhome while shrinkingUI is activated(tilting)?

I saw some old code behaves like that pressing home or holdhome would stay in the place instead of going to homescreen, but it doesn't work since some regression. I'd like to make sure what behaviour should be correct.

Comment 17

[bhavana bajaj [:bajaj]]

Passing it onto george as he seems to be investigating, please re-assign as needed.

Comment 18

[Juwei Huang[:juwei]]

Hi George,
In the beginning I was planning to stay in shrinking UI even home key is pressed. However, I remember there was some technical limitation that makes it complicated, so at the end we decided going back home when pressing homekey in shrinking UI.

Comment 19

[GitHub Autolander Bot]

Attachment: [gaia] cctuan:1155873 > mozilla-b2g:master

Comment 20

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8597833 [details] [review]
[gaia] cctuan:1155873 > mozilla-b2g:master

Hi Alive,

This patch would follow the original design from UX - preventing home/holdhome event while shrinkingUI is activated. Could you check if this patch is ok for you? Thanks.

Comment 21

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

Comment on attachment 8597833 [details] [review]
[gaia] cctuan:1155873 > mozilla-b2g:master

r=me, thanks!

Comment 22

[GitHub Autolander Bot]

Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.

Comment 23

[(Inactive after June) George Duan [:gduan] [:喬智]]

manually merge to master:
https://github.com/mozilla-b2g/gaia/commit/6503d8cc96f7ba473bf8f55942c2290bf878272a

Comment 24

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8597833 [details] [review]
[gaia] cctuan:1155873 > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Probably a regression of Bug 1050164 and hierachy_manager.
[User impact] if declined: Bad user experience when user launch shrinking ui from notification in homescreen. see comment 0.
[Testing completed]: Yes, manually test and unit test.
[Risk to taking this patch] (and alternatives if risky): No.
[String changes made]:

Comment 25

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/a0a45ed61ce91008476e3a0f64e7596ce4e76729

Comment 26

[Alison Shiue]

Verified on
[2.2]
Build ID               20150506162500
Gaia Revision          c6a6996841860ab335bf46b273477dc4bef19c95
Gaia Date              2015-05-06 17:01:57
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/c7fafa53b4e7
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150506.195956
Firmware Date          Wed May  6 20:00:07 EDT 2015
Bootloader             L1TC100118D0

[3.0]
Build ID               20150506160205
Gaia Revision          426fe6450ab8da92bb473fef12ccb39c6c920dd0
Gaia Date              2015-05-06 08:40:16
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/5593ac626826
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150506.193508
Firmware Date          Wed May  6 19:35:21 EDT 2015
Bootloader             L1TC100118D0