1083053 - [Midori] The screenshot preview window still overlays an app when switching apps

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Gallery, defect, P2)

Description

[sync-1]

FFOS: FFOS 2.0
 Mozilla Build ID: 20140916000205
 
 Created an attachment (id=972071)
 jrdlog
 
 DEFECT DESCRIPTION:
 Can not launch browser after preview screenshots from status bar
 
 REPRODUCING PROCEDURES:
 1.Launch browser->click the power key and home key at the same time
 2.Launch status bar->click the screenshots notification to preview the screenshots
 3.Click the home button->click the browser icon,it will auto switch to preview screenshots interface->KO
 
 
 Comments
 
 
 EXPECTED BEHAVIOR:
 KO:It should launch browser
 
 ASSOCIATE SPECIFICATION:
 
 TEST PLAN REFERENCE:
 
 TOOLS AND PLATFORMS USED:
 
 USER IMPACT:
 MID
 
 REPRODUCING RATE:
 5/5
 
 For FT PR, Please list reference mobile's behavior:

Comment 1

[sync-1]

Attachment: jrdlog

Comment 2

[sync-1]

[Blocking Requested - why for this release]:

Comment 3

[Wesly Huang (EPM)]

Hi Keven:

I guess this should be Gaia/Gecko's issue but not sure, would you help judge and assign proper team to look into? Thank you.

Comment 4

[Keven Kuo [:kkuo]]

Hi! Evelyn,

Per discussion, please take a look. Thanks

--
Keven

Comment 5

[Evelyn Hung]

Seems a gaia window management issue. 
Luke, could you take a look? It's on v2.0. I guess we could reproduce it on Flame.

Comment 6

[Luke Chang [:lchang] (inactive)]

Yes, it can be reproduced on flame/master. Apart from browser, it happens on every apps. I'll take a look at it.

Comment 7

[Luke Chang [:lchang] (inactive)]

Attachment: workaround patch

We currently use the MozActivity to preview the screenshot by gallery app. By design is that the activity window belongs to the foreground app. That's why we see the screenshot again after relaunching the app if we don't close it in advance. To change this behavior is a big task which is too risky to land on v2.0 branch.

The attachment is a workaround that we close the preview window whenever its "visibilityState" is changed to "hidden". It, however, has a side effect that the preview window will be closed as well if we just want to turn the screen off.


Hi Wesly,

I'm not sure if it's able to fulfill the requirement. Could you help to confirm that? Thanks.

Comment 8

[Guoqiang.CHEN]

Dear Luke,

Whether can do different treatment according to parentapp，
if parentapp is app://system.gaiamobile.org/manifest.webapp，then it will be closed when the app is switched to background, else it will not be closed?

Comment 9

[Guoqiang.CHEN]

Or if active's parentapp is not foreground app, the active will be closed when the app is switched to background?

Comment 10

[Wesly Huang (EPM)]

Thanks Luke, do you mean the patch can solve original issue, but will also close preview window after screen off, as a side effect? I feel compare with the original issue, this side effect is less trouble to user so it's a "good deal", but is there another way to solve issue w/o side effect?

Comment 11

[Luke Chang [:lchang] (inactive)]

(In reply to Guoqiang.CHEN from comment #9)
> Or if active's parentapp is not foreground app, the active will be closed
> when the app is switched to background?

My patch is to ask the activity window to close itself because the system app currently doesn't know the purpose of each activity window. However, an activity window isn't able to know who is its caller and the caller's status.

Comment 12

[Luke Chang [:lchang] (inactive)]

(In reply to Luke Chang [:lchang] from comment #11)
> My patch is to ask the activity window to close itself because the system
> app currently doesn't know the purpose of each activity window.

That means I can't just force system app to close the activity window in certain cases because we don't know the activity window of gallery app is called by app or notification.

(In reply to Wesly Huang from comment #10)
> ..., but is there another way to solve issue w/o side effect?

A proper solution may need to change the behavior of mozActivity or use another mechanism to replace it. Either way is too risky, especially for v2.0.

Comment 13

[Wesly Huang (EPM)]

Thanks Luke, so per discussion, next steps are (pls feel free to correct)

1. confirm w/ partner if this "quick fix" w/ side effect is acceptable. (so far no better solution)

If yes for (1),
2. get gallery owner's review to decide if we can land it in master.
3. Triage it and get it to be a blocker for raising priority, also decide which branch(s) to land (depends on (2)).

Hi Chenguo:

No perfect solution here unfortunately, would you let us know if the proposed workaround in comment#7 works for you? Thanks.

Comment 14

[Guoqiang.CHEN]

Dear Wesly,

It works for me, thanks.

Comment 15

[Wesly Huang (EPM)]

Thanks Chenguo!

@Luke: then per previous discussion next step is to discuss with gallery owner about the land target, would you help on that and let us know the result here, so we make proper decision in coming Triage? Thank you.

Comment 16

[Luke Chang [:lchang] (inactive)]

Attachment: pull request to master

Hi David,

Could you please kindly take a look at this patch? It's a workaround because we need a quick fix on v2.0 for now. I'm not sure should it be also landed on master before we figure out a proper solution. If not, I'll recreate a pull request to v2.0 only. Thanks a lot.

Comment 17

[Luke Chang [:lchang] (inactive)]

[Blocking Requested - why for this release]:
Partner's request.

Comment 18

[Wesly Huang (EPM)]

2.0 tag candidate for coming Triage.

Comment 19

[David Flanagan [:djf]]

Comment on attachment 8511833 [details] [review]
pull request to master

This patch looks good to me if you make the changes requested on github.

I'm okay landing it on master. Ideally there would be some better fix in the system app, though it is not clear to me what that fix is. Is there a followup bug that can be filed to address the underlying issue?

Comment 20

[Luke Chang [:lchang] (inactive)]

Hi David,

Thanks for reviewing. I've addressed the comments.

For the followup issue, I'd like to file another bug and raise a discussion about the feasibility of using mozActivity with "disposition: window".

Comment 21

[Luke Chang [:lchang] (inactive)]

landed on master: https://github.com/mozilla-b2g/gaia/commit/5d0ffc09f69dd3efa6c2f4d220f61b681568b0ec

Comment 22

[Wesly Huang (EPM)]

Indeed huge user impact and patch already available, candidate to tag in coming 2.0 Triage.

Comment 23

[Wesly Huang (EPM)]

[Tiage] tag for 2.0.

Comment 24

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8511833 [details] [review]
pull request to master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): 2.0+ blocker.
[User impact] if declined: it confuses users that they may think the app can't be switch back from cards view.
[Testing completed]: yes.
[Risk to taking this patch] (and alternatives if risky): very low.
[String changes made]: N/A

Comment 25

[Ryan VanderMeulen [:RyanVM]]

Presumably this needs to land on v2.1 as well? Please request approval for that as well if true :)

Comment 26

[Luke Chang [:lchang] (inactive)]

Yes, it can be reproduced on v2.1, too.

Comment 27

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8511833 [details] [review]
pull request to master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): 2.0+ blocker.
[User impact] if declined: it confuses users that they may think the app can't be switch back from cards view.
[Testing completed]: yes.
[Risk to taking this patch] (and alternatives if risky): very low.
[String changes made]: N/A

Comment 28

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/3cada9a34ea7f764802738894861559e3eed87f4

Comment 29

[Ryan VanderMeulen [:RyanVM]]

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

Comment 30

[Coler]

Verify passed, this issue can't be repro on Woodduck 2.0; Flame2.0&2.1&2.2
Attached: Verify_screenshot.mp4
Reproducing rate: 0/5

Woodduck build:
Gaia-Rev        cc690f8016b672475dc186bc7fd58aef45e684b7
Gecko-Rev       03d3ab62d5b07b915434f2d1d68495ad5915ecd2
Build-ID        20141118184148
Version         32.0

Flame 2.0 build:
Gaia-Rev        086a668942292168f312b3bb53e275fa0886fab1
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/a57b299c5cf2
Build-ID        20141117000200
Version         32.0

FLame 2.1 build:
Gaia-Rev        81160ad79e5b4c21967418dd63f1a1d08d77924e
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/3572aa3e6766
Build-ID        20141117001201
Version         34.0

FLame 2.2 build:
Gaia-Rev        ae3a84acaab80a5b35d5542d63e68462273c8a1b
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/2d0a51ef828d
Build-ID        20141117160200
Version         36.0a1

Comment 31

[Coler]

Attachment: Verify_Woodduck_screenshot.MP4

Comment 32

[Coler]

Sorry to update the Flame build:
Flame 2.0 build:
Gaia-Rev        1ede2666f1e6c1b3fd3b282011caf0cbc59544b0
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/2bea026d4f86
Build-ID        20141118000207
Version         32.0

FLame 2.1 build:
Gaia-Rev        1b231b87aad384842dfc79614b2a9ca68a4b4ff3
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/95fbd7635152
Build-ID        20141118001204
Version         34.0

FLame 2.2 build:
Gaia-Rev        4aee256937afe9db2520752650685ba61ce6097d
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/084441e904d1
Build-ID        20141118144012
Version         36.0a1