improvement: permission explanations #18978

ericli3690 · 2025-07-29T05:30:37Z

Adds permission explanation screen to OS settings to explain to the user why we need the permissions we request, via a new AllPermissionsExplanationFragment fragment hosted in a new PermissionsExplanationActivity. Arthur requested that I create a new activity for this purpose.

Purpose / Description

Launches the above activity via an intent filter defined in AndroidManifest.xml, as per the docs.
Added comments to AndroidManifest.xml to flag that future maintainers should add any new runtime permissions to AllPermissionsExplanationFragment.
Moved the logic for requesting external storage from the children of PermissionsFragment to the PermissionsFragment abstract class itself. This allows the methods to be called in both the PermissionsFragment children and the AllPermissionsExplanationFragment, minimizing code duplication.
Added strings for telling the user that they need to grant or revoke permissions manually from the OS settings, strings for the permissions explanations, etc.
Increased the amount of maximum lines in a PermissionItem's description so I could fit a longer explanation of why AnkiDroid needs notification permissions.
Added POST_NOTIFICATIONS and RECORD_AUDIO to constants.xml so they can be referred to by AllPermissionsExplanation.

How the permissions explanation screen looks on a Play Store build:

An example of the permissions explanation screen on a full debug build (not Play Store) (the strings in this screenshot are outdated, see the above screenshot for updated strings):

The snackbar that shows up when you try to revoke a permission:

Fixes

Requested by Brayan. It's best to explain why we need permissions not only when we are requesting them in-app, but also whenever the user goes to check on them in the OS settings.

Approach

Creates a new activity as requested by Arthur.
Based on my understanding of the app's permissions from reading AndroidManifest, it seems there are only really three kinds of permissions that we need to request from the user: externally managed storage, notifications, and microphone. Legacy storage permissions are only available until API 29, and this permissions explanation screen only shows up on API 31+. Please suggest wording changes if you dislike any wording I've used.

How Has This Been Tested?

Physical Samsung S23, API 34
Emulated Pixel 3a, API 28
Emulated Pixel 5, API 30
Emulated Pixel 6a, API 33

Checklist

You have a descriptive commit message with a short title (first line, max 50 chars).
You have commented your code, particularly in hard-to-understand areas
You have performed a self-review of your own code
UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
UI Changes: You have tested your change using the Google Accessibility Scanner

ericli3690 · 2025-07-29T05:35:01Z

Need people to review. Please let me know if:

You want any UI changes. Right now, it's very basic.
You think any wording is unclear or incorrect.
I've missed any permissions that should be on this list.
You know how I can test the managed-storage permissions. Is there some setting in the emulator I can tick to make it think I've installed a GitHub / F-Droid build, for instance?

Haven't tested on different API versions yet. The above image is how it looks on my phone, which is a Samsung S23, API 34. I wanted to get UX / implementation feedback first before committing more time to testing.

ericli3690 · 2025-07-29T05:44:25Z

Oops, forgot the copyright header and had to add the new activity to the ActivityList for tests. Should be fixed now.

BrayanDSO

Could you try reusing PermissionsActivity? Adding these permissions under an Optional section there should work.

ericli3690 · 2025-08-01T04:45:55Z

Refactored to use PermissionsActivity. Refactored the permission fragments to minimize code duplication, fixed some issues. See the PR description for an updated changelog.

I've done more testing and things seem to be working. The only slightly weird bit of behaviour is that if the user clicks on a permission item to enable it, but then hits "do not grant" in the dialog, the fragment thinks the dialog to enable permissions failed and opens the OS settings. I can't think of a way to stop this from happening, but I think it's actually alright behaviour. If the user clicks on a permission item that's currently disabled, then we can assume that they want to grant that permission, and so it makes sense that we show them the ability to toggle the permission in the OS settings if they immediately click "do not grant" on the dialog.

Important question: Does anyone know how I can test the externally-managed-storage permissions? I haven't been able to test these yet.

Also, as I mentioned in Discord, Full30and31PermissionsFragment and TiramisuPermissionsFragment are now essentially identical. Can I delete TiramisuPermissionsFragment?

david-allison · 2025-08-01T17:03:12Z

AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/permissions/AllPermissionsExplanation.kt

+        // Microphone
+        // Minimum API 23, which is less than AnkiDroid's minimum targeted API version, so we show it unconditionally
+        recordAudioPermission.apply {
+            isVisible = true
+            offerToGrantOrRevokeOnClick(Permissions.recordAudioPermission)
+        }


We currently show this when a user wants to record audio.

We /may/ want to inform users, but I feel it's just additional cognitive overhead here.
We don't need mic access to run the app

Hm, I actually disagree with this point. Notice in the UI above that when the user is in the OS settings, they can click on a little info button next to the microphone permission (the microphone permission shows up in the OS settings because we've declared it in the manifest file). I personally feel it would be weird to a user if they clicked an info button next to the microphone permission and proceeded to... not learn any new info about the microphone permission. Picture, for instance, a user who has forgotten that they once granted AnkiDroid the permission to access the microphone. Opening up the OS settings and seeing an explanation-less microphone permission might upset them a bit.

david-allison · 2025-08-01T17:04:45Z

AnkiDroid/src/main/res/values/12-dont-translate.xml

    <!-- TODO: Move this to a proper string resource file and collect all other strings associated with this feature once it is stable. -->
    <string name="schedule_reminders_do_not_translate" maxLength="28" comment="Do not translate this string, this feature is still in development. Name of the screen that appears when review reminders are being scheduled.">Schedule reminders</string>
+    <string name="manually_grant_permissions_do_not_translate" comment="Do not translate this string, this feature is still in development. Pop-up message that is shown to the user when they want to grant an app permission that must be granted via the OS settings screen.">This permission must be manually granted from Settings</string>
+    <string name="revoke_permissions_do_not_translate" comment="Do not translate this string, this feature is still in development. Pop-up message that is shown to the user when they are revoking an app permission.">This permission must be manually revoked from Settings</string>


This doesn't seem to be a dev only feature

Oop, good point. I've moved them to 10-preferences, let me know if someplace else would be better!

Found a better place in 01-core. I'll group these, and the rest of the strings I need for this feature, together with the strings used on the first-time-install permissions screen.

Arthur-Milchior

My first remark is that this commit is too big.
For example, setOnSwitchClickListener is called in three places, so I think that a first commit that add the parameter to the listener would be an acceptable commit by itself.

Something that shows that there is a problem its that your commit message seems like a bunch of unrelated changes. I don't really feel that reading this commit message helped me understand what I was going to review.

I think your commit message would generally need a first section that explains why you're doing this.

In this case, it would be something such as

Android offers each app a way to explains to show to the user which permissions the app request and why. This was not yet used by AnkiDroid. This commit introduce this page. In order to access it, the user must (list the steps. I'm really not sure what need to be done here)

Admitttedly, it'd be better if we could have the image in the commit message, as your PR description makes it very clear how to access this view, but we don't have such a feature, so it'll need to be text, or at worse a link to the image hosted on github.

Then you can explain why, in order to achieve your goal, you have to do other things.

Arthur-Milchior · 2025-08-01T21:57:02Z

AnkiDroid/src/main/AndroidManifest.xml

            android:configChanges="keyboardHidden|orientation|screenSize"
            />
+
+        <!-- Activity that hosts PermissionsFragments for requesting permissions from the user -->


Please don't close the comment at the end of the line. Be consistent with the remaining of the code. <!-- is multiline.

Arthur-Milchior · 2025-08-01T22:06:18Z

AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/permissions/AllPermissionsExplanation.kt

+            ActivityResultContracts.RequestMultiplePermissions(),
+        ) { requestedPermissions ->
+            if (!requestedPermissions.all { it.value }) {
+                // The permission dialog did not show up or the user denied the permission


If I understand correctly the code, requestedPermissions is a set of permission.
If there is at most one element, then your code should just use first or some method to get this element and note explicitely that it's the only element, maybe even check that it's the case.
If it can contains multiple element, the comment should indicate that this code is executed if any of the permission is denied.

That would make the code more verbose, though. I feel like requestedPermission.all is clean enough and that adding special case handling for if the hashmap contains one item or multiple is a bit too complex.

What would make the code more complex?
Now that I've read more, I see there can indeed be multiple permissions. In which case, I just request that you updated your comment to use "permissions" instead of "permission"

Thanks for implementing this at #19033!

Arthur-Milchior · 2025-08-01T22:07:06Z

AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/permissions/AllPermissionsExplanation.kt

+class AllPermissionsExplanation : PermissionsFragment(R.layout.all_permissions_explanation) {
+    /**
+     * Attempts to open the dialog for granting a permission. Falls back to opening the OS settings if
+     * the dialog fails to show up. This may happen if the user has previously denied the permission multiple times,


According to the comment below, it also shows the os settings if some permission is rejected by the user. This should be documented

Implemented.

Arthur-Milchior · 2025-08-02T16:00:05Z

AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/permissions/AllPermissionsExplanation.kt

+        val recordAudioPermission = view.findViewById<PermissionItem>(R.id.record_audio_permission_item)
+
+        // External storage
+        if (Permissions.canManageExternalStorage(requireContext())) {


I admit that I found canManageExternalStorage name to be strange. I was wondering why you'd add a preference if we can already manage it.
I think it would be helpful if, when you use a non-documented method, that you add documentation to it if you can.
In this case, I discovered while reading the implementation that actually it means "Is it possible for this installation of android to eventually manager external storage(potentially after requesting the permission to android)" and not "can this installation of ankidroid can currently manage the external storage"

I would admittedly prefer a clearer name, but I can't think of any name that is not absurdly long, such as manifestDeclaresExternalStoragePermission. So at least, having its meaning documented would help the next reader that is not familiar with the permission code

Thanks for documenting this at #19034!

Arthur-Milchior · 2025-08-02T16:22:34Z

AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/permissions/AllPermissionsExplanation.kt

+            }
+        }
+        // Notifications
+        if (Permissions.postNotification != null) {


I'd go with

Permissions.postNotification ?.let { notificationPermission.apply { isVisible = true offerToGrantOrRevokeOnClick(it) } }

Admittedly, here, postNotification is a value, so we know it can't become null, you don't have to use !! inside the code, so implementor choice.

Implemented!

Arthur-Milchior · 2025-08-02T21:40:01Z

AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/permissions/PermissionsActivity.kt


 /**
 * Screen responsible for getting permissions from the user.
+ * Also the activity that shows up when the user selects a more-info button in the OS permission settings.


"os permission settings" seems strange. It seems you are discussing the permission of the os.
Instead, I'd rephrase it as

When the user open the settings in Android, and navigate to AnkiDroid permission, there will find a "More info" button which will open this activity.

This way, it's clear that:

This action is related to action in android

android setting contains a subsection specific for ankidroid

this subsection can open android defined activity

Good point, fixed.

Arthur-Milchior · 2025-08-02T21:43:43Z

AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/permissions/PermissionsActivity.kt

+                // if the API version is at least 31. We double check the API version to pass the lint check.
+                if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) {
+                    showAllPermissionsExplanation()
+                }


As extra safety, I'd appreciate if the else branch could send a report to ankidroid. Because even if you are in theory right, Android has given us a lot of unexpected surprise. If you're right, the extra code takes very little space, otherwise it's great to know. The else branch could also open requestPermissionsInPermissionSet so that something occurs for the user

Hm, how do I send a report to AnkiDroid? Just via Timber.e, or is there some other library call I need to use?

Oh, this is why you asked about Timber.e
You need to use CrashReportService.sendExceptionReport

Since I've decided to follow your advice to create a new activity instead, this code is no longer necessary and has been removed.

Arthur-Milchior · 2025-08-02T21:51:54Z

AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/permissions/PermissionsActivity.kt


+        when (intent.action) {
+            Intent.ACTION_VIEW_PERMISSION_USAGE, Intent.ACTION_VIEW_PERMISSION_USAGE_FOR_PERIOD -> {
+                // These actions can trigger this activity due to an intent filter in the manifest


That's not exactly an explanation I fear.
That's technically true, but not really interesting for most reader.

I think instead, it'd be useful to state:

When the user opened android settings, navigated to android permission and tapped "learn more", this activity will be used with one of those two intents. This can only occurs with API 31 or more, so the following `if` should always be true.

This both explain your if and clarify to the reader when this case occur.

Moved this docstring to the top of AllPermissionsExplanationActivity and implemented your suggested change.

Arthur-Milchior · 2025-08-02T21:56:32Z

AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/permissions/PermissionsActivity.kt

        setContentView(R.layout.permissions_activity)
        setTransparentStatusBar()

+        when (intent.action) {


I must admit that it's not clear to me why this activity deal with permission_usage. It seems that it reuse almost no code in common.
Currently it uses the same permissions_activity which has exactly three values, a title, a fragment with the actual content and a button. And even the button, you hide it.
I think it'd be easier to reason about this code if you created a new layout, with the same title. And don't need to sometime hide part of it

I made this PR reuse PermissionsActivity because Brayan told me to in order to reduce redundant code.

Can I have the link please?
I feel like there is almost no code in common, so I don't see how it would create redundancy.
If you want to avoid creating a new layout, I can understand that. But you can have two different activities using the same layout.

Here's the link to Brayan's comment: #18978 (review)

I do find your rationale compelling, though; it's why I initially created a new Activity before Brayan told me to change it. I've brought back the new standalone activity now. It hosts a PermissionsFragment since doing so allows me to reuse some code from the other permission fragments.

Arthur-Milchior · 2025-08-02T22:51:07Z

AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/permissions/PermissionsUntil29Fragment.kt

+        ) { requestedPermissions ->
+            if (!requestedPermissions.all { it.value }) {
+                // The permission dialog did not show up or the user denied the permission
+                // Offer the ability to manually grant the permission via the OS settings


I find strange to put the "settings" at the end while it's an intermediary step. Also, it misses the fact that it's not any part of the os settings, but the ankidroid specific one

So I'd phrase it as

Offers to open the OS settings section for ankidroid. In this section, the user ca manually grant the permission

ericli3690 · 2025-08-03T00:34:18Z

Thanks Arthur for the review. I'll try to break this up into smaller commits. Will also add a more clean immediate explanation of the feature in the description.

Brayan's also let me know that I can delete TiramisuPermissionsFragment, so I'll do that too.

ericli3690 · 2025-08-04T04:03:54Z

@Arthur-Milchior I'm going to wait until #19033 is fully merged before I start making edits here, since you rename some of the variables I'm working with etc. and I don't want to make merging too messy. I'll focus on cleaning up my other draft PRs in the meantime.

Arthur-Milchior · 2025-08-23T23:40:49Z

It's merged

david-allison · 2025-09-06T15:46:41Z

I like it!

ericli3690 · 2025-09-06T19:29:29Z

Updated the following strings (see updated image in PR description):
- Notifications explanation: "Enables background media syncing, allows you to create review reminders"
- Microphone explanation: "Allows you to add sounds to cards, enables voice playback while reviewing cards"
- Title: "AnkiDroid works best with these permissions"
Rebased.

david-allison

Looks solid, cheers!

BrayanDSO

Looks great. Left some comments.

...oid/src/main/java/com/ichi2/anki/ui/windows/permissions/AllPermissionsExplanationFragment.kt

AnkiDroid/src/main/res/values/01-core.xml

GSoC 2025: Review Reminders - Adds permission explanation screen to OS settings to explain to the user why we need the permissions we request, via a new AllPermissionsExplanationFragment fragment hosted in a new PermissionsExplanationActivity. Arthur requested that I create a new activity for this purpose. - Launches the above activity via an intent filter defined in AndroidManifest.xml, as per [the docs](https://developer.android.com/training/permissions/explaining-access#privacy-dashboard). - Added comments to AndroidManifest.xml to flag that future maintainers should add any new runtime permissions to AllPermissionsExplanationFragment. - Moved the logic for requesting external storage from the children of PermissionsFragment to the PermissionsFragment abstract class itself. This allows the methods to be called in both the PermissionsFragment children and the AllPermissionsExplanationFragment, minimizing code duplication. Added logic for setting a PermissionItem to revoke a permission on click, too. - Added strings for telling the user that they need to grant or revoke permissions manually from the OS settings, strings for the permissions explanations, etc. - Increased the amount of maximum lines in a PermissionItem's description so I could fit a longer explanation of why AnkiDroid needs notification permissions. - Added POST_NOTIFICATIONS and RECORD_AUDIO to constants.xml so they can be referred to by AllPermissionsExplanationFragment.

ericli3690 · 2025-09-10T03:59:45Z

Implemented @BrayanDSO's feedback!
Rebased.

github-actions · 2025-09-10T18:21:51Z

Maintainers: Please Sync Translations to produce a commit with only the automated changes from this PR.

Read more about updating strings on the wiki,

ericli3690 added Needs Review GSoC Pull requests authored by a Google Summer of Code participant [Candidate/Selected], for GSoC mentors labels Jul 29, 2025

ericli3690 force-pushed the ericli3690-permissions-explanations branch from 438094f to 95af0d4 Compare July 29, 2025 05:43

ericli3690 force-pushed the ericli3690-permissions-explanations branch from 95af0d4 to 633a0a3 Compare July 29, 2025 06:00

BrayanDSO reviewed Jul 29, 2025

View reviewed changes

ericli3690 removed the Needs Review label Jul 30, 2025

ericli3690 marked this pull request as draft July 30, 2025 03:53

ericli3690 force-pushed the ericli3690-permissions-explanations branch 2 times, most recently from 0b0c9bc to 09f0b86 Compare August 1, 2025 04:25

ericli3690 marked this pull request as ready for review August 1, 2025 04:46

ericli3690 added the Needs Review label Aug 1, 2025

ericli3690 requested a review from BrayanDSO August 1, 2025 04:48

david-allison reviewed Aug 1, 2025

View reviewed changes

ericli3690 force-pushed the ericli3690-permissions-explanations branch from 09f0b86 to 4462657 Compare August 2, 2025 03:18

ericli3690 requested a review from david-allison August 2, 2025 03:22

ericli3690 removed the Needs Review label Aug 2, 2025

ericli3690 marked this pull request as draft August 2, 2025 22:59

Arthur-Milchior reviewed Aug 2, 2025

View reviewed changes

ericli3690 added the Blocked by dependency Currently blocked by some other dependent / related change label Aug 4, 2025

github-actions bot added the Has Conflicts label Aug 23, 2025

ericli3690 added Needs Author Reply Waiting for a reply from the original author and removed Blocked by dependency Currently blocked by some other dependent / related change labels Aug 26, 2025

ericli3690 mentioned this pull request Aug 27, 2025

fix: stop duplicate permissions request screen #19148

Merged

3 tasks

ericli3690 force-pushed the ericli3690-permissions-explanations branch from 4462657 to 6a42c70 Compare August 27, 2025 23:14

ericli3690 marked this pull request as draft September 6, 2025 16:35

ericli3690 removed the Needs Review label Sep 6, 2025

ericli3690 force-pushed the ericli3690-permissions-explanations branch from 00cb4c5 to dcec758 Compare September 6, 2025 19:01

ericli3690 marked this pull request as ready for review September 6, 2025 19:29

ericli3690 added the Needs Review label Sep 6, 2025

ericli3690 requested review from Arthur-Milchior and david-allison September 6, 2025 19:30

ericli3690 added the Strings label Sep 6, 2025

david-allison approved these changes Sep 7, 2025

View reviewed changes

david-allison added Needs Second Approval Has one approval, one more approval to merge and removed Needs Review labels Sep 7, 2025

BrayanDSO reviewed Sep 9, 2025

View reviewed changes

ericli3690 force-pushed the ericli3690-permissions-explanations branch from dcec758 to 2079497 Compare September 10, 2025 03:33

ericli3690 force-pushed the ericli3690-permissions-explanations branch from 2079497 to f25cc1e Compare September 10, 2025 03:34

ericli3690 requested a review from BrayanDSO September 10, 2025 03:59

BrayanDSO approved these changes Sep 10, 2025

View reviewed changes

BrayanDSO added Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) and removed Needs Second Approval Has one approval, one more approval to merge labels Sep 10, 2025

david-allison mentioned this pull request Sep 10, 2025

Updated strings from Crowdin 2025/09/10 14:08 #19209

Merged

david-allison added this pull request to the merge queue Sep 10, 2025

Merged via the queue into ankidroid:main with commit c46cd25 Sep 10, 2025
10 checks passed

github-actions bot added this to the 2.23 release milestone Sep 10, 2025

github-actions bot removed the Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) label Sep 10, 2025

david-allison mentioned this pull request Sep 10, 2025

Updated strings from Crowdin 2025/09/10 20:58 #19211

Merged

Uh oh!

improvement: permission explanations #18978

improvement: permission explanations #18978

Uh oh!

Conversation

ericli3690 commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose / Description

Fixes

Approach

How Has This Been Tested?

Checklist

Uh oh!

ericli3690 commented Jul 29, 2025

Uh oh!

ericli3690 commented Jul 29, 2025

Uh oh!

BrayanDSO left a comment

Choose a reason for hiding this comment

Uh oh!

ericli3690 commented Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Arthur-Milchior left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ericli3690 commented Jul 29, 2025 •

edited

Loading

ericli3690 commented Aug 1, 2025 •

edited

Loading