1097296 - [Window Management] Battery, Signal and time on the status bar will appear behind download update overlay

Status: VERIFIED FIXED

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

Description

[KTucker [:KTucker][Inactive 3/4/2016]]

Description:
If the user has a browser window open, pulls down the notification tray and taps on the "update available" notification, they will notice the battery, signal and time will appear behind the overlay. 

Also, if the user tries to edge gesture while on the the download update overlay, the status bar will completely disappear.

Repro Steps:
1)  Updated Flame to Build ID: 20141110001201
2)  Tap on the "Browser" icon to open a browser window.
3)  Pull down the notification tray and tap on the "Update Available" notification while on the browser window.

Notice the battery, signal and time appear behind the "download update" overlay.

4) Edge gesture while on the "download update" overlay.

Actual:
The battery, signal and time appear behind the "download update" overlay. When the user edge gestures, the status bar disappears completely.

Expected:
The battery, signal and time appear correctly on the "download update" overlay and do not disappear when the user performs an edge gesture.

Environmental Variables
Device: Flame 2.1 (319mb)(Kitkat Base)(Shallow Flash)
BuildID: 20141110001201
Gaia: 0ec1925fc37b7c71d129ae44e42516a0cfb013c4
Gecko: 97487a2d1ee6
Version: 34.0 (2.1)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Notes:
Repro frequency: 100%
See attached: video clip, logcat,

Comment 1

[KTucker [:KTucker][Inactive 3/4/2016]]

This issue also occurs on the Flame 2.2

Status bar appears behind the "download update" overlay and disappears when edge gesturing.

Flame 2.2

Device: Flame 2.2 Master (319mb)(Kitkat Base)(Shallow Flash)
BuildID: 20141111040202
Gaia: 6af3a8a833eb8bb651e8b188cb3f3c3a43bb4184
Gecko: 76b85b90a8cb
Version: 36.0a1 (2.2) 
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

--------------------------------------------------------------

This issue does not occur on the Flame 2.0

Status bar does not appear behind the "download update" overlay.

Flame 2.0

Device: Flame 2.0 (319mb)(Kitkat Base)(Shallow Flash)
BuildID: 20141111000205
Gaia: dfdd6268b9784eafdd509f0ddf71407698ed5080
Gecko: fcc5d31f84d7
Version: 32.0 (2.0)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 2

[KTucker [:KTucker][Inactive 3/4/2016]]

Attachment: Updateavailablestatuslog.txt

Comment 3

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

[Blocking Requested - why for this release]:

Status bar can disappear from view. Regression

Comment 4

[Gregor Wagner [:gwagner]]

Lets fix on master and uplift if there is a low risk patch.

Comment 5

[Manuel Casas Barrado [:mancas]]

Attachment: Proposed patch

Hey Etienne, the status bar is not behind the updates dialog. What happens here is that we conserve the status bar icons color so if we have an active app with light theme, we will see a dark status bar with light icons, which could seems that the status bar is behind.

So, I'm not sure about the approach I've implemented since I don't know if we have an elegant method to know which is the status bar theme. Could you review it and tell me if there is another way to fix the issue?

Thanks

Comment 6

[Joshua Mitchell (Inactive)]

I do not think this (any) regression window is valid. What I'm seeing on 'working' builds is that the notification bar icons are not even on the system update overlay / popup. Then the first broken has all the elements of this bug where they appear on the overlay and disappear as you edge swipe between apps behind the message screen. That seems to indicate that the feature where the icons show on the pop-up messages was implemented with this bug.

Central Regression Window:

Last Working:
Device: Flame 2.1
Build ID: 20140915120232
Gaia: 66af962728700c6d2cc13e93f8ed5d122305185a
Gecko: 9dce8a7d62fc
Gonk: Could not pull gonk.  Did you shallow Flash?
Version: 34.0a2 (2.1)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

First Broken:
Device: Flame 2.1
Build ID: 20140915142636
Gaia: b11e74449c1d54628d339c3b98ccc9802d59e904
Gecko: 7c2fa0c32d6f
Gonk: Could not pull gonk.  Did you shallow Flash?
Version: 34.0a2 (2.1)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Gaia/Gecko Swap
Last Working Gaia First Broken Gecko: Issue does NOT reproduce
Gaia: 66af962728700c6d2cc13e93f8ed5d122305185a
Gecko: 7c2fa0c32d6f
First Broken Gaia Last Working Gecko: Issue DOES reproduce
Gaia: b11e74449c1d54628d339c3b98ccc9802d59e904
Gecko: 9dce8a7d62fc

Gaia pushlog: https://github.com/mozilla-b2g/gaia/compare/66af962728700c6d2cc13e93f8ed5d122305185a...b11e74449c1d54628d339c3b98ccc9802d59e904

Comment 7

[Etienne Segonzac (:etienne)]

Comment on attachment 8522902 [details] [review]
Proposed patch

Like you anticipated, there are a few issues with this approach:
* strong coupling between UpdateManager and StatusBar
* UpdateManager accessing the StatusBar's DOM
* UpdateManager keeping state for the StatusBar

We can do something similar to the lockscreen:
* from the UpdateManager, dispatch custom events for |updatepromptshown| and |updateprompthidden|
* from the StatusBar, listening to those events to remove the light class / put the status bar back to the current app style [1]

Then we will have 1 issue left: lockscreen events and updateprompt event not mixing well together. For example: while showing the update prompt on top of a browser tab, locking then unlocking the phone will setAppearance for the tab and put back the icons in "light" mode.
Instead of adding more complexity to the StatusBar code I think we should just discard the download prompt when the phone is locked (by listening to the lockscreen event).

Hope this makes sense, cheers!

[1] https://github.com/mozilla-b2g/gaia/blob/6f6e4b9b395b95dd920ee701d0c5ec6aa2c1d805/apps/system/js/statusbar.js#L354

Comment 8

[Manuel Casas Barrado [:mancas]]

Comment on attachment 8522902 [details] [review]
Proposed patch

I took into account your comments. Could you review the patch again?

Thanks!

Comment 9

[Etienne Segonzac (:etienne)]

Comment on attachment 8522902 [details] [review]
Proposed patch

Awesome! r=me with a few more tests checking that the updateprompt* events are dispatched properly in update_manager_test.js

Comment 10

[Manuel Casas Barrado [:mancas]]

Comment on attachment 8522902 [details] [review]
Proposed patch

Etienne,please, take a quick look at the test cases to be sure everything it's ok.

Thanks!

Comment 11

[Etienne Segonzac (:etienne)]

Comment on attachment 8522902 [details] [review]
Proposed patch

Looks good (made a small comment on github).

Comment 12

[Manuel Casas Barrado [:mancas]]

Oh Etienne, I've missed the edge gestures. If you enable de |edges-debug| you will see that the |z-index| property of the edge panels is higher than the downloads overlay. What do you think about adding a class to the |screen| element, in order to downgrade the edge panels when the overlay is shown? Or there is another way to prevent the use of these panels?

AFAIK you worked on the edge gestures, didn't you?

Comment 13

[Etienne Segonzac (:etienne)]

(In reply to Manuel Casas Barrado [:mancas] from comment #12)
> Oh Etienne, I've missed the edge gestures. If you enable de |edges-debug|
> you will see that the |z-index| property of the edge panels is higher than
> the downloads overlay. What do you think about adding a class to the
> |screen| element, in order to downgrade the edge panels when the overlay is
> shown? Or there is another way to prevent the use of these panels?
> 
> AFAIK you worked on the edge gestures, didn't you?

The edge_swipe_detector.js should listen for the updateprompt* events and turn the edge gestures on/off accordingly (by setting this.lifeCycleEnabled).

Comment 14

[Manuel Casas Barrado [:mancas]]

Comment on attachment 8522902 [details] [review]
Proposed patch

Done! Could you review the patch when you get a chance? Everything should be ok now :)

Thanks!

Comment 15

[Etienne Segonzac (:etienne)]

Comment on attachment 8522902 [details] [review]
Proposed patch

Hey, I made a small but important comment :)
(Thanks for taking the time to cover all the cases!)

Comment 16

[Manuel Casas Barrado [:mancas]]

Comment on attachment 8522902 [details] [review]
Proposed patch

Good catch! I forgot that case. It should be cover, now.

Thanks!

Comment 17

[Etienne Segonzac (:etienne)]

Comment on attachment 8522902 [details] [review]
Proposed patch

all good, thanks! (tiny nits on github)

Comment 18

[Manuel Casas Barrado [:mancas]]

Nits solved, requesting check-in

Comment 19

[Carsten Book [:Tomcat]]

https://github.com/mozilla-b2g/gaia/commit/8ef7d3b9bf21760ffb3492e45a86b1725a66b389

Comment 20

[Carsten Book [:Tomcat]]

reverted for test failures like https://treeherder.mozilla.org/ui/logviewer.html#?job_id=890010&repo=b2g-inbound

Comment 21

[Manuel Casas Barrado [:mancas]]

Attachment: Proposed patch

New patch with the linter test fixed. let's wait for green light in travis

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/381951392f7b4aae51e22b463bf59c79a5a9df07

Comment 23

[Yeojin Chung [:YeojinC]]

This issue is verified fixed on Flame Master.

Result: The status bar icons do not disappear or get affected by edge gesture.

Environmental Variables:
Device: Flame 3.0
BuildID: 20150121010204
Gaia: 5e98dc164b17fd6decb48a9eaddef0e55b82e249
Gecko: 540077a30866
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Comment 24

[Etienne Segonzac (:etienne)]

Comment on attachment 8528214 [details] [review]
Proposed patch

This bug is really similar to bug 1144054: the edge gesture are active during a system level prompt which leads to confusing UX.

In one case an app install prompt in the other a update prompt.

I think we should block/uplift both or none of them.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

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

Comment 26

[helen（leave from Mozilla）]

Attachment: Flame_v2.2.3gp

This bug has been verified as pass on latest build of Flame v2.2 by the STR in Comment 0. 

Results:
The battery, signal and time appear correctly on the "download update" overlay and do not disappear when the user performs an edge gesture.

Attachment: Flame_v2.2.3gp
Rate: 0/15

Device: Flame v2.2 (Verified)
Build ID               20150707162503
Gaia Revision          ea11f422b687a982f0a961c9aea7858066561707
Gaia Date              2015-07-02 23:37:50
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/52d19a805d35
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150707.202802
Firmware Date          Tue Jul  7 20:28:14 EDT 2015
Bootloader             L1TC000118D0