1175461 - Truncated permission dialog when locked

Status: VERIFIED FIXED

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

Description

[Alberto Pastor [:albertopq]]

User-Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

DTR

- Leave the phone in the lockscreen with Adb and Devtools access

- Try to connect via WebIde

Expected

The connect permission dialog is not truncated by the SHB

Actual

See screenshot

Comment 1

[Alberto Pastor [:albertopq]]

Attachment: proc-vmstat.log

Comment 2

[Alberto Pastor [:albertopq]]

Attachment: proc-vmallocinfo.log

Comment 3

[Alberto Pastor [:albertopq]]

Attachment: proc-version.log

Comment 4

[Alberto Pastor [:albertopq]]

Attachment: system-b2g-application.ini

Comment 5

[Alberto Pastor [:albertopq]]

Attachment: proc-uptime.log

Comment 6

[Alberto Pastor [:albertopq]]

Attachment: proc-meminfo.log

Comment 7

[Alberto Pastor [:albertopq]]

Attachment: proc-kmsg.log

Comment 8

[Alberto Pastor [:albertopq]]

Attachment: proc-cmdline.log

Comment 9

[Alberto Pastor [:albertopq]]

Attachment: dev-log-system.log

Comment 10

[Alberto Pastor [:albertopq]]

Attachment: dev-log-radio.log

Comment 11

[Alberto Pastor [:albertopq]]

Attachment: properties.log

Comment 12

[Alberto Pastor [:albertopq]]

Attachment: dev-log-main.log

Comment 13

[Alberto Pastor [:albertopq]]

Attachment: screenshot.png

Comment 14

[Alberto Pastor [:albertopq]]

[Blocking Requested - why for this release]: Given that the STR is (may be) part of the Spark documentation to set up the device, this will be part of the first impression of the device.

Comment 15

[GitHub Autolander Bot]

Attachment: [gaia] jedireza:lockscreen-dialog-clipping > mozilla-b2g:master

Comment 16

[Reza Akhavan [:jedireza]]

I think I found a simple solution to this. I'm hoping my results can be validated before asking for a formal review. Locally tests are showing green lights.

Comment 17

[Michael Henretty [:mikehenrty][:mhenretty]]

Overall, I like the idea of this change, ie. should leverage the platform whenever possible to do re-sizing. However, did you check the different places the modal_dialog was being used to make sure it still worked when doing things like bringing up and dismissing the keyboard? We *should* have tests for these scenarios, but being the pessimist that I am, I am skeptical they cover our bases here. So making a change like this, while it seems like the right thing to do™, in practice will require a bunch of research.

Comment 18

[Reza Akhavan [:jedireza]]

> However, did you check the different places the modal_dialog was being used
> to make sure it still worked when doing things like bringing up and dismissing
> the keyboard?

I did try the prompt dialog (which has a text field) using the existing code and with my patch. In both cases the keyboard is brought up and dismissed the same way. The dialog doesn't resize itself in relation to the keyboard. And I wouldn't expect it to since the height is simply `LayoutManager.height - Statusbar.height`.

> ...while it seems like the right thing to do™, in practice will require a bunch of research.

I'm open to recommendations and guidance. With what I know now, combing through code looking for places where this is used and trying it out sounds like what's required. But doesn't sound very productive of us.

Comment 19

[Michael Henretty [:mikehenrty][:mhenretty]]

(In reply to Reza Akhavan [:jedireza] from comment #18)
> I'm open to recommendations and guidance. With what I know now, combing
> through code looking for places where this is used and trying it out sounds
> like what's required. But doesn't sound very productive of us.

Yeah, the problem with this type of change is the potential amount of time spent researching why things are the way the are, and then what the impact is for changing it. What I generally do is grep for all the places where the modules is used, and if it's small enough to test each use case then move forward with this approach. It sounds like this is already changing the behavior of window.prompt, which could break if there was a lot of text in it? I'm not sure, but overall this change sounds risky. 

Another approach I would look into is listening for the `system-resize` event (which is published by the layout manager for various reasons including unlocking the phone), and then updating the height of the dialog in the handler if the dialog is shown. It doesn't improve the situation the way your patch does, but it feels less risky.

Comment 20

[GitHub Autolander Bot]

Attachment: [gaia] jedireza:lockscreen-dialog-clipping-alternative > mozilla-b2g:master

Comment 21

[Reza Akhavan [:jedireza]]

New PR using a less risky approach. It is visually noticeable when the dialog is resized.

Comment 22

[Michael Henretty [:mikehenrty][:mhenretty]]

WFM

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/881c6ef2e8ea4fea6a0681e9f8fa7b70d41e35a9

Comment 24

[Pi Wei Cheng [:piwei] (inactive)]

This issue is verified fixed on Aries. The dialog is no longer obscured by the SHB bar. There is a missing step on original STR: the phone needs to be unlocked in order to see that dialog. I was confused when I flashed to a build around reported date trying to repro the bug first.

Device: Aries (dogfood debug build)
BuildID: 20150721153949
Gaia: 805cf546729ba742bf23febda52970fcb35c0e8f
Gecko: 512c7e8f0030
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 42.0a1 (2.5 Master) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0