Closed Bug 1074608 Opened 7 years ago Closed 7 years ago

[SHB] Permission prompt doesn't cover screen with SHB enabled

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

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

VERIFIED FIXED
2.1 S6 (10oct)
blocking-b2g 2.1+
Tracking Status
b2g-v2.0 --- wontfix
b2g-v2.1 --- verified
b2g-v2.2 --- verified

People

(Reporter: mikehenrty, Assigned: mikehenrty)

References

Details

(Whiteboard: [systemsfe])

Attachments

(4 files, 2 obsolete files)

When SHB is enabled, the permission prompt does not display correctly.
[Blocking Requested - why for this release]:

SHB is a requirement for Tako, and taking a look at the screenshot it seems this is bad enough to block the release on.
blocking-b2g: --- → 2.1?
Attached image 2014-09-30-16-24-57.png
Also reproduced with SIM PIN dialog
Broken feature
blocking-b2g: 2.1? → 2.1+
Assignee: nobody → mhenretty
Adding qawanted for branch checks.
Keywords: qawanted
QA Contact: ckreinbring
Just as an FYI, there are at least four issues going on here.

1.) In portrait mode, the prompt doesn't cover the screen.
2.) In landscape mode, the prompt covers the SHB.
3.) The prompt overlaps the statusbar.
4.) Comment 2.

Comment 2 is bug 1074123. I already have a WIP patch that solves that, so I'll probably just split that out and upload there.
The bug repros on Flame 2.2, Flame 2.1, Flame 2.0 and Flame 2.0 Base using shallow flash.
Actual result:  The permission pages are rendered with exposed space at the bottom of the screen if the Software Home Screen option is enabled.

Flame 2.2
BuildID: 20141001060621
Gaia: a23d2c490b39c4699c9375e25c4acdf396a2fa85
Gecko: 835ef55e175e
Platform Version: 35.0a1
Firmware Version: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Flame 2.1
BuildID: 20141001105624
Gaia: c1cc61e30f5cb3446f1692ff9fd1e32232f6a231
Gecko: 3b859b93de23
Platform Version: 34.0a2
Firmware Version: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Flame 2.0
BuildID: 20141001123531
Gaia: 8079cba2133e6f5443dba24dad077f7f91e6c978
Gecko: 3a656e533675
Platform Version: 32.0
Firmware Version: V180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Flame 2.0 Base
BuildID: 20140904160718
Gaia: 506da297098326c671523707caae6eaba7e718da
Gecko: 
Platform Version: 32.0
Firmware Version: V180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
Flags: needinfo?(jmitchell)
Keywords: qawanted, regression
Alive for review. Eli for feedback since he has been doing a lot of work in SHB recently, and might know if this could break something else.
Attachment #8498544 - Flags: review?(alive)
Attachment #8498544 - Flags: feedback?(eperelman)
QA Whiteboard: [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
Comment on attachment 8498544 [details] [review]
[Gaia PR] Fix permission prompt with SHB

nit in github
Attachment #8498544 - Flags: review?(alive) → review+
Attachment #8498544 - Flags: feedback?(eperelman) → feedback+
Comment on attachment 8498544 [details] [review]
[Gaia PR] Fix permission prompt with SHB

[Approval Request Comment]

[Bug caused by] (feature/regressing bug #):
This bug has been around since at least 2.0, but we've never cared that much about the software home button.

[User impact] if declined:
Majorly broken permission dialogs, especially in landscape mode.

[Testing completed]:
Manual testing by multiple people on 2.2 and 2.1. No automated tests yet, since these are CSS only fixes and we have no easy way to test SHB yet. This is my first rule of business after feature complete: automated tests for SHB.

[Risk to taking this patch] (and alternatives if risky):
Risk is mainly to permissions prompts when software home button is enabled. Honestly, they cannot get much worse than they already are, so low risk.


[String changes made]: none
Attachment #8498544 - Flags: approval-gaia-v2.1?
Comment on attachment 8498544 [details] [review]
[Gaia PR] Fix permission prompt with SHB

I don't think this is ready for uplift as it's still broken in master. Attaching a follow-up which will fix things shortly.
Attachment #8498544 - Flags: approval-gaia-v2.1?
Eli - Could you review this for me? Thanks!
Attachment #8499929 - Flags: review?(eperelman)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Please make sure Permission Dialogs still work in landscape mode. It looks like this change might break that, and we don't have a way to test for that yet.

Also, can you give an example where this was broken, so we can consider it on our refactor?
Comment on attachment 8499929 [details]
Pull request - follow-up, handle nested dialog in permission prompt

Pretty much all permission dialogs are currently broken on truck. I think something may have landed after to cause it though, so maybe we could fix it, but probably better to ensure everything works.

Yeah, probably breaks landscape - I'll double check before getting a review.
Attachment #8499929 - Flags: review?(eperelman)
Target Milestone: --- → 2.1 S6 (10oct)
Hey Guys - here's the updated fix and test. Let me know what you think. Thanks!
Attachment #8499964 - Attachment is obsolete: true
Attachment #8500523 - Flags: review?(mhenretty)
Attachment #8500523 - Flags: review?(eperelman)
Comment on attachment 8500523 [details] [review]
Pull request - follow-up, handle nested dialog in permission prompt

r+ since this adds a much needed test. In bug 1077579 we will be removing those `@media not all and (-moz-physical-home-button)` definitions entirely [1], but this is a good fix for now. Plus it's a good reminder to me that I need to test this on more than just a flame.

1.) https://github.com/mozilla-b2g/gaia/pull/24778/files#diff-d4acccad5fac312368cadcbf5287bcb0L44
Attachment #8500523 - Flags: review?(mhenretty)
Attachment #8500523 - Flags: review?(eperelman)
Attachment #8500523 - Flags: review+
Thanks Mike! There is a gaia-try failure, but this was on trunk, and resolved with a landing upstream, so I feel comfortable landing this. 

In master: https://github.com/mozilla-b2g/gaia/commit/39b0d0e568d9da0b9262247286d2489b3b4be5cd
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Comment on attachment 8498544 [details] [review]
[Gaia PR] Fix permission prompt with SHB

See comment 10 for the original approval request. Copied here:

[Approval Request Comment]

[Bug caused by] (feature/regressing bug #):
This bug has been around since at least 2.0, but we've never cared that much about the software home button.

[User impact] if declined:
Majorly broken permission dialogs, especially in landscape mode.

[Testing completed]:
Manual testing by multiple people on 2.2 and 2.1. No automated tests yet, since these are CSS only fixes and we have no easy way to test SHB yet. This is my first rule of business after feature complete: automated tests for SHB.

(Update: Automated tests are in-progress, and have been landing on master now. The other commit in this bug comes with an automated test)

[Risk to taking this patch] (and alternatives if risky):
Risk is mainly to permissions prompts when software home button is enabled. Honestly, they cannot get much worse than they already are, so low risk.


[String changes made]: none
Attachment #8498544 - Flags: approval-gaia-v2.1?(fabrice)
Comment on attachment 8500523 [details] [review]
Pull request - follow-up, handle nested dialog in permission prompt

Please see the previous comment. This bug should be uplifted with both patches.
Attachment #8500523 - Flags: approval-gaia-v2.1?(fabrice)
Duplicate of this bug: 1078821
This issue is verified fixed on 2.2 Flame

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

The permission prompt is covering the whole screen.
------------------------------------------------------------------------

The issue mentioned in Comment 2 with the SIM PIN page is being tracked in bug 1074123.

------------------------------------------------------------------------

Leaving verifyme tag on bug because 2.1 has not been uplifted yet.
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Keywords: verifyme
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Attachment #8500523 - Flags: approval-gaia-v2.1?(fabrice) → approval-gaia-v2.1+
Attachment #8498544 - Flags: approval-gaia-v2.1?(fabrice) → approval-gaia-v2.1+
Issue verified as fixed on 2.1

Device: Flame 2.1 KK (319mb) (Full Flash)
BuildID: 20141012001201
Gaia: d18e130216cd3960cd327179364d9f71e42debda
Gecko: 610ee0e6a776
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

With Software Home Button enabled the permissions screen covers available screen space fully
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Keywords: verifyme
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Integration test added in bug 1077579.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.