Closed Bug 1047115 Opened 10 years ago Closed 10 years ago

[User Story] Display soft home key while the Utility Tray is open

Categories

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

x86
macOS
defect
Not set
normal

Tracking

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

VERIFIED FIXED
2.1 S3 (29aug)
feature-b2g 2.1
Tracking Status
b2g-v2.1 --- verified
b2g-v2.2 --- verified

People

(Reporter: rmacdonald, Assigned: markus)

References

Details

(Whiteboard: [systemsfe][tako])

Attachments

(4 files)

As a user, I want the soft home key to be visible when the Utility Tray is fully revealed / opened.

Acceptance Criteria:
1. The Utility tray must not overlap the Soft Home Key Bar when the Utility Tray is fully revealed / open.
feature-b2g: --- → 2.1
Target Milestone: --- → 2.1 S1 (1aug)
Attached file Github patch
Hi Rob.

Please take a look at the attached github pull request.
Flags: needinfo?(rmacdonald)
Hi Francis - Sorry I'm still having problems applying patches. Can you review this patch from Markus as well? - Rob
Flags: needinfo?(fdjabri)
Attached video Utility_tray.MOV
After applying this patch, it seems as if the Utility tray is still overlapping the software home button bar.
Flags: needinfo?(fdjabri)
However, the problem seems to be fixed after applying the patch for Bug #1032768 from https://github.com/mozilla-b2g/gaia/pull/21529.
Comment on attachment 8466062 [details] [review]
Github patch

Hi Michael.

Could you please review the patch and if possible, can you figure out if there's any difference compared to https://github.com/mozilla-b2g/gaia/pull/21529 which Francis said worked better. To me it looks exactly the same :/
Attachment #8466062 - Flags: review?(mhenretty)
Target Milestone: 2.1 S1 (1aug) → 2.1 S2 (15aug)
(In reply to Markus Nilsson from comment #6)
> Comment on attachment 8466062 [details] [review]
> Github patch
> 
> Hi Michael.
> 
> Could you please review the patch and if possible, can you figure out if
> there's any difference compared to
> https://github.com/mozilla-b2g/gaia/pull/21529 which Francis said worked
> better. To me it looks exactly the same :/

Hi Markus...

FYI - I tried out both patches and to me they appeared the same as well.

- Rob
Flags: needinfo?(rmacdonald)
Assignee: nobody → markus.nilsson
Comment on attachment 8466062 [details] [review]
Github patch

I was also able to reproduce the problem in Francis' video. To reproduce:

1.) Flash your patch
2.) Go to Settings app
3.) Open the utility tray all the way.
4.) Close the utility tray
5.) Enable soft-home button
6.) Open the utility tray

Results: you will see that the utility tray icons at the bottom overlap the bottom of the utility tray.

Note: if you go into landscape mode in the browser, and then back into portrait, it seems to fix it. I think it has something to do with the utility tray icons at the bottom, because if you disable soft-home button and open the utility tray, they display higher than they should.
Attachment #8466062 - Flags: review?(mhenretty)
Hi Francis.

I've updated the github pull request with a fix for the problem shown in the video you attached.
Please review again.
Flags: needinfo?(fdjabri)
Hi Markus, 

I've tested the patch again. The old issue of the utility tray covering the soft home key is no longer there, but I noticed that banners cover the soft home key and the soft home key obscures buttons in full screen dialogs. See upcoming screenshots
Flags: needinfo?(fdjabri)
Attached image 2014-08-15-18-12-42.png
soft home key covers buttons
Attached image 2014-08-15-18-13-17.png
Toast covers soft home key
(In reply to Francis Djabri [:djabber] from comment #10)
> Hi Markus, 
> 
> I've tested the patch again. The old issue of the utility tray covering the
> soft home key is no longer there, but I noticed that banners cover the soft
> home key and the soft home key obscures buttons in full screen dialogs. See
> upcoming screenshots

Those are separate bugs. Covering the dialog buttons has already been filed here in bug 1048620. I just filed bug 1054716 to track the marketplace toaster issue.
Comment on attachment 8466062 [details] [review]
Github patch

Alive, can you take a look at this?
Attachment #8466062 - Flags: review?(alive)
Comment on attachment 8466062 [details] [review]
Github patch

r+ but the utility tray change deserves some tests.
Attachment #8466062 - Flags: review?(alive) → review+
Markus, let me know if you need some help with test writing.
Blocks: 1054716
Target Milestone: 2.1 S2 (15aug) → 2.1 S3 (29aug)
Flags: in-moztrap?(mozillamarcia.knous)
Markus, any movement on this?
Flags: needinfo?(markus.nilsson)
Updated the PR with a unit test. Sorry that it took so long.
Flags: needinfo?(markus.nilsson)
Comment on attachment 8466062 [details] [review]
Github patch

Hi Markus,

This patch needs to be rebased against master, since it will not merge as is right now.

Also, I'm not sure the unit test is testing your functionality. Since you stub getBoundingClientRect with adjustedHeight, your unit tests really end up comparing adjustedHeight with adjustedHeight. So for instance, you could fire 'software-button-enabled' on line 476 instead of 'software-button-disabled', and the tests will still pass. I think you need to fake the dom like utility_tray_test.js does in the it's setup for things like notifications and statusbar.

Let me know if I can help with this. Given the timeframe, we might need to do this in a followup.
Flags: needinfo?(markus.nilsson)
(In reply to Michael Henretty [:mhenretty] from comment #20)
> Comment on attachment 8466062 [details] [review]
> Github patch
> 
> Hi Markus,
> 
> This patch needs to be rebased against master, since it will not merge as is
> right now.

I've rebased the PR, should hopefully be OK now.

> Also, I'm not sure the unit test is testing your functionality. Since you
> stub getBoundingClientRect with adjustedHeight, your unit tests really end
> up comparing adjustedHeight with adjustedHeight. So for instance, you could
> fire 'software-button-enabled' on line 476 instead of
> 'software-button-disabled', and the tests will still pass. I think you need
> to fake the dom like utility_tray_test.js does in the it's setup for things
> like notifications and statusbar.
> 
> Let me know if I can help with this. Given the timeframe, we might need to
> do this in a followup.

I agree that it doesn't matter if you fire 'software-button-enabled' or 'software-button-disabled' (or 'resize'). What the test makes sure is that both events triggers a update of the cached screenHeight, so I wouldn't say that it's a test of adjustedHeight equals adjustedHeight.
If you would add a check if screenHeight and adjustedHeight are equal at line 469 or 476 they wouldn't be, it's only when the events are fired that screenHeight is updated to equal adjustedHeight.
Flags: needinfo?(markus.nilsson)
(In reply to Markus Nilsson from comment #21)
> I agree that it doesn't matter if you fire 'software-button-enabled' or
> 'software-button-disabled' (or 'resize'). What the test makes sure is that
> both events triggers a update of the cached screenHeight, so I wouldn't say
> that it's a test of adjustedHeight equals adjustedHeight.
> If you would add a check if screenHeight and adjustedHeight are equal at
> line 469 or 476 they wouldn't be, it's only when the events are fired that
> screenHeight is updated to equal adjustedHeight.

Fair enough. Let's land this sucka!
master: https://github.com/mozilla-b2g/gaia/commit/6da06cbed520b0bd9cb9f45d127ca6c3da2ab522
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
The issue is verified fixed in Flame 2.1 and Flame 2.2:

Flame 2.1 KitKat Base (319mb)(Full Flash)

Environmental Variables:
Device: Flame 2.1
BuildID: 20141007000203
Gaia: 7f738edf66b9298bceef8a4981d05d04fd04e540
Gecko: b9d04c58580a
Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Flame 2.2 KitKat Base (319mb)(Full Flash)

Environmental Variables:
Device: Flame 2.2 Master
BuildID: 20141007040205
Gaia: 83de447d9ae9a59459d7a445f9348a254c661850
Gecko: 9ee9e193fc48
Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

SHB is displayed correctly while the utility tray is open.
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: