Closed Bug 1205216 Opened 9 years ago Closed 9 years ago

API 23: SYSTEM_ALERT_WINDOW permission is not granted automatically and can't be requested

Categories

(Firefox for Android Graveyard :: Overlays, defect)

defect
Not set
normal

Tracking

(firefox46 fixed)

RESOLVED FIXED
Firefox 46
Tracking Status
firefox46 --- fixed

People

(Reporter: sebastian, Assigned: sebastian)

References

Details

Attachments

(9 files)

When targetSdkVersion is set to API 23 (new permission system!) then SYSTEM_ALERT_WINDOW is not granted automatically anymore (On devices running Android 6+). We use this permission to draw on other apps (Tab queues). Unfortunately this permission can't be requested at runtime. Instead the user has to manually(!) enable this for the app in the system settings: Settings -> Apps -> (gear icon) -> Draw over other apps -> Select app -> Enable "Permit drawing over other apps". We will need to guide Android 6+ users through this process when they want to enable tab queues. It's possible to use the Settings.ACTION_MANAGE_OVERLAY_PERMISSION Intent action to send the user directly to the settings page for a specific application. More: https://medium.com/@rotxed/drawing-over-other-apps-marshmallow-edition-987eff9f99a9 http://www.androidpolice.com/2015/09/07/android-m-begins-locking-down-floating-apps-requires-users-to-grant-special-permission-to-draw-on-other-apps/
I recently filed bug 1197189. Currently we crash if you revoke this permission on Android M.
Component: General → Overlays
Anthony: Do we have final mockups for this? This is the last piece I need to fix before I can start with runtime permissions. :)
Flags: needinfo?(alam)
Attached image flow_oib_Mperms1.png (deleted) —
Ah, this was the comment I had left in Aha FENN-321 but I should've posted it here. From what I've seen other apps do, I don't think our Helper UI needs to change. We do however, need to write some copy to use in the dialog as well as the heads up notification. Here's a rough UX flow with some placeholder text. I'll need to noodle on the copy a bit more. I've triggered this a few times in other apps but can't seem to find an example right now.
Flags: needinfo?(alam)
Attached image prev_oib_headsupnotif.png (deleted) —
Here's a rough mock of the Heads up notification as well if we need it.
Flags: needinfo?(s.kaspari)
Perfect. I'll start with this and post some screenshots. We can then start to tweak things if needed.
Flags: needinfo?(s.kaspari)
Attached image Facebook: Chat heads settings screen (deleted) —
Facebook now shows the following screen when you want to enable chat heads on Android 6 (Previously they have hidden the setting).
Actually, I was thinking about using our Helper UI here as well. Since we already have one for "Opening multiple links?" we could just forego the "Opt in dialog" screen and combine the messaging in the same Helper UI. That would save us 1 screen/ tap too!
Assignee: nobody → s.kaspari
Status: NEW → ASSIGNED
If we need an uplift-able patch then we could: * Disable the prompt after opening X external URLs on Android 6+ (To limit the number of people opting in until we have a good flow) * Just use the toast-without-button fallback, that we already have in place, for users enabling the feature in our settings => This doesn't need strings. To implement a better solution we need to (Android 6+): * attachment 8682272 [details] - Tell the user about the setting that needs to be changed. Then launch an intent to the specific page in Android settings. * Do the same from within our settings * Only enable the feature if the user has enabled the setting => Needs UI and strings
Additionally: * Show a notification whenever the feature is used but the permission is missing!
Attached image tabqueues_prompt_draw_over_apps.png (deleted) —
This is a first version of an adapted prompt. * If we do have the permission for drawing over other apps then the prompt looks and behaves like it does now. * If we do not have the permission then I show a slightly different text ("Turn on Permit drawing over other apps") and change the button to "Go to settings". * When switching to settings I show "Turn on Permit drawing over other apps" as a toast again * if the user enabled the setting and goes back then I enable tab queues and the user can continue to browse * If the user did not enable the setting then the prompt will still be there waiting.
Attachment #8706953 - Flags: feedback?(alam)
Comment on attachment 8706958 [details] MozReview Request: Bug 1205216 - Tab queue prompt: Ask user to enable "Draw over other apps" if needed. r=nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30545/diff/1-2/
Attached image tab_queue_notification_permission.png (deleted) —
This is how the notification would look like in a situation where we do not have the permission but tab queue is enabled. The notification will be shown as soon as a tab is queued.
Attachment #8706984 - Flags: feedback?(alam)
Attached image settings-prompt.png (deleted) —
The last entry point for enabling tab queues is settings: A simple and quick solution is to reuse our tab queue prompt here. We could later go ahead and improve this.
Attachment #8707467 - Flags: feedback?(alam)
Comment on attachment 8706958 [details] MozReview Request: Bug 1205216 - Tab queue prompt: Ask user to enable "Draw over other apps" if needed. r=nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30545/diff/2-3/
Comment on attachment 8707382 [details] MozReview Request: Bug 1205216 - Show notification if tab queue is enabled but permission is missing. r=nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30669/diff/1-2/
Comment on attachment 8706953 [details] tabqueues_prompt_draw_over_apps.png NICE! these diagrams are amazingly clear.
Attachment #8706953 - Flags: feedback?(alam) → feedback+
Comment on attachment 8706984 [details] tab_queue_notification_permission.png Perfect!
Attachment #8706984 - Flags: feedback?(alam) → feedback+
Comment on attachment 8707467 [details] settings-prompt.png Wfm!
Attachment #8707467 - Flags: feedback?(alam) → feedback+
Attachment #8706958 - Flags: review?(nalexander)
Attachment #8707382 - Flags: review?(nalexander)
Attachment #8707468 - Flags: review?(nalexander)
Comment on attachment 8706958 [details] MozReview Request: Bug 1205216 - Tab queue prompt: Ask user to enable "Draw over other apps" if needed. r=nalexander https://reviewboard.mozilla.org/r/30545/#review27739 I have nits and questions, but nothing that blocks landing. I'll leave it to you to respond and request a second pass if you feel it's necessary, or to land if there are no issues. ::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:114 (Diff revision 3) > +import android.provider.Settings; nit: extraneous? ::: mobile/android/base/java/org/mozilla/gecko/tabqueue/TabQueuePrompt.java:35 (Diff revision 3) > + private static final int SETTINGS_REQUEST_CODE = 1; Can we make this more likely to be unique? I worry about such things. ::: mobile/android/base/java/org/mozilla/gecko/tabqueue/TabQueuePrompt.java:82 (Diff revision 3) > + okButton.setVisibility(canDrawOverlays ? View.VISIBLE : View.GONE); Consider making this an `if` block, at your discretion. I understand this is shorter, but it's also harder to read. ::: mobile/android/base/java/org/mozilla/gecko/tabqueue/TabQueuePrompt.java:156 (Diff revision 3) > + Toast.makeText(this, R.string.tab_queue_prompt_permit_drawing_over_apps, Toast.LENGTH_LONG).show(); nit: kill blank line. ::: mobile/android/base/java/org/mozilla/gecko/tabqueue/TabQueuePrompt.java:167 (Diff revision 3) > + // User enabled the setting nit: trailing period. Also, let's use "permission" consistently when talking about this. It's possible to get here if the user does *not* enable the permission, right? (I.e., they hit hardware back and return with `RESULT_CANCEL`) or similar. What UI do they see in this case? Is the prompt still shown, with the same button to go to settings? If so, how do they cancel the prompt? ::: mobile/android/base/java/org/mozilla/gecko/tabqueue/TabQueuePrompt.java:168 (Diff revision 3) > + setResult(TabQueueHelper.TAB_QUEUE_YES); Should we be calling `onConfirmButtonPressed`, to get the nice animations? Also, we're losing the telemetry calls. I think we want them, since otherwise it'll look like no Android M users use Tab Queues. ::: mobile/android/base/locales/en-US/android_strings.dtd:248 (Diff revision 3) > +<!ENTITY tab_queue_prompt_permit_drawing_over_apps "Turn on Permit drawing over other apps"> Consider running this string by antlam; I don't know what the copy should be here. ::: mobile/android/base/locales/en-US/android_strings.dtd:253 (Diff revision 3) > +<!ENTITY tab_queue_prompt_settings_button "Go to settings"> nit: emphasize that it's an Android *permission* in the localization note, and not part of Fennec's settings. Is Settings right? Can we say "Android Settings", to distinguish from Fennec's preferences? Also, I see that we use "Settings" in `tab_queue_prompt_tip_text2` and "settings" in `tracking_protection_prompt_tip_text`. Can you make the capitalization uniform while you're here? (There is no need to rev the entity names for a cosmetic change like this.) ::: mobile/android/base/resources/layout/tab_queue_prompt.xml:76 (Diff revision 3) > + tools:text="Turn on Permit drawing over other apps" /> Nice! (Although, I have a ticket -- languishing -- that should make this no longer necessary. Thanks for the reminder. (Bug 1074258))
Attachment #8706958 - Flags: review?(nalexander) → review+
Comment on attachment 8707382 [details] MozReview Request: Bug 1205216 - Show notification if tab queue is enabled but permission is missing. r=nalexander https://reviewboard.mozilla.org/r/30669/#review27741 I see no issues. I wish it was easier to do this "in App": i.e., show a persistent snackbar or doorhanger under the URL bar when searching about things that are in bad state. We could surface bad configurations like this one; but also, Sync and Firefox Account error states, etc. Oh well, not blocking this.
Attachment #8707382 - Flags: review?(nalexander) → review+
Comment on attachment 8707468 [details] MozReview Request: Bug 1205216 - GeckoPreferences: Ask user to enable "Draw over other apps" if needed. r=nalexander https://reviewboard.mozilla.org/r/30717/#review27743 One major concern, which I trust you to address. ::: mobile/android/base/java/org/mozilla/gecko/preferences/GeckoPreferences.java:1245 (Diff revision 1) > + startActivityForResult(promptIntent, REQUEST_CODE_TAB_QUEUE); As discussed on IRC, it's not clear to me that the instance that invokes `startActivityForResult` is the same instance that receives the `onActivityResult` callback. Therefore, let's be safe and always maintain the `tabQueuePreference`, regardless of whether we've interacted with this preference. That way we know that preference is never null. (Of course, we'll never release it either.) ::: mobile/android/base/java/org/mozilla/gecko/tabqueue/TabQueueHelper.java:295 (Diff revision 1) > + return enabled; You could also `return resultCOde == TAB_QUEUE_YES`, at your discretion.
Attachment #8707468 - Flags: review?(nalexander) → review+
https://reviewboard.mozilla.org/r/30545/#review27739 > Can we make this more likely to be unique? I worry about such things. This only needs to be unique for *this* activity, doesn't it? > nit: trailing period. > > Also, let's use "permission" consistently when talking about this. It's possible to get here if the user does *not* enable the permission, right? (I.e., they hit hardware back and return with `RESULT_CANCEL`) or similar. What UI do they see in this case? Is the prompt still shown, with the same button to go to settings? If so, how do they cancel the prompt? Yes, the prompt is still shown. They can dismiss it by pressing back or pressing almost anywhere else. > Should we be calling `onConfirmButtonPressed`, to get the nice animations? > > Also, we're losing the telemetry calls. I think we want them, since otherwise it'll look like no Android M users use Tab Queues. I didn't call onConfirmButtonPressed to avoid showing the animation while there's already the animation from dismissing Android's settings activity. But I added the missing telemetry call!
Comment on attachment 8706958 [details] MozReview Request: Bug 1205216 - Tab queue prompt: Ask user to enable "Draw over other apps" if needed. r=nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30545/diff/3-4/
Attachment #8706958 - Attachment description: MozReview Request: Bug 1205216 - Tab queue prompt: Ask user to enable "Draw over other apps" if needed. r? → MozReview Request: Bug 1205216 - Tab queue prompt: Ask user to enable "Draw over other apps" if needed. r=nalexander
Comment on attachment 8707382 [details] MozReview Request: Bug 1205216 - Show notification if tab queue is enabled but permission is missing. r=nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30669/diff/2-3/
Attachment #8707382 - Attachment description: MozReview Request: Bug 1205216 - Show notification if tab queue is enabled but permission is missing. r? → MozReview Request: Bug 1205216 - Show notification if tab queue is enabled but permission is missing. r=nalexander
Comment on attachment 8707468 [details] MozReview Request: Bug 1205216 - GeckoPreferences: Ask user to enable "Draw over other apps" if needed. r=nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30717/diff/1-2/
Attachment #8707468 - Attachment description: MozReview Request: Bug 1205216 - GeckoPreferences: Ask user to enable "Draw over other apps" if needed. r? → MozReview Request: Bug 1205216 - GeckoPreferences: Ask user to enable "Draw over other apps" if needed. r=nalexander
https://hg.mozilla.org/integration/fx-team/rev/c9d99a2255f50b45d7572c8e8ee4b441ae9ccffa Bug 1205216 - Tab queue prompt: Ask user to enable "Draw over other apps" if needed. r=nalexander https://hg.mozilla.org/integration/fx-team/rev/6e5c16c8d3e79fca9cefcfb6a8d7161468b45cf1 Bug 1205216 - Show notification if tab queue is enabled but permission is missing. r=nalexander https://hg.mozilla.org/integration/fx-team/rev/05078e713365050c2a06e5e3e8728fd72c075b64 Bug 1205216 - GeckoPreferences: Ask user to enable "Draw over other apps" if needed. r=nalexander
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
The quotes in the notification don't show, dagnabbit http://developer.android.com/guide/topics/resources/string-resource.html#scaping_quotes, needs \&quot;. Thanks to Sebastian for testing. Probably needs a follow-up, but no ID change.
Depends on: 1244241
QA Contact: teodora.vermesan
Depends on: 1244722
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: