1224281 - secure window manager does not give focus back to camera app after geolocation dialog is dismissed

Status: RESOLVED FIXED

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

Description

[David Flanagan [:djf]]

This is a followup to bug 1176913.  If you have a screenlock PIN, and launch the camera from the lockscreen, and it displays the geolocation permission prompt to you, then after you dismiss the permission, the volume keys won't work until you touch the screen somewhere. To reproduce this you need to get the geolocation permission dialog to come up, and you can do that with Settings->App Permissions->Camera

This does not reproduce when the camera is not in secure mode, so I think we already have code in the regular window manager to handle focus in this case, and just need to copy that code to the secure window manager.

Comment 1

[David Flanagan [:djf]]

Greg,

I've taken this bug so that I don't forget about it. But I suspect that you are more qualified to fix it than I am, so feel free to take it from me and fix it yourself.

See https://bugzilla.mozilla.org/show_bug.cgi?id=1176913#c24 for more details on the bug.  It seems pretty clear to me that this is another focus management issue and the volume button key events are not being sent to the camera app in this one specific case where there is a lockscreen pin and the permission dialog was shown.

Comment 2

[Greg Weng [:snowmantw][:gweng][:λ]]

I'm afraid that it's a bit more complicated than just fix it in SecureWindowManager, since before his leave Alive implemented the HierarchyManager, which is supposed to manager different windows, dialogs and layers. Unfortunately, I were totally out of his work since at the moment I also focused on my LockScreen tasks.

So, the first thing is I need to drill down the code to see now focuses are supposed to be managed in which component, and then give a fix. However, since I'm now stuck at some ongoing Gij works and system performance issue, I can only keep it in my queue first.

Comment 3

[Greg Weng [:snowmantw][:gweng][:λ]]

And I keep one NI to remind me about the bug.

Comment 4

[David Flanagan [:djf]]

There is another case here. If the camera is running from the secure window manager and you press and hold the power button to bring up the sleep menu, then dismiss that menu, then focus is also lost and the volume keys no longer take a picture.

app_window_manager has this code, and I think we need the same for the secure case:

    // XXX: Remove this once permission dialog is moved into AppWindow.
    '_handle_permissiondialoghide': function() {
      this._activeApp && this._activeApp.broadcast('focus');
    },

    // When the sleep menu is hidden we need to re-focus the active app
    // or key events won't be properly dispatched to it. See bug 1200351.
    '_handle_sleepmenuhide': function() {
      this._activeApp && this._activeApp.broadcast('focus');
    },

Comment 5

[GitHub Autolander Bot]

Attachment: [gaia] davidflanagan:bug1224281 > mozilla-b2g:master

Comment 6

[David Flanagan [:djf]]

Comment on attachment 8687322 [details] [review]
[gaia] davidflanagan:bug1224281 > mozilla-b2g:master

Greg,

I fixed this the same way I fixed bug 1176913. It may not be a perfect fix, but since it parallels existing code, maybe it is good enough for now.

Comment 7

[Greg Weng [:snowmantw][:gweng][:λ]]

Comment on attachment 8687322 [details] [review]
[gaia] davidflanagan:bug1224281 > mozilla-b2g:master

Okay, from the code I think your solution is perfect enough for the current architecture, and it also indicates to this bug with discussions. So here is the r+. Thanks for patching.

Comment 8

[David Flanagan [:djf]]

Landed on master: https://github.com/mozilla-b2g/gaia/commit/5ea88ea368f48482e484d9a6af25ad001996f0c4

Comment 9

[David Flanagan [:djf]]

Comment on attachment 8687322 [details] [review]
[gaia] davidflanagan:bug1224281 > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): this is not a regression, but it is a followup to bug 1176913, which landed in time for 2.5. It would be nice to have the complete fix in 2.5.

[User impact] if declined: This is an relatively hard to trigger bug: users who have a lockscreen pin and use the camera from lockscreen won't be able to use the hardware keys to take a picture if they accidentally show the power menu first, or on the first run after the geolocation prompt.

[Testing completed]: yes

[Risk to taking this patch] (and alternatives if risky): not risky

[String changes made]: none

Comment 10

[Mahendra Potharaju [:mahe]]

Comment on attachment 8687322 [details] [review]
[gaia] davidflanagan:bug1224281 > mozilla-b2g:master

Approved for 2.5. 

Thanks

Comment 11

[Carsten Book [:Tomcat]]

for 2.5 https://github.com/mozilla-b2g/gaia/commit/39d8e98bbd22b4409b82c496d5adc8323924a05a