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)
Tracking
(feature-b2g:2.1, 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.
Updated•10 years ago
|
feature-b2g: --- → 2.1
Target Milestone: --- → 2.1 S1 (1aug)
Assignee | ||
Comment 1•10 years ago
|
||
Assignee | ||
Comment 2•10 years ago
|
||
Hi Rob.
Please take a look at the attached github pull request.
Flags: needinfo?(rmacdonald)
Reporter | ||
Comment 3•10 years ago
|
||
Hi Francis - Sorry I'm still having problems applying patches. Can you review this patch from Markus as well? - Rob
Flags: needinfo?(fdjabri)
Comment 4•10 years ago
|
||
After applying this patch, it seems as if the Utility tray is still overlapping the software home button bar.
Flags: needinfo?(fdjabri)
Comment 5•10 years ago
|
||
However, the problem seems to be fixed after applying the patch for Bug #1032768 from https://github.com/mozilla-b2g/gaia/pull/21529.
Assignee | ||
Comment 6•10 years ago
|
||
Comment on attachment 8466062 [details]
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)
Updated•10 years ago
|
Target Milestone: 2.1 S1 (1aug) → 2.1 S2 (15aug)
Reporter | ||
Comment 7•10 years ago
|
||
(In reply to Markus Nilsson from comment #6)
> Comment on attachment 8466062 [details]
> 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)
Updated•10 years ago
|
Assignee: nobody → markus.nilsson
Comment 8•10 years ago
|
||
Comment on attachment 8466062 [details]
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)
Assignee | ||
Comment 9•10 years ago
|
||
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)
Comment 10•10 years ago
|
||
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)
Comment 11•10 years ago
|
||
soft home key covers buttons
Comment 12•10 years ago
|
||
Toast covers soft home key
Comment 13•10 years ago
|
||
(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 14•10 years ago
|
||
Comment on attachment 8466062 [details]
Github patch
Alive, can you take a look at this?
Attachment #8466062 -
Flags: review?(alive)
Comment 15•10 years ago
|
||
Comment on attachment 8466062 [details]
Github patch
r+ but the utility tray change deserves some tests.
Attachment #8466062 -
Flags: review?(alive) → review+
Comment 16•10 years ago
|
||
Markus, let me know if you need some help with test writing.
Updated•10 years ago
|
Target Milestone: 2.1 S2 (15aug) → 2.1 S3 (29aug)
Updated•10 years ago
|
Flags: in-moztrap?(mozillamarcia.knous)
Assignee | ||
Comment 19•10 years ago
|
||
Updated the PR with a unit test. Sorry that it took so long.
Flags: needinfo?(markus.nilsson)
Comment 20•10 years ago
|
||
Comment on attachment 8466062 [details]
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)
Assignee | ||
Comment 21•10 years ago
|
||
(In reply to Michael Henretty [:mhenretty] from comment #20)
> Comment on attachment 8466062 [details]
> 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)
Comment 22•10 years ago
|
||
(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!
Comment 23•10 years ago
|
||
Comment 24•10 years ago
|
||
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?]
status-b2g-v2.2:
--- → verified
Flags: needinfo?(ktucker)
Updated•10 years ago
|
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.
Description
•