1068470 - [soft-home-button] SHB missing when lockscreen is on, and there is an incoming call

Status: VERIFIED FIXED

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

Description

[Eric Chang [:ericcc] [:echang]]

Attachment: LockscreenIncomingCall.png

### STR
1. Turn on soft home button on Flame
2. Turn on lockscreen
3. Reboot
4. Make call to Flame

### Actual
SHB missing, background shown

### Reproduce rate
100%
But only for the first time after reboot. a second call won't have this problem. 

### Version
Gaia      47939f4c41d0c941e5047e5d1af74a79b7d8e0d5
Gecko     https://hg.mozilla.org/releases/mozilla-aurora/rev/e20869e87e23
BuildID   20140916160202
Version   34.0a2
ro.build.version.incremental=110
ro.build.date=Fri Jun 27 15:57:58 CST 2014
B1TC00011230

Comment 1

[:Eli Perelman]

Eric, could you please re-test this now that bug 1067441 has landed?

Comment 2

[Eric Chang [:ericcc] [:echang]]

Thanks, it is fixed in V2.2, so waiting for the uplift to v2.1.

Comment 3

[Eric Chang [:ericcc] [:echang]]

Verify against v2.1 after bug 1067441 got uplifted

Comment 4

[:Eli Perelman]

Eric, would you mind verifying this now that bug 1067441 is uplifted?

Comment 5

[Eric Chang [:ericcc] [:echang]]

Actually this is not fixed by bug 1067441, the bar is still missing.

Comment 6

[:Eli Perelman]

In my own testing, I have found that this is not resolved in v2.2/master either.

Comment 7

[:Eli Perelman]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24491

Comment 8

[Etienne Segonzac (:etienne)]

Comment on attachment 8496195 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24491

A few comments on github.

Comment 9

[Kevin Grandon :kgrandon]

Is there a more generic fix that we can put into AttentionWindow? Otherwise we will have to make multiple fixes here. This issue also reproduces with clock alarms it seems.

Comment 10

[Martin Shuman [:Marty]]

I'm also seeing this issue at the lockscreen when long-pressing the power button to see the 'restart/power off' menu.

I can write up another bug for this if you end up wanting to track these all as separate issues.

Comment 11

[:Eli Perelman]

@Marty: provided that we do the generic fix that Kevin was mentioning, that issue should be resolved with this bug. Let's definitely keep an eye out for it.

Comment 12

[Alberto Pastor [:albertopq]]

Just to double check, when SBH is enabled and in the lockscreen, for these kind of cases:

1.- Incoming call
2.- Longpress power button
3.- Alarm

Should the SBH be displayed or should those cases take the whole screen?

Comment 13

[Guillaume Marty [:gmarty]]

Whatever is the expected behaviour in this case, I already started working on a test.
Here's the PR, but it will have to be adapted when the fix is in:
https://github.com/mozilla-b2g/gaia/pull/24893

Comment 15

[Candice Serran (:cserran)]

duplicate of https://bugzilla.mozilla.org/show_bug.cgi?id=1077567. blocking this bug for high resolution issue. broken functionality.

Comment 16

[Rob MacDonald [:robmac]]

(In reply to Alberto Pastor [:albertopq] from comment #12)
> Just to double check, when SBH is enabled and in the lockscreen, for these
> kind of cases:
> 
> 1.- Incoming call
> 2.- Longpress power button
> 3.- Alarm
> 
> Should the SBH be displayed or should those cases take the whole screen?

I tried each of these cases using the hardware button and, after consulting with other ux team members, we don't feel it's necessary to show the soft home button in these scenarios while on lock screen. 

My build is buggy and I'm unable to answer a call but what happens in the scenario where the phone is locked and the user answers the call? In this case the soft home button would be needed.

Comment 17

[Guillaume Marty [:gmarty]]

Attachment: Github PR

This patch fixes the height issue on the attention window, thus benefitting the callscreen and the alarm.

I added a marionette test and fix the unit tests.

Etienne, can you have a look at it?

Comment 18

[Etienne Segonzac (:etienne)]

Comment on attachment 8501772 [details] [review]
Github PR

kudos on the integration test

r=me with the test mentioned on github added

Comment 19

[Kevin Grandon :kgrandon]

Comment on attachment 8501772 [details] [review]
Github PR

lgtm as well.

Comment 20

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

Landing this, so it gets in the next QA build.

Comment 21

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

master: https://github.com/mozilla-b2g/gaia/commit/0adb71046f24709743ed8947a52777bb7b7911db

Guillaume, please request uplift.

Comment 22

[Guillaume Marty [:gmarty]]

Comment on attachment 8501772 [details] [review]
Github PR

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Software home button
[User impact] if declined: The attention windows (callscreen, alarm...) are cut when the SHB is enabled
[Testing completed]: The patch contains marionette tests to check conformity and avoid regressions
[Risk to taking this patch] (and alternatives if risky): The fix itself is trivial so no risks
[String changes made]: None

Comment 23

[bhavana bajaj [:bajaj]]

Eric, can you please help verify this finally fixes the issue you reported? Thanks !!

Comment 24

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/774d3dd487417e15a57df5ca384be0587d4771ff

Comment 25

[[:RJ]]

Issue is verified fixed on Flame 2.2

the gap where the software home button was is no longer empty and the call screen takes the whole page after these STR 

Flame 2.2 Master KK (319mb) (Full Flash)

Device: Flame 2.2 Master KK (319mb) (Full Flash)
BuildID: 20141012040203
Gaia: 717ad4e8b7fc10ab8248500d00ba5ba0977fa8ab
Gecko: 44168a7af20d
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0


The issue was not fixed and cannot be verified on 2.1 flame 

The area where the software home button occupy s is an empty gap on the lock screen  

Environmental Variables:
Device: Flame 2.1 KK (319mb) (Full Flash)
Build ID: 20141012000201
Gaia: d71f8804d7229f4b354259d5d8543c25b4796064
Gecko: 7fa82c9acdf2
Version: 34.0a2 Flame 2.1 KK (319mb)
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 26

[Eric Chang [:ericcc] [:echang]]

Hi RJ, the v2.1 looks good on my flame. Could you check again if the str is probably not the same, and there is a new bug, thanks.

Gaia-Rev        f5d4ff60ffed8961f7d0380ada9d0facfdfd56b1
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-aurora/rev/e96a7a4f3bbe
Build-ID        20141012000201
Version         34.0a2Gaia-Rev        f5d4ff60ffed8961f7d0380ada9d0facfdfd56b1
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-aurora/rev/e96a7a4f3bbe
Build-ID        20141012000201
Version         34.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141012.033729
FW-Date         Sun Oct 12 03:37:40 EDT 2014
Bootloader      L1TC00011840
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141012.033729
FW-Date         Sun Oct 12 03:37:40 EDT 2014
Bootloader      L1TC00011840

Gaia-Rev        f5d4ff60ffed8961f7d0380ada9d0facfdfd56b1
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-aurora/rev/75cd3a8fa031
Build-ID        20141012160202
Version         34.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141012.193822
FW-Date         Sun Oct 12 19:38:32 EDT 2014
Bootloader      L1TC00011840

Comment 27

[[:RJ]]

Hello Eric, on today’s build using the STR's from the description I was able to reproduce this issue, I will enclose a video of my repro to get to that screen again, just to show my steps to make sure this is the same issue and not a new issue. Thanks! 

https://www.youtube.com/watch?v=5joNGgI4iag 

Today’s build Environment variables  

Flame 2.1 KK (319mb) (Full Flash)
Environmental Variables:
Device: Flame Master
Build ID: 20141013001201
Gaia: d18e130216cd3960cd327179364d9f71e42debda
Gecko: 610ee0e6a776
Version: 34.0a2 (Master)
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 28

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

Guillaume, could you take a look regarding comment 27?

Comment 29

[Eric Chang [:ericcc] [:echang]]

Thanks RJ, I checked the gaia/gecko commit, the one shown in video seems to be this tinderbox build(1010), and the issue is reproducible with this. I tried 1013 aurora & b2g34_v2_1, both builds are okay.



-- Tinderbox(reproducible)
https://pvtbuilds.mozilla.org/pvt/mozilla.org/b2gotoro/tinderbox-builds/mozilla-b2g34_v2_1-flame-kk-eng/20141010110704/
Gaia-Rev        d18e130216cd3960cd327179364d9f71e42debda
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/610ee0e6a776
Build-ID        20141010110704
Version         34.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141010.143133
FW-Date         Fri Oct 10 14:31:44 EDT 2014
Bootloader      L1TC00011840


-- aurora (build okay)
https://pvtbuilds.mozilla.org/pvt/mozilla.org/b2gotoro/nightly/mozilla-aurora-flame-kk-eng/2014/10/2014-10-13-00-02-05/
Gaia-Rev        f5d4ff60ffed8961f7d0380ada9d0facfdfd56b1
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-aurora/rev/ad497694e258
Build-ID        20141013000205
Version         34.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141013.032854
FW-Date         Mon Oct 13 03:29:05 EDT 2014
Bootloader      L1TC00011840

-- b2g34_v2_1 (build okay)
https://pvtbuilds.mozilla.org/pvt/mozilla.org/b2gotoro/nightly/mozilla-b2g34_v2_1-flame-kk-eng/2014/10/2014-10-13-16-12-04/
Gaia-Rev        d51956f84ebec0dd8998654f01f9f24fe8527f51
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/a0647b2686eb
Build-ID        20141013161204
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141013.200818
FW-Date         Mon Oct 13 20:08:29 EDT 2014
Bootloader      L1TC00011840

Comment 30

[Guillaume Marty [:gmarty]]

Clearing the ni? flag.

Comment 32

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

I am still seeing this issue on master and v2.1.

Created attachment 8518483 [details]
[screenshot] weird callscreen

STR:

1.) Enable SHB and lockscreen.
2.) Make a phone call and wait for the call to connect.
3.) Press power button twice to lock phone.

Expected results:
Callscreen takes up entire height of device.

Actual result:
Lockscreen displays partially below the callscreen.

Note: this does not reproduce 100% of the time, but it does most times for me. There might be a race condition between the lockscreen and the callscreen resize logic.

Comment 33

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

Guillaume, can you take another look?

Comment 34

[Guillaume Marty [:gmarty]]

I tried in a wide varieties of different configurations (incl. v2.1 and master) but I couldn't repro this.
Can you give me more details about your system or solid STR?

Comment 35

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

I just reproduced this on a freshly flashed v2.1 build, 20141110001201.

Here is a more solid STR:

1.) Make phone call with device.
2.) Wait for second phone to ring.
3.) Press power button.
4.) Wait 1 second.
5.) Press power button again.

If this still doesn't work, can you tell me what you see when you press the power button again? Do you see the callscreen resize itself to be fullscreen? Is it immediately fullscreen? Do you see the SHB at the button?

Comment 36

[Guillaume Marty [:gmarty]]

Attachment: Github PR

Thanks Michael, exactly what I needed. So it's not strictly speaking a regression from this bug as the original bug fixes the callscreen while this one impacts the dialer (let's be precise here because it took me a while to realize that).

In this pull request I address this issue by setting a dialer CSS class on the screen element. Can you please have a look to see if it looks good to you? If so, I'll add tests and ask for r?. Thanks.

Comment 37

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

(In reply to Guillaume Marty [:gmarty] from comment #36)
> Created attachment 8520655 [details] [review]
> Github PR
> 
> Thanks Michael, exactly what I needed. So it's not strictly speaking a
> regression from this bug as the original bug fixes the callscreen while this
> one impacts the dialer (let's be precise here because it took me a while to
> realize that).

Ah, should we open a new bug then and close this one? Perhaps we can un-dupe bug 1095142.

Comment 38

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

Comment on attachment 8520655 [details] [review]
Github PR

I'm not a good guy to give feedback on dialer changes, so I'll re-direct. I will say that it seems weird to fix a SHB issue by adding a css class. I feel like we are just hiding the underlying race condition that gives the dialer window the wrong size in certain conditions. Maybe that is a workable fix though.

In any case, I'll leave it to Etienne to give better feedback.

Comment 39

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

Comment on attachment 8520655 [details] [review]
Github PR

Adding Doug as Etienne may be swamped with reviews when he comes back.

Comment 40

[Etienne Segonzac (:etienne)]

Comment on attachment 8520655 [details] [review]
Github PR

This should be our last resort :)

We're trying to move away from this kind of global state hidden in a css class on #screen. Especially asymmetric ones set in on file and removed in another.

Also, I think we should try to make this fix work for all attention windows and not just the callscreen.

AttentionWindowManager#hasActiveWindow should probably help fix the race once we figure out where it comes from :) (ping me on IRC for any attention windows questions)

Comment 41

[Guillaume Marty [:gmarty]]

To avoid confusion, I'm closing this bug and dedupe from Bug 1095142 and let's continue the discussion from there.

Comment 42

[sarsenyev]

As per comment 41, the bug 1095142 will be followed up,
changing the status to verified based on 2.2 verification

Device: Flame 2.2 Master KK
BuildID: 20141112040208
Gaia: 5ae28ff11b982e2bd7d1aa097cda131536952bdc
Gecko: 688f821edcd4
Version: 36.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Comment 43

[Sue]

Attachment: video of issue verify

This issue has been successfully verified on Flame 2.1
See attachment: verify_video.MP4
Reproducing rate: 0/5
Flame 2.1 versions:
Gaia-Rev        f8d3bf44029e0afc0124600a4bb34dba8fc1ad21
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/f70a67a7f846
Build-ID        20141120001207
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141120.034911
FW-Date         Thu Nov 20 03:49:22 EST 2014
Bootloader      L1TC00011880