Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix scoped storage permission issues causing a crash when opening a typst project folder that was not just created by the app (+ manual refresh for big documents + auto refresh onResume for cloud syncing + background preview refresh without freezing UI) #151

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

lrq3000
Copy link

@lrq3000 lrq3000 commented May 20, 2024

Before this fix, creating a folder worked fine, but accessing an already existing folder crashed. The only exception is when we create a folder, and then try to reopen it just after. But once we erase the app's cache, or reinstall it, or create another folder, then trying to open the previously created folder will crash, just like for any other folder.

This is because of scoped storage limitations introduced in Android 10+. An app can freely create a folder, and access all files inside (files are then considered as "media" files, and the folder a media database of resources), but this is only temporary: the access to this "media" folder will be lost once the app requests the access to another folder or is reinstalled or its cache is erased. This is exactly what happened here.

The solution is to go through the hoops of the scoped storage new system to ask specifically to access an already existing folder and then ask for a permission to write in it. The functions all changed and it's very complicated to get it right.

Fortunately, there is the SimpleStorage library. I implented it here only to open already existing Typst folders, the biggest issue currently IMHO.

The same fix should be applied to creating a Typst folder (although it works currently, the folder is created with the improper method, as a "media" collection instead of being just a raw folder of files - in practice it doesn't change much in this case I think but to be more future-proof it would be better to also use SimpleStorage for creation too) ; and also the fix should be applied to opening files. I did not try to open markdown files because I am using another editor for this purpose, but I guess the same issue will arise.

This PR may also be unclean, I did it very roughly and quickly as I am currently lacking time, it's more a proof of concept than a polished implementation, so please feel free to cherry pick and edit as you see fit.

Note that it includes additional edits than the scoped storage fix (all the changes in TypstProjectViewModel.kt were made prior to try to make rustService more robust against crashes or more explicit with log messages, you can keep or remove as you prefer).

Fixes #149

For those who want to try the debug apk: https://github.com/lrq3000/BeauTyXT/releases/tag/scoped-storage-fix4-and-background-refresh

/EDIT: Since I aim to use this app to write my whole PhD thesis, I am going to use it extensively on a daily basis. I added a few other features:

  1. another feature that I think is necessary, the ability to disable the automatic preview refresh, and added a manual refresh button in the top toolbar. This allows to work on documents that are bigger than toy examples, otherwise they are way too slow after each typed character since there is a refresh, whereas here we can refresh only when we want.
    • Note: Also the manual refresh doubles as a way to manually force a refresh after syncing the files (eg with syncthing), whereas before it was necessary to type in the current document to force the refresh (note that it does NOT refresh the edited text, but only the preview and list of files, to refresh the edited text, it's still necessary to reopen the current file, this is done because otherwise it would be surprising for the user that the refresh button may change their whole inputs!).
  2. onResume() is now implemented to auto refresh the content in the text editor when the app regains focus. Essentially this means that when the app in the background or the screen is turned off, and there is a background cloud files synchronization service that updates the files (such as Syncthing or Dropbox), then when the app regains focus, the latest version of the file will be displayed. This avoids overwriting newer changes with an older version by mistake (eases multidevices synchronization).
    • Note: a better implementation would be an explicit detection of changes in the background (difference between current value in the app versus the file's content) and ask the user what to do, but the implementation I propose is much easier and works for 80% of cases.
  3. renderProjectToSvgs(), which manages the preview refresh for Typst projects, is now launched in a background process and with a mutex lock to avoid multiple calls in parallel which can crash the app (eg, auto refresh on typing, with a user who types fast!). This avoids non toy Typst projects from freezing the whole app during preview refreshes, so now the user can type and even open other files in the project while the preview is smoothly updated in the background. This kind of removes the need for the manual refresh, but I keep it because it can still be useful for those who want to save on battery or for when the files are edited from outside the app.

TODO:

  • Test all features on a real phone.
  • Test on Android 10 (API 29) (real phone, ARM64)
  • Test on Android 14 (API 34) (emulated) -- could not test because emulator requires compiling the libraries for x86_64

lrq3000 added 7 commits May 20, 2024 03:35
…age permission issues (fixed via anggrayudi's SimpleStorage lib) (fixes soupslurpr#149)

Signed-off-by: Stephen L. <[email protected]>
Signed-off-by: Stephen L. <[email protected]>
…was still happening after a while because of silently deleted permission) + add comments to help to reproduce by other devs

Signed-off-by: Stephen L. <[email protected]>
… button (allow support for bigger documents)

Signed-off-by: Stephen L. <[email protected]>
…ocus)

This feature helps a lot with background cloud synchronization services, such as Syncthing or Dropbox. Essentially, the app will auto reload the latest version of the text when not in focus (but if the app is still in focus, the user must manually reload the current file to accept the new changes).

Signed-off-by: Stephen L. <[email protected]>
@lrq3000 lrq3000 changed the title Fix scoped storage permission issues causing a crash when opening a typst project folder that was not just created by the app Fix scoped storage permission issues causing a crash when opening a typst project folder that was not just created by the app (+ manual refresh for big documents + auto refresh onResume for cloud syncing) May 20, 2024
lrq3000 added 3 commits May 22, 2024 14:07
…bles auto refresh on project loading

This is very necessary for any non toy project, need to prioritize fast loading and input (this is a mobile note taking app after all), rendering can always be done after manually.

Signed-off-by: Stephen L. <[email protected]>
…nd thread to avoid UI freezes with big documents

Signed-off-by: Stephen L. <[email protected]>
… refresh does not crash when repetitively called (eg, auto refresh on typing)

Signed-off-by: Stephen L. <[email protected]>
@lrq3000 lrq3000 changed the title Fix scoped storage permission issues causing a crash when opening a typst project folder that was not just created by the app (+ manual refresh for big documents + auto refresh onResume for cloud syncing) Fix scoped storage permission issues causing a crash when opening a typst project folder that was not just created by the app (+ manual refresh for big documents + auto refresh onResume for cloud syncing + background preview refresh without freezing UI) May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

App crashes upon opening a Typst project folder
1 participant