Closed Bug 1068470 Opened 5 years ago Closed 5 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:2.1+, b2g-v2.1 verified, b2g-v2.2 verified)

VERIFIED FIXED
2.1 S9 (21Nov)
blocking-b2g 2.1+
Tracking Status
b2g-v2.1 --- verified
b2g-v2.2 --- verified

People

(Reporter: echang, Assigned: gmarty)

References

Details

(Whiteboard: [systemsfe])

Attachments

(5 files)

### 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
QA Whiteboard: [COM=Gaia::System]
Eric, could you please re-test this now that bug 1067441 has landed?
Assignee: nobody → eperelman
Status: NEW → ASSIGNED
Flags: needinfo?(echang)
Target Milestone: --- → 2.1 S5 (26sep)
Thanks, it is fixed in V2.2, so waiting for the uplift to v2.1.
Flags: needinfo?(echang)
Verify against v2.1 after bug 1067441 got uplifted
Flags: needinfo?(echang)
Eric, would you mind verifying this now that bug 1067441 is uplifted?
Actually this is not fixed by bug 1067441, the bar is still missing.
Flags: needinfo?(echang)
In my own testing, I have found that this is not resolved in v2.2/master either.
Comment on attachment 8496195 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24491

A few comments on github.
Attachment #8496195 - Flags: review?(etienne)
Target Milestone: 2.1 S5 (26sep) → ---
Blocks: 1077579
Blocks: 1077567
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.
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.
@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.
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?
Flags: needinfo?(rmacdonald)
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
Duplicate of this bug: 1077567
duplicate of https://bugzilla.mozilla.org/show_bug.cgi?id=1077567. blocking this bug for high resolution issue. broken functionality.
blocking-b2g: --- → 2.1+
Assignee: eperelman → gmarty
(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.
Flags: needinfo?(rmacdonald)
Target Milestone: --- → 2.1 S6 (10oct)
Attached file 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?
Attachment #8501772 - Flags: review?(etienne)
Attachment #8501772 - Flags: feedback?(kgrandon)
Comment on attachment 8501772 [details] [review]
Github PR

kudos on the integration test

r=me with the test mentioned on github added
Attachment #8501772 - Flags: review?(etienne) → review+
Comment on attachment 8501772 [details] [review]
Github PR

lgtm as well.
Attachment #8501772 - Flags: feedback?(kgrandon) → feedback+
Component: Gaia::Settings → Gaia::System
Landing this, so it gets in the next QA build.
master: https://github.com/mozilla-b2g/gaia/commit/0adb71046f24709743ed8947a52777bb7b7911db

Guillaume, please request uplift.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: needinfo?(gmarty)
Resolution: --- → FIXED
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
Attachment #8501772 - Flags: approval-gaia-v2.1?(bbajaj)
Flags: needinfo?(gmarty)
Attachment #8501772 - Flags: approval-gaia-v2.1?(bbajaj) → approval-gaia-v2.1+
Eric, can you please help verify this finally fixes the issue you reported? Thanks !!
Flags: needinfo?(echang)
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
Status: RESOLVED → VERIFIED
QA Whiteboard: [COM=Gaia::System] → [COM=Gaia::System[failed-verification] [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
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
Flags: needinfo?(echang) → needinfo?(rmitchell)
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
Flags: needinfo?(rmitchell)
QA Whiteboard: [COM=Gaia::System[failed-verification] [QAnalyst-Triage?] → [COM=Gaia::System[failed-verification] [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Guillaume, could you take a look regarding comment 27?
Flags: needinfo?(gmarty)
QA Whiteboard: [COM=Gaia::System[failed-verification] [QAnalyst-Triage+] → [COM=Gaia::System][failed-verification] [QAnalyst-Triage+]
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
Clearing the ni? flag.
Flags: needinfo?(gmarty)
Flags: in-testsuite+
Duplicate of this bug: 1095142
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.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Guillaume, can you take another look?
Flags: needinfo?(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?
Flags: needinfo?(gmarty) → needinfo?(mhenretty)
Target Milestone: 2.1 S6 (10oct) → 2.1 S9 (21Nov)
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?
Flags: needinfo?(mhenretty) → needinfo?(gmarty)
Attached file 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.
Flags: needinfo?(gmarty)
Attachment #8520655 - Flags: feedback?(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 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.
Attachment #8520655 - Flags: feedback?(mhenretty) → feedback?(etienne)
Comment on attachment 8520655 [details] [review]
Github PR

Adding Doug as Etienne may be swamped with reviews when he comes back.
Attachment #8520655 - Flags: feedback?(etienne)
Attachment #8520655 - Flags: feedback?(drs.bugzilla)
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)
Attachment #8520655 - Flags: feedback?(etienne)
Attachment #8520655 - Flags: feedback?(drs.bugzilla)
Attachment #8520655 - Flags: feedback-
To avoid confusion, I'm closing this bug and dedupe from Bug 1095142 and let's continue the discussion from there.
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
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
Status: RESOLVED → VERIFIED
QA Whiteboard: [COM=Gaia::System][failed-verification] [QAnalyst-Triage+] → [COM=Gaia::System][QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [COM=Gaia::System][QAnalyst-Triage?] → [COM=Gaia::System][QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Attached video 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
You need to log in before you can comment on or make changes to this bug.