Michael Henretty [:mikehenrty][:mhenretty] Reporter
	Description • 10 years ago

STR:

1.) Enable SHB and lockscreen
2.) Make a phone call
3.) Lock phone

Expected results:
Callscreen takes up entire height of device.

Actual result:
Lockscreen displays partially below the callscreen, see attachment.

	Michael Henretty [:mikehenrty][:mhenretty] Reporter
	Comment 1 • 10 years ago

[Blocking Requested - why for this release]:
Broken UI.

	Michael Henretty [:mikehenrty][:mhenretty] Reporter
	Comment 2 • 10 years ago

Alberto, this will most likely block 2.1. Can you take a look?

	KTucker [:KTucker][Inactive 3/4/2016]
	Comment 3 • 10 years ago

I think this issue is a dupe of bug 1068470 which is still not fixed on 2.1

	Alberto Pastor [:albertopq]
	Comment 4 • 10 years ago

It worked for me with latest 2.1 pvt build. Waiting for Michael's input for closing it as dupe.

	Michael Henretty [:mikehenrty][:mhenretty] Reporter
	Comment 5 • 10 years ago

This is definitely still happening on my device. I flashed from pvt, and then flashed the latest v2.1 gaia. The weird thing is, sometimes it works sometimes it doesn't. ie, sometimes the window will resize after the turning on and off the lockscreen. In any case, you're right KTucker, this is a dupe of bug 1068470.

BuildID: 20141107001205
Gaia: f08c2743bdb875ece34aaaaa6b4a8b12bd67a635

https://github.com/mozilla-b2g/gaia/commit/f08c2743bdb875ece34aaaaa6b4a8b12bd67a635

	Guillaume Marty [:gmarty] Assignee
	Comment 6 • 10 years ago

I think we need some clarification from UX. The current behaviour is that, when a call is in progress, the SHB disappears after a lock/unlock sequence. This doesn't look consistent to me.

Rob, can you tell me what is supposed to happen after unlocking the screen? Should the status bar remains as it as before locking? Or should it disappears and the dialer be resized fullscreen?

	Guillaume Marty [:gmarty] Assignee
	Comment 7 • 10 years ago

Talked to Rob offline and the SHB should always be visible even after the screen has been locked and unlocked.

	Guillaume Marty [:gmarty] Assignee
	Comment 8 • 10 years ago

Here's the patch. It's kept to a minimal set of changes for uplifting to v2.1 but eventually, we'll want a refactoring to get rid of some CSS class set in screen.
Alive, I worked with Etienne on this patch, so can you review it please?

	Etienne Segonzac (:etienne)
	Comment 9 • 10 years ago

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

Commented on github, mainly missing tests :)

Alive a bit more information about the approach:
we're having more and more special cases in `layoutManager.height` and looks like it would be way more manageable if the layoutManager had an optional argument so you could request height for the lockscreenwindow or an attentionwindow. Basically replacing a bunch of global state with a simple class condition.

We don't want to change everything in this 2.1+ patch of course, but do you like the direction?

	Guillaume Marty [:gmarty] Assignee
	Comment 10 • 10 years ago

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

Now with added tests. How does it look?

	Etienne Segonzac (:etienne)
	Comment 11 • 10 years ago

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

r=me with the comments addressed, I'd like to wait for Alive's feedback before landing though.

	Guillaume Marty [:gmarty] Assignee
	Comment 12 • 10 years ago

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

Kevin, can you take over the review from Alive while he's away?

	Kevin Grandon :kgrandon
	Comment 13 • 10 years ago

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

If Alive is out, I think we are ok with landing this with Etienne's review due to the deadline. I'll leave my F+ here. Would prefer a cleaner abstraction around the height methods, but I think this is probably ok to land.

	Guillaume Marty [:gmarty] Assignee
	Comment 14 • 10 years ago

As Kevin said, landing as is for now because of the deadline.
In master: https://github.com/mozilla-b2g/gaia/commit/012a9d7a77942f3cf267b60c6a2c756c2b8bfea0

	Guillaume Marty [:gmarty] Assignee
	Comment 15 • 10 years ago

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

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Software home button and attention windows
[User impact] if declined: The software home button disappears on some edge cases of the attention window.
[Testing completed]: There are unit and marionette tests. Manual testing of the dialer should be done.
[Risk to taking this patch] (and alternatives if risky): We made sure to reduce the impact of the patch for the uplift, so the risk is low.
[String changes made]: None.

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 16 • 10 years ago

Trying to cherry pick this to v2.1, there's a conflict that I don't feel confident in fixing for myself in apps/system/js/layout_manager.js:

<<<<<<< HEAD
/* global KeyboardManager, softwareButtonManager, System,
          AppWindowManager */
=======
/* global inputWindowManager, softwareButtonManager, Service, AttentionWindow */
>>>>>>> 012a9d7... Merge pull request #26297 from gmarty/Bug-1095142-Dialer-screen-does-not-fully-cover-lockscreen

I mean, it's all commented out so probably doesn't matter, but still. Could I get a branch-specific patch to apply to be sure it's all correct?

	Guillaume Marty [:gmarty] Assignee
	Comment 17 • 10 years ago

Just add AttentionWindow at the end of the list like this:
/* global KeyboardManager, softwareButtonManager, System, AppWindowManager, AttentionWindow */

This annotation is used by JSHint for linting, so no impact on code or tests.

	Yeojin Chung [:YeojinC]
	Comment 18 • 10 years ago

This issue is verified fixed on Flame 2.2.

Result: SHB appears on the callscreen when the device is locked.

Device: Flame 2.2 (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
=======================================================

However, I found another issue that SHB does not disappear on the lockscreen when the user selects SHB from the callscreen. I filed a new bug 1105043.

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 19 • 10 years ago

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

	KTucker [:KTucker][Inactive 3/4/2016]
	Comment 20 • 10 years ago

Yeojin, please verify 2.1 on tomorrow's build.

	Elie
	Comment 21 • 10 years ago

This issue has been successfully verified on Flame 2.1:
Gaia-Rev        1bdd49770e2cb7a7321e6202c9bf036ab5d8f200
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/db893274d9a6
Build-ID        20141125001201
Version         34.0
Device-Name     flame
FW-Release      4.4.2

	Ryan VanderMeulen [:RyanVM]
	Comment 22 • 10 years ago

Reverted for Linter failures.
https://treeherder.mozilla.org/ui/logviewer.html#?job_id=45918&repo=mozilla-b2g34_v2_1

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

	Guillaume Marty [:gmarty] Assignee
	Comment 23 • 10 years ago

Here is a patch that fixes both linting and tests:
https://github.com/mozilla-b2g/gaia/pull/26480

	Ryan VanderMeulen [:RyanVM]
	Comment 24 • 10 years ago

v2.1: https://github.com/mozilla-b2g/gaia/commit/0e557640a58b7e5288ecbe23ac09d96c762bba0d

	Yeojin Chung [:YeojinC]
	Comment 25 • 10 years ago

This issue is verified fixed on Flame 2.1.

Result: SHB appears on the callscreen when the device is locked.

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