Anki-android: Enhancement: AnkiDroid should not use Storage permission

Created on 11 May 2019  Â·  20Comments  Â·  Source: ankidroid/Anki-Android

Reproduction Steps
  1. Start AnkiDroid for the first time
  2. Click "Deny" on the "Allow AnkiDroid to access photos, media and files on your device?" dialog
Expected Result

AnkiDroid should default to a directory that doesn't require WRITE_EXTERNAL_STORAGE. For example, /data/data/com.ichi2.anki/files/ (Context.getFilesDir()) or /sdcard/Android/data/com.ichi2.anki/ (Context.getExternalFilesDirs(null)).

Actual Result

AnkiDroid defaults to /sdcard/AnkiDroid and exits with "AnkiDroid directory is inaccessible".

Research
  • [x] I have read the support page and am reporting a bug or enhancement request specific to AnkiDroid

  • [x] I have checked the manual and the FAQ and could not find a solution to my issue

  • [x] I have searched for similar existing issues here and on the user forum

Enhancement Keep Open Priority-High

Most helpful comment

I had some recent experience with this where if you use the directories provided as getExternalStorage I think you can actually drop the permission. Problem being that we don't use that directory currently. Either way this is a popular request, I'll leave it open

All 20 comments

I don't disagree. Please feel free to propose a PR, it must be backwards compatible with existing installs

@mikehardy Is this done?Or Can I work on this?

This is not done - and no one is working on it that I am aware of. I'm unsure of the requirements for acceptance - I mentioned "backwards-compatible with existing installs" but I have not thought through what that means. @timrae was thinking about this before and likely has ideas - Tim do you have any specific thoughts?

And @MonikaJethani I will say that since this affects the app very deeply - it is where people have their beloved collections stored :-) - probably best to post your detailed implementation idea here before starting work, or if you use code to flesh out the idea propose a PR but prepared for the whole approach to be questioned at the idea level until we're sure it will work.

But hopefully that doesn't sound too discouraging, it would be great for AnkiDroid to use the system directory access APIs and not require special permission, and a PR that did that would be great in my opinion

@mikehardy I would implement it thusly:

  • Add a new preference named along the lines of "Use internal storage". On existing installations, it should be disabled, but on new installations it should default to enabled.

  • If it is enabled, the "AnkiDroid directory" preference should be disabled and its value overridden with Context.getFilesDir().

  • If it is disabled, then use the value of "AnkiDroid directory" and prompt for Storage permissions if necessary.

Using the new preference should have the same semantics as changing the "AnkiDroid directory" preference which, as far as I know, just initializes the new directory and doesn't touch the old directory.

In the long run, the "AnkiDroid directory" preference will become obsolete because apps targeting Android Q will not be able to access external storage (they will be given a sandboxed view and must use specific APIs which is a huge privacy improvement).

Solid start - interesting, I try to read the preview API changes but haven't had time for Q as of yet.

I have read it now though, and I have a slightly different take on Q's changes though - 2 big things:

  • With regard to internal vs external - https://developer.android.com/reference/android/content/Context.html#getExternalFilesDir(java.lang.String) will be a viable route to external storage. This is a big deal. You're discussing "internal storage", but note the enhancement is to drop the "Storage permission". They are separate things. I submit we should target dropping the permission and becoming Q-sandbox compliant as a criteria for any PR here, but that's still separate from internal vs external. I do not want to enforce or set a default for internal storage as the collections many people use are large. So external storage should be default, but to meet the drop-permission and Q-sandbox criteria, using Context#getExternalFilesDir() seems best. I support the idea of internal storage as an option (again through the Q-sandbox compliant Context API) but I would not default to internal.

  • With regard to access on existing /sdcard/AnkiDroid - I quote from your link:

An app that has a sandboxed view always has access to the files that it creates, both 
inside and outside its app-specific directory. 

as well as:

File management and media creation apps typically manage groups of files in a 
directory hierarchy. These apps can invoke the ACTION_OPEN_DOCUMENT_TREE intent to 
allow the user to grant access to an entire directory tree. Such an app would be able 
to edit any file in the selected directory, as well as any of its sub-directories.

Using this interface, users can access files from any installed instance of 
DocumentsProvider, which any locally-backed or cloud-based solution can support.

This makes me think that we could continue to access /sdcard/AnkiDroid even after Q, because we know the locations. We'd have to test whether we can just maintain access since we created the files originally or whether we need to obtain permission in Q-sandbox mode With an intent and a conversion to a DocumentsProvider but it should be possible? Not ideal, but "external, legacy mode" does not appear to be forbidden even in the scoped-storage Q-sandbox, just irritating.

So I would propose a slightly different idea -

  • have a 3-state drop-down for where to store things
    -- "external, legacy mode" visible in and default only for existing installs that still have data in /sdcard/AnkiDroid. if selected, there is a preference for setting the directory to use, defaults to /sdcard/AnkiDroid. For new installs does not exist
    -- "external" as default for new installs, option for others, if selected, uses Context#getExernalFilesDir()
    -- "internal" as option for others, uses Context#getFilesDir()

This would get us to a place where we were able to work in either of the three areas - a big step and functional / usable at this point. But it would just initialize the directory, as you say.

  • Then - and most important for the future I think - we have code that performs migrations between the locations when the user changes storage location. A dialog indicating what will happen and that it may take a while for large collections, then an AsyncTask doing the work with a modal dialog showing progress (all very very similar to the database check code). Implementation should be just closing the database, calling filesystem moves on the directory, and restarting the activity with an intent that has extra info indicating what we did so we can show a success or fail dialog to the user.

This part is vital or if I understand the scoped-storage Q-sandbox we may have to implement DocumentsProvider access and the DOCUMENT_TREE intent thing just to keep accessing /sdcard/AnkiDroid or we will orphan the data of a 1.3 million active user installed base (!)

Finally, I note this warning on the Q link, which is I think talking about the year 2020 R platform:

Scoped storage will be required in next year's major platform release for all apps, 
independent of target SDK level. Therefore, you should ensure that your app works 
with scoped storage well in advance. To do so, make sure that the behavior is enabled 
on Android Q devices running your app.

So that gives us around a year to get this done and have it rolled out long enough to migrate people, which leads to the final step:

  • Finally on version upgrades in the startup show dialogs code flow, we start pushing people with a popup explaining the need to migrate, the deadline, and offering the choice to go the migrate preference that does it, so we can move people from external legacy to external and move towards sandboxed Q compliance

What do you think? Not especially different for the first step I think, but a little twist there (prefer external), but the 2nd (migration) and 3rd (popup to push migration) are big differences. ?

Add a new preference named along the lines of "Use internal storage". On existing installations, it should be disabled, but on new installations it should default to enabled.

We used to have a setting for this, which nobody used. We can't make it the default location as many people have collections on the order of hundreds of megabytes. This is not an approach that I would be willing to take.

Ideally I'd like to avoid add a preference for this. We should just migrate to the new APIs as needed. Most likely that means increasing the minimum SDK, so we'll need to discuss the best way to handle that.

From an implementation perspective, I think this issue is the same as #3106. It's not a small job, but if you have the time and patience to see it through to the end then I'd be very happy to start a design discussion.

Ideally I'd like to avoid add a preference for this. We should just migrate to the new APIs as needed. Most likely that means increasing the minimum SDK, so we'll need to discuss the best way to handle that.

No preference - fine by me. I can imagine that being very simplifying

As for the API mention, the only necessary API I think is https://developer.android.com/reference/android/content/Context#getExternalFilesDir(java.lang.String) which is API8

If we had to DocumentsProvider for some reason (which we would not, assuming we moved inside of Context#getExternalFilesDir, if I understand correctly), that's API19 / KitKat / 4.4 and represents about 11,000 active users out of about 1,127,000 according to the play store just now. Easily a 2.10 bump and justifiable for 2.9 this moment if it opened things up for this work to proceed cleanly, IMHO

We used to have a setting for this, which nobody used. We can't make it the default location as many people have collections on the order of hundreds of megabytes. This is not an approach that I would be willing to take.

@timrae All devices that AnkiDroid supports should use the same underlying storage for internal storage and external storage. If users are using a microSD card, that is not the same as the path returned by Context#getExternalStorageDirectory. Therefore, there's little to no point in supporting both Context#getFilesDir and Context#getExternalStorageDirectory

As far as I know, that's incorrect. Most devices have a separate partition on the internal SD card with root only permissions, and this is where the applications and application data (Context#getFilesDir) gets stored. This partition is much smaller than the non-root partition, so you can't store large files there.

I was thinking the same thing but wanted to back it up with docs or a test and didn't have time - I do believe they have differences though, just wanted proof

@timrae Most devices that ship with Android 4 and above (which is the minimum API level supported by AnkiDroid) use a single partition, and a FUSE filesystem to emulate the SD card.

The only devices that support AnkiDroid and use a separate partition will be devices that shipped with an old version of Android and are running a custom ROM or a few low-end exceptions.

Hmm, I'm away from my computer ATM, but I found someone's df output which
seems to agree with what you're saying

https://android.stackexchange.com/a/119072

It seems like a dangerous and unnecessary assumption to make though, since
it's entirely up to the manufacturer what limitations they place on the
size of the /data virtual partition.

On Sat., 25 May 2019, 5:42 pm Saleem Rashid, notifications@github.com
wrote:

@timrae https://github.com/timrae Most devices that ship with Android 4
and above (which is the minimum API level supported by AnkiDroid) use a
single partition, and a FUSE filesystem to emulate the SD card.

The only devices that support AnkiDroid and use a separate partition will
be devices running a custom ROM or a few low-end exceptions.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/ankidroid/Anki-Android/issues/5304?email_source=notifications&email_token=AAVQBYX334LKL2MACWYXEITPXD3XXA5CNFSM4HMI2STKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWHKOAA#issuecomment-495888128,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAVQBYU4NONQ5LHUBOIUV5LPXD3XXANCNFSM4HMI2STA
.

On Sat., 25 May 2019, 5:42 pm Saleem Rashid, notifications@github.com
wrote:

@timrae https://github.com/timrae Most devices that ship with Android 4
and above (which is the minimum API level supported by AnkiDroid) use a
single partition, and a FUSE filesystem to emulate the SD card.

The only devices that support AnkiDroid and use a separate partition will
be devices running a custom ROM or a few low-end exceptions.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/ankidroid/Anki-Android/issues/5304?email_source=notifications&email_token=AAVQBYX334LKL2MACWYXEITPXD3XXA5CNFSM4HMI2STKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWHKOAA#issuecomment-495888128,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAVQBYU4NONQ5LHUBOIUV5LPXD3XXANCNFSM4HMI2STA
.

I'm not so familiar with storage and permission management on Android and so I can't say for sure whether I got it all correctly, but from my understanding my issue (which I was hesitant to open a new thread for) is related to the above.

I have an Android 6.0.1 and the latest Ankidroid app installed. I have an SD card to which I moved the Anki app via the application manager, since it has more storage capacity than my phone's internal storage.
However––– all the media (and backups) are still on the internal storage. The application manager shows that AnkiDroid uses 22.88MB of (external) storage, including "364KB of data". My naive understanding therefore is that Android doesn't treat the collection.media as part of the AnkiDroid app, but simply as any other files which AnkiDroid simply happens to manipulate.
By itself it would not be a problem, it would even be an advantage, if only it was possible (as the end user) to dictate AnkiDroid the path to the media directory, but I'm not aware that this is possible. As it is now, I have Anki's (indeed a few hundreds of MBs of) media dir stored in an immovable manner, which is a problem given the precious real estate value of the internal phone's storage.

Hi sorry I accidentally pressed the close button. Your assessment that Ankidroid does not treat the flashcard and media databases as part of the app is correct. We do not officially support using external SD cards. However there is a workaround available to get this working if you wish to s do it anyway. Please read the FAQ for more info on this.

Hello 👋, this issue has been opened for more than 2 months with no activity on it. If the issue is still here, please keep in mind that we need community support and help to fix it! Just comment something like _still searching for solutions_ and if you found one, please open a pull request! You have 7 days until this gets closed automatically

I had some recent experience with this where if you use the directories provided as getExternalStorage I think you can actually drop the permission. Problem being that we don't use that directory currently. Either way this is a popular request, I'll leave it open

Failure to open the app on API 29 without enabling legacy storage in the manifest.

This is high priority, as we have a hard deadline of roughly a year before we need to move to API 30.

2020-10-04 11:59:49.419 7380-7380/com.ichi2.anki I/DeckPicker: User has permissions to access collection
2020-10-04 11:59:49.420 7380-7380/com.ichi2.anki E/CollectionHelper: Could not initialize AnkiDroid directory
    com.ichi2.anki.exception.StorageAccessException: Failed to create AnkiDroid directory /storage/emulated/0/AnkiDroid
        at com.ichi2.anki.CollectionHelper.initializeAnkiDroidDirectory(CollectionHelper.java:197)
        at com.ichi2.anki.CollectionHelper.getCol(CollectionHelper.java:121)
        at com.ichi2.anki.CollectionHelper.getCol(CollectionHelper.java:111)
        at com.ichi2.anki.CollectionHelper.getColSafe(CollectionHelper.java:152)
        at com.ichi2.anki.DeckPicker.firstCollectionOpen(DeckPicker.java:578)
        at com.ichi2.anki.DeckPicker.onCreate(DeckPicker.java:435)
        at android.app.Activity.performCreate(Activity.java:7802)
        at android.app.Activity.performCreate(Activity.java:7791)
        at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1299)
        at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3245)
        at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3409)
        at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:83)
        at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135)
        at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2016)
        at android.os.Handler.dispatchMessage(Handler.java:107)
        at android.os.Looper.loop(Looper.java:214)
        at android.app.ActivityThread.main(ActivityThread.java:7356)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:930)

Yep - no argument it's high priority as it's a big change and we need design thinking first which is something we are not well practiced in (though we execute designs quite well...)

Just to note that we do have legacy storage enabled now though so we should have bought ourselves that year of time without crashing I believe?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

mikehardy picture mikehardy  Â·  5Comments

mashinbaz1 picture mashinbaz1  Â·  6Comments

david-allison-1 picture david-allison-1  Â·  5Comments

mikehardy picture mikehardy  Â·  4Comments

Anthropos888 picture Anthropos888  Â·  5Comments