ARK-Builders / ARK-Navigator

Android app for navigation through your data
MIT License
15 stars 15 forks source link

App can crash if permissions were revoked #234

Closed kirillt closed 1 year ago

kirillt commented 2 years ago

Information from @CloudLevi (see https://github.com/ARK-Builders/ARK-Navigator/pull/161)

Currently started reworking the permission request. There is a bug here, but in a very rare scenario. Steps to reproduce:

  1. On a fresh app, open the app
  2. Grant storage permissions
  3. Add a root
  4. Without closing the app completely, go to app settings on the device, and revoke the storage permissions
  5. Go back into the app (the app should ask for storage permissions again)
  6. Deny the permission (saved root folder should still be present)
  7. Click on those roots and click the navigate button, the app may crash or behave unexpectedly

Otherwise, I tried blocking all interactions until we have storage permissions. How do you think we should handle that event, and similar ones? My suggestions are:

  1. Clear the fragments, so the user will end up with a blank screen and nothing to interact with
  2. Have some kind of a custom Router, that will check storage permissions when trying to make Fragment transactions
  3. (I think it's a bad one, as it can cause memory leaks, but will toss it here) store "hasStoragePermissions" in MainActivity or other place as a variable, and check it from the Presenter and other places thoughout the app
melvin4u445 commented 2 years ago

This is reproducible even with basic steps noted in #198 like:

  1. Install the app and open it
  2. Deny access to files
  3. If the Plus button isn't visible in "Manage folders" section, then open "Navigate data" tab and switch back to "Manage folders" to see the plus button
  4. Tap the Plus button in "Manage folders" section
  5. A folder picker modal pops up. Try to open any folder in the modal

https://user-images.githubusercontent.com/17034768/167716065-1c6e7605-c7b1-43ca-b6ef-32d5b56bc201.mp4

melvin4u445 commented 2 years ago
  1. Clear the fragments, so the user will end up with a blank screen and nothing to interact with

Nothing to interact with sounds good. Can we show a better error message along with it? https://github.com/ARK-Builders/ARK-Navigator/issues/199

kirillt commented 2 years ago

@melvin4u445 quoting you from closed issue, still good action point for whoever will work on it:

Happens when access request is denied after the app is installed for the first time. We could show a better error message, something like "Please provide read and write access to use the app" or we could use all the white space to create a better error message screen explaining why we need access to their files

kirillt commented 2 years ago

[off-topic] @mdrlzy is any of this or this useful?