Closed Bug 1095142 Opened 5 years ago Closed 5 years ago
[SHB] Dialer screen does not fully cover lockscreen
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.
[Blocking Requested - why for this release]: Broken UI.
blocking-b2g: --- → 2.1?
Alberto, this will most likely block 2.1. Can you take a look?
I think this issue is a dupe of bug 1068470 which is still not fixed on 2.1
Assignee: nobody → apastor
It worked for me with latest 2.1 pvt build. Waiting for Michael's input for closing it as dupe.
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
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1068470
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Summary: [SHB] Callscreen does not fully cover lockscreen → [SHB] Dialer screen does not fully cover lockscreen
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?
Talked to Rob offline and the SHB should always be visible even after the screen has been locked and unlocked.
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?
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?
Comment on attachment 8525438 [details] [review] Github PR Now with added tests. How does it look?
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.
Attachment #8525438 - Flags: review?(etienne) → review+
Comment on attachment 8525438 [details] [review] Github PR Kevin, can you take over the review from Alive while he's away?
Attachment #8525438 - Flags: review?(alive) → review?(kgrandon)
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.
Attachment #8525438 - Flags: review?(kgrandon) → feedback+
As Kevin said, landing as is for now because of the deadline. In master: https://github.com/mozilla-b2g/gaia/commit/012a9d7a77942f3cf267b60c6a2c756c2b8bfea0
Status: REOPENED → RESOLVED
Closed: 5 years ago → 5 years ago
Resolution: --- → FIXED
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.
Attachment #8525438 - Flags: approval-gaia-v2.1?(bbajaj)
Attachment #8525438 - Flags: approval-gaia-v2.1?(bbajaj) → approval-gaia-v2.1+
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?
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.
Flags: needinfo?(gmarty) → needinfo?(kwierso)
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.
Yeojin, please verify 2.1 on tomorrow's build.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage-]
Flags: needinfo?(ktucker) → needinfo?(ychung)
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
Here is a patch that fixes both linting and tests: https://github.com/mozilla-b2g/gaia/pull/26480
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
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
You need to log in before you can comment on or make changes to this bug.