1105043 - [SHB] SHB does NOT disappear on lockscreen when pressed from callscreen

Status: VERIFIED FIXED

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

Description

[Yeojin Chung [:YeojinC]]

Attachment: logcat_20141125_SHBonLockscreen.txt

Description:
When SHB is enabled, the software home button appears properly when the user gets an incoming call from the lockscreen. However, it does NOT disappear when SHB is pressed and remains on top of the lockscreen.

Pre-requisite: Software home button is enabled.

Repro Steps:
1) Update a Flame device to BuildID: 20141125040209.
2) Press the power button to lock the device.
3) Make a phone call to DUT.
4) When the callscreen appears, press the software home button.
  
Actual:
SHB appears on top of the lockscreen.
  
Expected: 
SHB disappears when the lockscreen is displayed.
  
Environmental Variables:
Device: Flame 2.2 Master (319mb, KK, Shallow Flash)BuildID: 20141125040209
Gaia: 824a61cccec4c69be9a86ad5cb629a1f61fa142f
Gecko: acde07cb4e4d
Version: 36.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0
  
Repro frequency: 100%
See attached:video clip, logcat
http://youtu.be/cGMzMvrIVE8

Comment 1

[Yeojin Chung [:YeojinC]]

Unable to reproduce this bug on Flame 2.1. As discussed on bug 1068470 and bug 1095142, the SHB is missing on the callscreen, and 2.1 is still affected. 

Device: Flame 2.1 (319mb, KK, Shallow Flash)
BuildID: 20141125001201
Gaia: 1bdd49770e2cb7a7321e6202c9bf036ab5d8f200
Gecko: db893274d9a6
Version: 34.0 (2.1)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 2

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

The SHB should not appear and the lockscreen is cutoff so nominating 2.2?

Comment 3

[Manuel Casas Barrado [:mancas]]

Attachment: Proposed patch

Hey Alive, could you take a look at the patch? Maybe, we could fixed this issue by using another implementation, so let me know what your thought about it

Thanks

Comment 4

[Yeojin Chung [:YeojinC]]

(In reply to Yeojin Chung [:YeojinC] from comment #1)
> Unable to reproduce this bug on Flame 2.1. As discussed on bug 1068470 and
> bug 1095142, the SHB is missing on the callscreen, and 2.1 is still
> affected. 

Flame 2.1 is fixed on bug 1095142 now. This issue also reproduces on Flame 2.1.

Result: SHB appears on top of the lockscreen.

Device: Flame 2.1 (319mb, KK, Full Flash)
BuildID: 20141201001201
Gaia: ccb49abe412c978a4045f0c75abff534372716c4
Gecko: 18fb67530b22
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 34.0 (2.1)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 5

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

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

The problem is not in attention window side but in software button side. We should not change attention window to fulfill the requirement of software home.

A better way to me is to listen to hieraychychanged event in software home button to hide the active home button if Service.query('getTopMostWindow').CLASS_NAME === 'LockscreenWindow'

Comment 6

[Manuel Casas Barrado [:mancas]]

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

Hey Alive, I've took into account your comments, but the event |hierarchychanged| does not exists (AFAIK) I think you mean |stackchanged|. However, by using this events we have to face two main problems:

1 - The event is dispatched before the animation ends, so when asking for the class name of the top window we obtain the CallScreen in this case.

2 - Not every attention window can become a toaster, I think. So i believe is more elegant to dispatch custom events when an attention window becomes a toaster. In that case we can act consecuently in the sowftware home button manager.

Please take a look at the patch and let me know what you think about the approach.

Thanks!

Comment 7

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

(In reply to Manuel Casas Barrado [:mancas] from comment #6)
> Comment on attachment 8530233 [details] [review]
> Proposed patch
> 
> Hey Alive, I've took into account your comments, but the event
> |hierarchychanged| does not exists (AFAIK) I think you mean |stackchanged|.
> However, by using this events we have to face two main problems:
> 
> 1 - The event is dispatched before the animation ends, so when asking for
> the class name of the top window we obtain the CallScreen in this case.
> 
> 2 - Not every attention window can become a toaster, I think. So i believe
> is more elegant to dispatch custom events when an attention window becomes a
> toaster. In that case we can act consecuently in the sowftware home button
> manager.
> 
> Please take a look at the patch and let me know what you think about the
> approach.
> 
> Thanks!

Sorry if I didn't make it clear;
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/hierarchy_manager.js#L111
It's no mispelled so it is 'hierachychanged'.

And the event means the top most UI is changed; so if you monitors it from software home button and observe
'Service.query('getTopMostWindow')' you will be able to know now the top most window is lockscreen or attention.

After that, please change the style of shb directly instead of putting something else on screen element.

Comment 8

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

Just notice v2.1 is affected;
for v2.1 there is no hierarchy-manager, so your current patch mostly works but:
* Listen to attention-inactive/attentionopened in software home button
* Change the element style directly

Comment 9

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

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

See comments above, let me know if you have problems.

Comment 10

[Manuel Casas Barrado [:mancas]]

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

Thanks for your feedback! It's more clear for me, now. Please take a look at the patch when you get a chance

Thanks!

Comment 11

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

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

Good! Thanks for your patience.

Comment 12

[Gregor Wagner [:gwagner]]

Can we land this?

Comment 13

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

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

Rob, can you take a look here and make sure this is what we want?

Comment 14

[Gregor Wagner [:gwagner]]

ui review ping :)

Comment 15

[Manuel Casas Barrado [:mancas]]

Hey Rob, could you review the UX in order to land this issue?

Thanks!

Comment 16

[Rob MacDonald [:robmac]]

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

Thanks for your patience. I'm plus'ing the patch with one minor suggestion that could be raised as a separate bug if required. 

As the soft home button transitions in (~.5 seconds), you can briefly see the wallpaper underneath. Normally this would like fine but with the white background behind the unlock mechanism it looks unintentional. Could we extend the white background behind the incoming call unlock mechanism to the bottom of the screen? I suspect this is probably more complicated than it sounds. :)

Comment 17

[Manuel Casas Barrado [:mancas]]

Hey Rob, I've been trying to repro what you said, but I couldn't. Let's land this patch to fix this issue first, but feel free to open a new bug in order to track the issue. I will take care of the new bug for sure =)

Comment 18

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/246e806da48b66182b3a285b608ea6f3f3eccb5c

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Please request Gaia v2.2 approval on this when you get a chance.

Comment 20

[Manuel Casas Barrado [:mancas]]

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

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Software Home Button
[User impact] if declined: SHB will still displayed when the user press it from the callscreen which will cause an overlapping the lockscreen
[Testing completed]: Manual and unit test
[Risk to taking this patch] (and alternatives if risky): Low
[String changes made]: None

Comment 21

[Ryan VanderMeulen [:RyanVM]]

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

Comment 22

[Yeojin Chung [:YeojinC]]

This issue is verified fixed on Flame Master and 2.2.

Result: SHB disappears when the user selects the home button, and the lock screen is not covered by SHB.
 
Device: Flame Master (KK, 319mb, full flash)
Build ID: 20150220010206
Gaia: e4f7c67378e33e83f88d38ddb4a6c2cabf1423c3
Gecko: 1b4c5daa7b7a
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

Device: Flame 2.2 (KK, 319mb, full flash)
Build ID: 20150220002501
Gaia: ce79d35b92261e7cbfeaefebf87859ebeb0979b4
Gecko: b864abe1c6b3
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0a2 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0