1143926 - Status bar icon color is off

Status: VERIFIED FIXED

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

Description

[Gregor Wagner [:gwagner]]

https://www.youtube.com/watch?v=KkoEi-xwU6w&feature=youtu.be

On current trunk:
STR:
(don't have wifi or data connection)
Open browser and load link
Press home button
Open browser and load another link
Press on Check settings button
Use edge gesture to go back to previous browser instance

Comment 1

[Gregor Wagner [:gwagner]]

Is this a regression?

Comment 2

[Gregor Wagner [:gwagner]]

Its hard to see at the end of the video but the status bar icons remain white on white.

Comment 3

[Pi Wei Cheng [:piwei] (inactive)]

This issue reproduces on Flame 3.0 and Flame 2.2. Following STR, status bar icons appear white on white background.

Device: Flame 3.0
BuildID: 20150316010202
Gaia: 4868c56c0a3b7a1e51d55b24457e44a7709ea1ae
Gecko: 436686833af0
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 39.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Device: Flame 2.2
BuildID: 20150316002502
Gaia: a6b2d3f8478ec250beb49950fecbb8a16465ff6f
Gecko: 18619f8f6c5c
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

----

This issue does not occur on Flame 2.1. Following STR, status bar icons appear gray on white background.

Device: Flame 2.1
BuildID: 20150316001204
Gaia: 90f4ca618048b608c8e663d3e1d84781cfa719a8
Gecko: 65e67896e813
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 34.0 (2.1) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 4

[Michael Henretty [:mikehenrty][:mhenretty]]

I'll take a look.

Comment 5

[Michael Henretty [:mikehenrty][:mhenretty]]

The problem is that we receive the `activityterminated` in the statusbar long after the edge gesture transition completes, and so the setAppearance call [1] causes us to use the wrong window chrome to set the theme color [1].

1.) https://github.com/mozilla-b2g/gaia/blob/c39e15f631de80c69467fda0d4ea0bcda9e194ca/apps/system/js/statusbar.js#L367
2.) https://github.com/mozilla-b2g/gaia/blob/c39e15f631de80c69467fda0d4ea0bcda9e194ca/apps/system/js/statusbar.js#L395

I'm investigating ways to fix this currently.

Comment 6

[GitHub Autolander Bot]

Attachment: [gaia] mikehenrty:bug-1143926-icon-colors-off > mozilla-b2g:master

Comment 7

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8579726 [details] [review]
[gaia] mikehenrty:bug-1143926-icon-colors-off > mozilla-b2g:master

I think we can undo the patch from bug 1081346, and not setAppearance on activitytermincated since the heiarchytopmostwindowchange event handles setting icon color properly even when closing the bookmark menu. Alive, what do you think?

Comment 8

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

Comment on attachment 8579726 [details] [review]
[gaia] mikehenrty:bug-1143926-icon-colors-off > mozilla-b2g:master

Yap! It's what I plan to do in bug 1094759 as well - to simplify all *opened/*closed event handler in Statusbar. It's fine to do this earlier.

Comment 9

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8579726 [details] [review]
[gaia] mikehenrty:bug-1143926-icon-colors-off > mozilla-b2g:master

Alright, manually backed out bug 1081346, and added some unit test protection against similar workarounds in the future. Alive?

Comment 10

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

Comment on attachment 8579726 [details] [review]
[gaia] mikehenrty:bug-1143926-icon-colors-off > mozilla-b2g:master

\O/

Comment 11

[Michael Henretty [:mikehenrty][:mhenretty]]

master: https://github.com/mozilla-b2g/gaia/commit/7a6e62d7666b536b1b834dcddad1fe5554cec1c4

Comment 12

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8579726 [details] [review]
[gaia] mikehenrty:bug-1143926-icon-colors-off > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
bug 1081346

[User impact] if declined:
Improper statusbar colors when edge-gesturing between windows with activities opened.

[Testing completed]:
Manually tested, and added a unit test to prevent the same type of regression.

[Risk to taking this patch] (and alternatives if risky):
This patch simplifies logic when calculating statusbar icon colors because it backs out the workaround in bug 1081346. It is low risk, and there are no better alternatives.

[String changes made]: none.

Comment 13

[Hermes Cheng (inactive after July 27, 2015)]

NI myself for following verification.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

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

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Now with less rebase fail...

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

Comment 16

[Hermes Cheng (inactive after July 27, 2015)]

Verify this bug with below builds

*2.2
Build ID               20150323162503
Gaia Revision          e54c4ed1cc188f70ddf1156534d364005dc45490
Gaia Date              2015-03-23 19:09:26
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/7ba1778d237b
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150323.200543
Firmware Date          Mon Mar 23 20:05:54 EDT 2015
Bootloader             L1TC000118D0

*master
Serial: f2cfccfb (State: device)
Build ID               20150323160205
Gaia Revision          8eac260ee81a8aca05770d18c5736536d44ee7a7
Gaia Date              2015-03-23 02:41:53
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/bc85c479668a
Gecko Version          39.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150323.193421
Firmware Date          Mon Mar 23 19:34:34 EDT 2015
Bootloader             L1TC100118D0