From 494297afe3a14d9aeef6b54515b7dceff5e1cc5f Mon Sep 17 00:00:00 2001 From: Christian Lohmaier Date: Thu, 28 Sep 2017 19:56:45 +0200 Subject: pass context as parameter instead of risk of leaking memory Also adjust to dynamic permissions after bumping target-SDK. There still is some confusion about the concept of "external storage" in the code. LocalDocuments already is "external storage" - clean that up a little and use AppCompat function instead of using a legacy class for ExternalDocuments provider. Doesn't help for broken ROMs though, that would need guessing pathname for a mounted SD (in addition to separate storage partition of builtin storage). Also c6e8c96d50fc2082a3c4b9553196a42bbdd6df37 incorrectly changed the conditional around, making the whole ExternalDocumentsProvider useless/a copy of the Local one (i.e. the primary, first returned by the system). Real fix for tdf#99539 likely was 66be4feef7e0d3661f01fbb2372700de5eeea070 Change-Id: I88ca7742c0f2e89d63c338c8852ad88be0a46e4b Reviewed-on: https://gerrit.libreoffice.org/45572 Tested-by: Jenkins Reviewed-by: Christian Lohmaier --- android/source/res/menu/navigation_menu.xml | 2 +- android/source/res/values/strings.xml | 2 +- .../org/libreoffice/LibreOfficeMainActivity.java | 2 +- .../storage/DocumentProviderFactory.java | 12 +-- .../org/libreoffice/storage/IDocumentProvider.java | 12 ++- .../src/java/org/libreoffice/storage/IFile.java | 5 +- .../storage/external/BrowserSelectorActivity.java | 2 +- .../libreoffice/storage/external/ExternalFile.java | 4 +- .../storage/external/ExtsdDocumentsProvider.java | 79 +++++++++------- .../external/IExternalDocumentProvider.java | 5 +- .../external/LegacyExtSDDocumentsProvider.java | 103 --------------------- .../storage/external/OTGDocumentsProvider.java | 20 ++-- .../local/LocalDocumentsDirectoryProvider.java | 40 +++++++- .../storage/local/LocalDocumentsProvider.java | 7 +- .../org/libreoffice/storage/local/LocalFile.java | 4 +- .../libreoffice/storage/owncloud/OwnCloudFile.java | 6 +- .../storage/owncloud/OwnCloudProvider.java | 8 +- .../org/libreoffice/ui/LibreOfficeUIActivity.java | 51 +++++----- 18 files changed, 158 insertions(+), 206 deletions(-) delete mode 100644 android/source/src/java/org/libreoffice/storage/external/LegacyExtSDDocumentsProvider.java (limited to 'android/source') diff --git a/android/source/res/menu/navigation_menu.xml b/android/source/res/menu/navigation_menu.xml index 532c9ed60639..db680a9a1b8c 100644 --- a/android/source/res/menu/navigation_menu.xml +++ b/android/source/res/menu/navigation_menu.xml @@ -6,7 +6,7 @@ + android:icon="@drawable/ic_folder_black_24dp" /> Document locations Close document locations - Local documents + Documents directory Local file system External SD OTG device (experimental) diff --git a/android/source/src/java/org/libreoffice/LibreOfficeMainActivity.java b/android/source/src/java/org/libreoffice/LibreOfficeMainActivity.java index 534eaf44de59..48722dcd9bfd 100644 --- a/android/source/src/java/org/libreoffice/LibreOfficeMainActivity.java +++ b/android/source/src/java/org/libreoffice/LibreOfficeMainActivity.java @@ -323,7 +323,7 @@ public class LibreOfficeMainActivity extends AppCompatActivity implements Settin try { // rebuild the IFile object from the data passed in the Intent IFile mStorageFile = DocumentProviderFactory.getInstance() - .getProvider(providerId).createFromUri(documentUri); + .getProvider(providerId).createFromUri(LibreOfficeMainActivity.this, documentUri); // call document provider save operation mStorageFile.saveDocument(mInputFile); } diff --git a/android/source/src/java/org/libreoffice/storage/DocumentProviderFactory.java b/android/source/src/java/org/libreoffice/storage/DocumentProviderFactory.java index f73a2b0d543a..acf5aebcd6c6 100644 --- a/android/source/src/java/org/libreoffice/storage/DocumentProviderFactory.java +++ b/android/source/src/java/org/libreoffice/storage/DocumentProviderFactory.java @@ -13,7 +13,6 @@ import java.util.HashSet; import java.util.Set; import org.libreoffice.storage.external.ExtsdDocumentsProvider; -import org.libreoffice.storage.external.LegacyExtSDDocumentsProvider; import org.libreoffice.storage.external.OTGDocumentsProvider; import org.libreoffice.storage.local.LocalDocumentsDirectoryProvider; import org.libreoffice.storage.local.LocalDocumentsProvider; @@ -21,7 +20,6 @@ import org.libreoffice.storage.owncloud.OwnCloudProvider; import android.content.Context; import android.content.SharedPreferences.OnSharedPreferenceChangeListener; -import android.os.Build; /** * Keeps the instances of the available IDocumentProviders in the system. @@ -68,13 +66,7 @@ public final class DocumentProviderFactory { instance.providers[OTG_PROVIDER_INDEX] = new OTGDocumentsProvider(OTG_PROVIDER_INDEX, context); instance.providers[4] = new OwnCloudProvider(4, context); - if(Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) { - instance.providers[EXTSD_PROVIDER_INDEX] - = new ExtsdDocumentsProvider(EXTSD_PROVIDER_INDEX, context); - } else { - instance.providers[EXTSD_PROVIDER_INDEX] - = new LegacyExtSDDocumentsProvider(EXTSD_PROVIDER_INDEX, context); - } + instance.providers[EXTSD_PROVIDER_INDEX] = new ExtsdDocumentsProvider(EXTSD_PROVIDER_INDEX, context); // initialize document provider names list instance.providerNames = new String[instance.providers.length]; @@ -121,7 +113,7 @@ public final class DocumentProviderFactory { * @return default provider. */ public IDocumentProvider getDefaultProvider() { - return providers[1]; + return providers[0]; } public Set getChangeListeners() { diff --git a/android/source/src/java/org/libreoffice/storage/IDocumentProvider.java b/android/source/src/java/org/libreoffice/storage/IDocumentProvider.java index 2b0460a8ee6e..044d7ddb422b 100644 --- a/android/source/src/java/org/libreoffice/storage/IDocumentProvider.java +++ b/android/source/src/java/org/libreoffice/storage/IDocumentProvider.java @@ -9,6 +9,8 @@ package org.libreoffice.storage; +import android.content.Context; + import java.net.URI; /** @@ -22,19 +24,22 @@ public interface IDocumentProvider { * * @return Content root element. * @throws RuntimeException in case of error. + * @param context */ - IFile getRootDirectory(); + IFile getRootDirectory(Context context); /** * Transforms some URI into the IFile object that represents that content. * + * + * @param context * @param uri * URI pointing to some content object that has been previously * retrieved with IFile.getUri(). * @return IFile object pointing to the content represented by uri. * @throws RuntimeException in case of error. */ - IFile createFromUri(URI uri); + IFile createFromUri(Context context, URI uri); /** * Get internationalized name for this provider. This name is intended to be @@ -59,6 +64,7 @@ public interface IDocumentProvider { * Checks if the Document Provider is available or not. * * @return A boolean value based on provider availability. + * @param context */ - boolean checkProviderAvailability(); + boolean checkProviderAvailability(Context context); } diff --git a/android/source/src/java/org/libreoffice/storage/IFile.java b/android/source/src/java/org/libreoffice/storage/IFile.java index 3655ba484ede..c9cfa7f1198d 100644 --- a/android/source/src/java/org/libreoffice/storage/IFile.java +++ b/android/source/src/java/org/libreoffice/storage/IFile.java @@ -9,6 +9,8 @@ package org.libreoffice.storage; +import android.content.Context; + import java.io.File; import java.io.FileFilter; import java.net.URI; @@ -91,8 +93,9 @@ public interface IFile { * Returns the pparent of this file. * * @return this file's parent or null if it does not have it. + * @param context */ - IFile getParent(); + IFile getParent(Context context); /** * Returns the document wrapped by this IFile as a local file. The result diff --git a/android/source/src/java/org/libreoffice/storage/external/BrowserSelectorActivity.java b/android/source/src/java/org/libreoffice/storage/external/BrowserSelectorActivity.java index fe12804e620c..90a15ae95af4 100644 --- a/android/source/src/java/org/libreoffice/storage/external/BrowserSelectorActivity.java +++ b/android/source/src/java/org/libreoffice/storage/external/BrowserSelectorActivity.java @@ -64,7 +64,7 @@ public class BrowserSelectorActivity extends AppCompatActivity { IExternalDocumentProvider provider = (IExternalDocumentProvider) DocumentProviderFactory.getInstance() .getProvider(providerIndex); - String previousDirectoryPath = preferences.getString(preferenceKey, provider.guessRootURI()); + String previousDirectoryPath = preferences.getString(preferenceKey, provider.guessRootURI(this)); Intent i = new Intent(this, DirectoryBrowserActivity.class); i.putExtra(DirectoryBrowserActivity.DIRECTORY_PATH_EXTRA, previousDirectoryPath); startActivityForResult(i, REQUEST_INTERNAL_BROWSER); diff --git a/android/source/src/java/org/libreoffice/storage/external/ExternalFile.java b/android/source/src/java/org/libreoffice/storage/external/ExternalFile.java index 7c7f09fc1ade..aff33e4413ef 100644 --- a/android/source/src/java/org/libreoffice/storage/external/ExternalFile.java +++ b/android/source/src/java/org/libreoffice/storage/external/ExternalFile.java @@ -102,11 +102,11 @@ public class ExternalFile implements IFile{ } @Override - public IFile getParent() { + public IFile getParent(Context context) { // this is the root node if(docFile.getParentFile() == null) return null; - return new ExternalFile(provider, docFile.getParentFile(), context); + return new ExternalFile(provider, docFile.getParentFile(), this.context); } @Override diff --git a/android/source/src/java/org/libreoffice/storage/external/ExtsdDocumentsProvider.java b/android/source/src/java/org/libreoffice/storage/external/ExtsdDocumentsProvider.java index 4ce77f0e1916..72599ec0d17b 100644 --- a/android/source/src/java/org/libreoffice/storage/external/ExtsdDocumentsProvider.java +++ b/android/source/src/java/org/libreoffice/storage/external/ExtsdDocumentsProvider.java @@ -1,6 +1,5 @@ package org.libreoffice.storage.external; -import android.annotation.TargetApi; import android.content.Context; import android.content.SharedPreferences; import android.content.SharedPreferences.OnSharedPreferenceChangeListener; @@ -8,7 +7,9 @@ import android.net.Uri; import android.os.Build; import android.os.Environment; import android.preference.PreferenceManager; +import android.support.v4.content.ContextCompat; import android.support.v4.provider.DocumentFile; +import android.util.Log; import org.libreoffice.R; import org.libreoffice.storage.DocumentProviderSettingsActivity; @@ -34,45 +35,49 @@ public class ExtsdDocumentsProvider implements IExternalDocumentProvider, private int id; private File cacheDir; - private Context context; private String rootPathURI; public ExtsdDocumentsProvider(int id, Context context) { this.id = id; - this.context = context; - setupRootPathUri(); - setupCache(); + setupRootPathUri(context); + setupCache(context); } - private void setupRootPathUri() { + private void setupRootPathUri(Context context) { SharedPreferences preferences = PreferenceManager.getDefaultSharedPreferences(context); - String rootURIGuess = guessRootURI(); rootPathURI = preferences.getString( - DocumentProviderSettingsActivity.KEY_PREF_EXTERNAL_SD_PATH_URI, rootURIGuess); + DocumentProviderSettingsActivity.KEY_PREF_EXTERNAL_SD_PATH_URI, guessRootURI(context)); } - //Android 4.4 specific - @TargetApi(Build.VERSION_CODES.KITKAT) - public String guessRootURI() { - File[] options = context.getExternalFilesDirs(null); - File internalSD = Environment.getExternalStorageDirectory(); - String internalSDPath = internalSD.getAbsolutePath(); - - for (File option: options) { + public String guessRootURI(Context context) { + // TODO: unfortunately the getExternalFilesDirs function relies on devices to actually + // follow guidelines re external storage. Of course device manufacturers don't and as such + // you cannot rely on it returning the actual paths (neither the compat, nor the native variant) + File[] possibleRemovables = ContextCompat.getExternalFilesDirs(context,null); + // the primary dir that is already covered by the "LocalDocumentsProvider" + // might be emulated/part of internal memory or actual SD card + // TODO: change to not confuse android's "external storage" with "expandable storage" + String primaryExternal = Environment.getExternalStorageDirectory().getAbsolutePath(); + + for (File option: possibleRemovables) { // Returned paths may be null if a storage device is unavailable. - if (null == option) { continue; } - + if (null == option) { + Log.w(LOGTAG,"path was a null option :-/"); continue; } String optionPath = option.getAbsolutePath(); + if(optionPath.contains(primaryExternal)) { + Log.v(LOGTAG, "did get file path - but is same as primary storage ("+ primaryExternal +")"); + continue; + } - if(optionPath.contains(internalSDPath)) - return option.toURI().toString(); - + return option.toURI().toString(); } - return ""; + // TODO: do some manual probing of possible directories (/storage/sdcard1 and similar) + Log.i(LOGTAG, "no secondary storage reported"); + return null; } - private void setupCache() { + private void setupCache(Context context) { // TODO: probably we should do smarter cache management cacheDir = new File(context.getExternalCacheDir(), "externalFiles"); if (cacheDir.exists()) { @@ -94,41 +99,42 @@ public class ExtsdDocumentsProvider implements IExternalDocumentProvider, } @Override - public IFile getRootDirectory() { + public IFile getRootDirectory(Context context) { if(android.os.Build.VERSION.SDK_INT <= Build.VERSION_CODES.KITKAT) { - return android4RootDirectory(); + return android4RootDirectory(context); } else { - return android5RootDirectory(); + return android5RootDirectory(context); } } - private ExternalFile android4RootDirectory() { + private ExternalFile android4RootDirectory(Context context) { try{ File f = new File(new URI(rootPathURI)); return new ExternalFile(this, DocumentFile.fromFile(f), context); } catch (Exception e) { //invalid rootPathURI - throw buildRuntimeExceptionForInvalidFileURI(); + throw buildRuntimeExceptionForInvalidFileURI(context); } } - private ExternalFile android5RootDirectory() { + private ExternalFile android5RootDirectory(Context context) { try { return new ExternalFile(this, DocumentFile.fromTreeUri(context, Uri.parse(rootPathURI)), context); } catch (Exception e) { //invalid rootPathURI - throw buildRuntimeExceptionForInvalidFileURI(); + throw buildRuntimeExceptionForInvalidFileURI(context); } } - private RuntimeException buildRuntimeExceptionForInvalidFileURI() { + private RuntimeException buildRuntimeExceptionForInvalidFileURI(Context context) { + // ToDo: discarding the original excpeption / catch-all handling is bad style return new RuntimeException(context.getString(R.string.ext_document_provider_error)); } @Override - public IFile createFromUri(URI javaURI) { + public IFile createFromUri(Context context, URI javaURI) { //TODO: refactor when new DocumentFile API exist //uri must be of a DocumentFile file, not directory. Uri androidUri = Uri.parse(javaURI.toString()); @@ -148,8 +154,13 @@ public class ExtsdDocumentsProvider implements IExternalDocumentProvider, } @Override - public boolean checkProviderAvailability() { - return Environment.getExternalStorageState().equals(Environment.MEDIA_MOUNTED) && Environment.isExternalStorageRemovable(); + public boolean checkProviderAvailability(Context context) { + // too many devices (or I am just unlucky) don't report the mounted state properly, and other + // devices also consider dedicated part of internal storage to be "mounted" so cannot use + // getExternalStorageState().equals(Environment.MEDIA_MOUNTED) && isExternalStorageRemovable() + // but they refer to the primary external storage anyway, so what currently is covered by the + // "LocalDocumentsProvider" + return rootPathURI!=null; } @Override diff --git a/android/source/src/java/org/libreoffice/storage/external/IExternalDocumentProvider.java b/android/source/src/java/org/libreoffice/storage/external/IExternalDocumentProvider.java index 34bbcbc4bad6..a439417b60cd 100644 --- a/android/source/src/java/org/libreoffice/storage/external/IExternalDocumentProvider.java +++ b/android/source/src/java/org/libreoffice/storage/external/IExternalDocumentProvider.java @@ -1,5 +1,7 @@ package org.libreoffice.storage.external; +import android.content.Context; + import org.libreoffice.storage.IDocumentProvider; @@ -13,7 +15,8 @@ public interface IExternalDocumentProvider extends IDocumentProvider { * browsing using the internal DirectoryBrowser. * * @return a guess of the root file's URI. + * @param context */ - String guessRootURI(); + String guessRootURI(Context context); } diff --git a/android/source/src/java/org/libreoffice/storage/external/LegacyExtSDDocumentsProvider.java b/android/source/src/java/org/libreoffice/storage/external/LegacyExtSDDocumentsProvider.java deleted file mode 100644 index 1ac34405b361..000000000000 --- a/android/source/src/java/org/libreoffice/storage/external/LegacyExtSDDocumentsProvider.java +++ /dev/null @@ -1,103 +0,0 @@ -package org.libreoffice.storage.external; - -import android.content.Context; -import android.content.SharedPreferences; -import android.os.Environment; -import android.preference.PreferenceManager; -import android.text.TextUtils; -import android.util.Log; - -import org.libreoffice.R; -import org.libreoffice.storage.DocumentProviderSettingsActivity; -import org.libreoffice.storage.IFile; -import org.libreoffice.storage.IOUtils; -import org.libreoffice.storage.local.LocalFile; - -import java.io.File; -import java.net.URI; - -/** - * Legacy document provider for External SD cards, for Android 4.3 and below. - * - * Uses the LocalFile class. - */ -public class LegacyExtSDDocumentsProvider implements IExternalDocumentProvider, - SharedPreferences.OnSharedPreferenceChangeListener{ - private static final String LOGTAG = LegacyExtSDDocumentsProvider.class.getSimpleName(); - - private int id; - private Context context; - private String rootPathURI; - - public LegacyExtSDDocumentsProvider(int id, Context context) { - this.id = id; - this.context = context; - setupRootPathUri(); - } - - private void setupRootPathUri() { - SharedPreferences preferences = PreferenceManager.getDefaultSharedPreferences(context); - String rootURIGuess = guessRootURI(); - - rootPathURI = preferences.getString( - DocumentProviderSettingsActivity.KEY_PREF_EXTERNAL_SD_PATH_URI, rootURIGuess); - } - - public String guessRootURI() { - //hacky method of obtaining extsdcard root - final String value = System.getenv("SECONDARY_STORAGE"); - Log.d(LOGTAG, "guesses: " + value); - if (!TextUtils.isEmpty(value)) { - final String[] paths = value.split(":"); - for (String path : paths) { - File file = new File(path); - if(path.contains("ext") && file.isDirectory()) { - return file.toURI().toString(); - } - } - } - return ""; - } - - @Override - public IFile getRootDirectory() { - if(rootPathURI.equals("")) { - throw new RuntimeException(context.getString(R.string.ext_document_provider_error)); - } - - File f = IOUtils.getFileFromURIString(rootPathURI); - if(IOUtils.isInvalidFile(f)) { - //missing device - throw new RuntimeException(context.getString(R.string.legacy_extsd_missing_error)); - } - return new LocalFile(f); - } - - @Override - public IFile createFromUri(URI uri) { - return new LocalFile(uri); - } - - @Override - public int getNameResource() { - return R.string.external_sd_file_system; - } - - @Override - public int getId() { - return id; - } - - @Override - public boolean checkProviderAvailability() { - return Environment.getExternalStorageState().equals(Environment.MEDIA_MOUNTED) && Environment.isExternalStorageRemovable(); - } - - @Override - public void onSharedPreferenceChanged(SharedPreferences preferences, String key) { - if (key.equals(DocumentProviderSettingsActivity.KEY_PREF_EXTERNAL_SD_PATH_URI)) { - rootPathURI = preferences.getString(key, ""); - } - } - -} diff --git a/android/source/src/java/org/libreoffice/storage/external/OTGDocumentsProvider.java b/android/source/src/java/org/libreoffice/storage/external/OTGDocumentsProvider.java index d4ce3492c1ea..138ec9479755 100644 --- a/android/source/src/java/org/libreoffice/storage/external/OTGDocumentsProvider.java +++ b/android/source/src/java/org/libreoffice/storage/external/OTGDocumentsProvider.java @@ -4,6 +4,7 @@ import android.content.Context; import android.content.SharedPreferences; import android.content.pm.PackageManager; import android.preference.PreferenceManager; +import android.util.Log; import org.libreoffice.R; import org.libreoffice.storage.DocumentProviderSettingsActivity; @@ -22,24 +23,22 @@ public class OTGDocumentsProvider implements IExternalDocumentProvider, private static final String LOGTAG = OTGDocumentsProvider.class.getSimpleName(); - private Context context; private String rootPathURI; private int id; public OTGDocumentsProvider(int id, Context context) { - this.context = context; this.id = id; - setupRootPath(); + setupRootPath(context); } - private void setupRootPath() { + private void setupRootPath(Context context) { SharedPreferences preferences = PreferenceManager.getDefaultSharedPreferences(context); rootPathURI = preferences.getString( DocumentProviderSettingsActivity.KEY_PREF_OTG_PATH_URI, ""); } @Override - public IFile createFromUri(URI uri) { + public IFile createFromUri(Context context, URI uri) { return new LocalFile(uri); } @@ -54,15 +53,16 @@ public class OTGDocumentsProvider implements IExternalDocumentProvider, } @Override - public IFile getRootDirectory() { - + public IFile getRootDirectory(Context context) { + // TODO: handle this with more fine-grained exceptions if(rootPathURI.equals("")) { + Log.e(LOGTAG, "rootPathURI is empty"); throw new RuntimeException(context.getString(R.string.ext_document_provider_error)); } File f = IOUtils.getFileFromURIString(rootPathURI); if(IOUtils.isInvalidFile(f)) { - //missing device + Log.e(LOGTAG, "rootPathURI is invalid - missing device?"); throw new RuntimeException(context.getString(R.string.otg_missing_error)); } @@ -78,12 +78,12 @@ public class OTGDocumentsProvider implements IExternalDocumentProvider, } @Override - public String guessRootURI() { + public String guessRootURI(Context context) { return ""; } @Override - public boolean checkProviderAvailability() { + public boolean checkProviderAvailability(Context context) { // check if system supports USB Host return context.getPackageManager().hasSystemFeature(PackageManager.FEATURE_USB_HOST); } diff --git a/android/source/src/java/org/libreoffice/storage/local/LocalDocumentsDirectoryProvider.java b/android/source/src/java/org/libreoffice/storage/local/LocalDocumentsDirectoryProvider.java index fb4236969cb2..02d58d329122 100644 --- a/android/source/src/java/org/libreoffice/storage/local/LocalDocumentsDirectoryProvider.java +++ b/android/source/src/java/org/libreoffice/storage/local/LocalDocumentsDirectoryProvider.java @@ -14,7 +14,13 @@ import java.io.File; import org.libreoffice.storage.IFile; import org.libreoffice.R; +import android.Manifest; +import android.content.Context; +import android.content.pm.PackageManager; +import android.os.Build; import android.os.Environment; +import android.support.v4.content.ContextCompat; +import android.util.Log; /** * A convenience IDocumentProvider to browse the /sdcard/Documents directory. @@ -29,11 +35,31 @@ public class LocalDocumentsDirectoryProvider extends LocalDocumentsProvider { super(id); } + private static File getDocumentsDir() { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) { + // DIRECTORY_DOCUMENTS is 19 or later only + return Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOCUMENTS); + } else { + return new File(Environment.getExternalStorageDirectory() + "/Documents"); + } + } + @Override - public IFile getRootDirectory() { - File documentsDirectory = new File( - Environment.getExternalStorageDirectory(), "Documents"); - documentsDirectory.mkdirs(); + public IFile getRootDirectory(Context context) { + File documentsDirectory = getDocumentsDir(); + if (!documentsDirectory.exists()) { + // might be a little counter-intuitive: if we were granted READ permission already, we're also granted the write-permission + // when we ask for it, since they are both in the same storage group (for 5.1 and lower it is granted at install-time already) + // seehttps://developer.android.com/guide/topics/permissions/requesting.html#perm-groups + if (ContextCompat.checkSelfPermission(context, Manifest.permission.WRITE_EXTERNAL_STORAGE) == PackageManager.PERMISSION_GRANTED) { + if(!documentsDirectory.mkdirs()) { + // fallback to the toplevel dir - might be due to the dir not mounted/used as USB-Mass-Storage or similar + // TODO: handle unavailability of the storage/failure of the mkdir properly + Log.e("LocalDocumentsProvider", "not sure how we ended up here - if we have read permissions to use it in the first place, we also should have the write-permissions.."); + documentsDirectory = Environment.getExternalStorageDirectory(); + } + } + } return new LocalFile(documentsDirectory); } @@ -41,4 +67,10 @@ public class LocalDocumentsDirectoryProvider extends LocalDocumentsProvider { public int getNameResource() { return R.string.local_documents; } + + @Override + public boolean checkProviderAvailability(Context context) { + File documentsDirectory = getDocumentsDir(); + return documentsDirectory.exists() || ContextCompat.checkSelfPermission(context, Manifest.permission.WRITE_EXTERNAL_STORAGE) == PackageManager.PERMISSION_GRANTED; + } } diff --git a/android/source/src/java/org/libreoffice/storage/local/LocalDocumentsProvider.java b/android/source/src/java/org/libreoffice/storage/local/LocalDocumentsProvider.java index 42c253322ce7..39c91dc951e0 100644 --- a/android/source/src/java/org/libreoffice/storage/local/LocalDocumentsProvider.java +++ b/android/source/src/java/org/libreoffice/storage/local/LocalDocumentsProvider.java @@ -16,6 +16,7 @@ import org.libreoffice.storage.IFile; import org.libreoffice.R; +import android.content.Context; import android.os.Environment; /** @@ -30,12 +31,12 @@ public class LocalDocumentsProvider implements IDocumentProvider { } @Override - public IFile getRootDirectory() { + public IFile getRootDirectory(Context context) { return new LocalFile(Environment.getExternalStorageDirectory()); } @Override - public IFile createFromUri(URI uri) { + public IFile createFromUri(Context context, URI uri) { return new LocalFile(uri); } @@ -50,7 +51,7 @@ public class LocalDocumentsProvider implements IDocumentProvider { } @Override - public boolean checkProviderAvailability() { + public boolean checkProviderAvailability(Context context) { return true; } } diff --git a/android/source/src/java/org/libreoffice/storage/local/LocalFile.java b/android/source/src/java/org/libreoffice/storage/local/LocalFile.java index 2d5554b419ee..4ff5bbf119f4 100644 --- a/android/source/src/java/org/libreoffice/storage/local/LocalFile.java +++ b/android/source/src/java/org/libreoffice/storage/local/LocalFile.java @@ -9,6 +9,8 @@ package org.libreoffice.storage.local; +import android.content.Context; + import java.io.File; import java.io.FileFilter; import java.net.URI; @@ -75,7 +77,7 @@ public class LocalFile implements IFile { } @Override - public IFile getParent() { + public IFile getParent(Context context) { return new LocalFile(file.getParentFile()); } diff --git a/android/source/src/java/org/libreoffice/storage/owncloud/OwnCloudFile.java b/android/source/src/java/org/libreoffice/storage/owncloud/OwnCloudFile.java index c218961a2266..fa74a54b08e2 100644 --- a/android/source/src/java/org/libreoffice/storage/owncloud/OwnCloudFile.java +++ b/android/source/src/java/org/libreoffice/storage/owncloud/OwnCloudFile.java @@ -1,5 +1,7 @@ package org.libreoffice.storage.owncloud; +import android.content.Context; + import java.io.File; import java.io.FileFilter; import java.io.UnsupportedEncodingException; @@ -123,12 +125,12 @@ public class OwnCloudFile implements IFile { } @Override - public IFile getParent() { + public IFile getParent(Context context) { if (parentPath == null) // this is the root node return null; - return provider.createFromUri(URI.create(parentPath)); + return provider.createFromUri(context, URI.create(parentPath)); } @Override diff --git a/android/source/src/java/org/libreoffice/storage/owncloud/OwnCloudProvider.java b/android/source/src/java/org/libreoffice/storage/owncloud/OwnCloudProvider.java index e10c88bee93a..335a34aeb361 100644 --- a/android/source/src/java/org/libreoffice/storage/owncloud/OwnCloudProvider.java +++ b/android/source/src/java/org/libreoffice/storage/owncloud/OwnCloudProvider.java @@ -72,12 +72,12 @@ public class OwnCloudProvider implements IDocumentProvider, } @Override - public IFile getRootDirectory() { - return createFromUri(URI.create(FileUtils.PATH_SEPARATOR)); + public IFile getRootDirectory(Context context) { + return createFromUri(context, URI.create(FileUtils.PATH_SEPARATOR)); } @Override - public IFile createFromUri(URI uri) { + public IFile createFromUri(Context context, URI uri) { ReadRemoteFileOperation refreshOperation = new ReadRemoteFileOperation( uri.getPath()); RemoteOperationResult result = refreshOperation.execute(client); @@ -179,7 +179,7 @@ public class OwnCloudProvider implements IDocumentProvider, } @Override - public boolean checkProviderAvailability() { + public boolean checkProviderAvailability(Context context) { return true; } } diff --git a/android/source/src/java/org/libreoffice/ui/LibreOfficeUIActivity.java b/android/source/src/java/org/libreoffice/ui/LibreOfficeUIActivity.java index 5b9dd0cc2ee0..f8d08aae171f 100644 --- a/android/source/src/java/org/libreoffice/ui/LibreOfficeUIActivity.java +++ b/android/source/src/java/org/libreoffice/ui/LibreOfficeUIActivity.java @@ -9,6 +9,7 @@ package org.libreoffice.ui; +import android.Manifest; import android.app.Activity; import android.app.AlertDialog; import android.content.BroadcastReceiver; @@ -18,6 +19,7 @@ import android.content.DialogInterface; import android.content.Intent; import android.content.IntentFilter; import android.content.SharedPreferences; +import android.content.pm.PackageManager; import android.content.pm.ShortcutInfo; import android.content.pm.ShortcutManager; import android.graphics.drawable.Icon; @@ -30,6 +32,7 @@ import android.preference.PreferenceManager; import android.support.annotation.NonNull; import android.support.design.widget.FloatingActionButton; import android.support.design.widget.NavigationView; +import android.support.v4.app.ActivityCompat; import android.support.v4.content.ContextCompat; import android.support.v4.view.ViewCompat; import android.support.v4.widget.DrawerLayout; @@ -91,6 +94,8 @@ public class LibreOfficeUIActivity extends AppCompatActivity implements Settings private int viewMode; private int sortMode; private boolean showHiddenFiles; + // dynamic permissions IDs + private static int PERMISSION_READ_EXTERNAL_STORAGE = 0; FileFilter fileFilter; FilenameFilter filenameFilter; @@ -160,9 +165,20 @@ public class LibreOfficeUIActivity extends AppCompatActivity implements Settings filter.addAction(UsbManager.ACTION_USB_DEVICE_DETACHED); registerReceiver(mUSBReceiver, filter); // init UI and populate with contents from the provider + if (ContextCompat.checkSelfPermission(this, Manifest.permission.READ_EXTERNAL_STORAGE) != PackageManager.PERMISSION_GRANTED) { + // TODO: remove local document providers if really is denied, code right now assumes it is granted/ + // there is no onRequestPermissionsResult evaluating the callback + // without the read permissions, LO could only load documents passed via intent from other apps + Log.i(LOGTAG, "no permission to read external storage - asking for permission"); + ActivityCompat.requestPermissions(this, + new String[]{Manifest.permission.READ_EXTERNAL_STORAGE}, + PERMISSION_READ_EXTERNAL_STORAGE); + } + switchToDocumentProvider(documentProviderFactory.getDefaultProvider()); createUI(); - getAnimations(); + fabOpenAnimation = AnimationUtils.loadAnimation(this, R.anim.fab_open); + fabCloseAnimation = AnimationUtils.loadAnimation(this, R.anim.fab_close); } public void createUI() { @@ -199,7 +215,7 @@ public class LibreOfficeUIActivity extends AppCompatActivity implements Settings final ArrayList recentFiles = new ArrayList(); for (String recentFileString : recentFileStrings) { try { - recentFiles.add(documentProvider.createFromUri(new URI(recentFileString))); + recentFiles.add(documentProvider.createFromUri(this, new URI(recentFileString))); } catch (URISyntaxException e) { e.printStackTrace(); } catch (RuntimeException e){ @@ -227,20 +243,16 @@ public class LibreOfficeUIActivity extends AppCompatActivity implements Settings // Loop through the document providers menu items and check if they are available or not for (int index = 0; index < providerNames.size(); index++) { MenuItem item = navigationDrawer.getMenu().getItem(index); - boolean isDocumentProviderAvailable = checkDocumentProviderAvailability(documentProviderFactory.getProvider(index)); - if (!isDocumentProviderAvailable){ - item.setEnabled(false); - } + item.setEnabled(documentProviderFactory.getProvider(index).checkProviderAvailability(this)); } - final Context context = this; //needed for anonymous method below navigationDrawer.setNavigationItemSelectedListener(new NavigationView.OnNavigationItemSelectedListener() { @Override public boolean onNavigationItemSelected(@NonNull MenuItem item) { switch (item.getItemId()) { case R.id.menu_storage_preferences: { - startActivity(new Intent(context, DocumentProviderSettingsActivity.class)); + startActivity(new Intent(LibreOfficeUIActivity.this, DocumentProviderSettingsActivity.class)); return true; } @@ -300,11 +312,6 @@ public class LibreOfficeUIActivity extends AppCompatActivity implements Settings drawerToggle.syncState(); } - private void getAnimations() { - fabOpenAnimation = AnimationUtils.loadAnimation(this, R.anim.fab_open); - fabCloseAnimation = AnimationUtils.loadAnimation(this, R.anim.fab_close); - } - private void expandFabMenu() { ViewCompat.animate(editFAB).rotation(45.0F).withLayer().setDuration(300).setInterpolator(new OvershootInterpolator(10.0F)).start(); drawLayout.startAnimation(fabOpenAnimation); @@ -331,10 +338,6 @@ public class LibreOfficeUIActivity extends AppCompatActivity implements Settings isFabMenuOpen = false; } - private boolean checkDocumentProviderAvailability(IDocumentProvider provider) { - return provider.checkProviderAvailability(); - } - @Override protected void onPostCreate(Bundle savedInstanceState) { super.onPostCreate(savedInstanceState); @@ -429,7 +432,7 @@ public class LibreOfficeUIActivity extends AppCompatActivity implements Settings // a different thread try { documentProvider = provider[0]; - homeDirectory = documentProvider.getRootDirectory(); + homeDirectory = documentProvider.getRootDirectory(LibreOfficeUIActivity.this); currentDirectory = homeDirectory; filePaths = currentDirectory.listFiles(FileUtilities .getFileFilter(filterMode)); @@ -444,7 +447,7 @@ public class LibreOfficeUIActivity extends AppCompatActivity implements Settings } }); startActivity(new Intent(activity, DocumentProviderSettingsActivity.class)); - Log.e(LOGTAG, e.getMessage(), e.getCause()); + Log.e(LOGTAG, "failed to switch document provider "+ e.getMessage(), e.getCause()); } return null; } @@ -613,7 +616,7 @@ public class LibreOfficeUIActivity extends AppCompatActivity implements Settings protected IFile doInBackground(Void... dir) { // this operation may imply network access and must be run in // a different thread - return currentDirectory.getParent(); + return currentDirectory.getParent(LibreOfficeUIActivity.this); } @Override @@ -830,10 +833,10 @@ public class LibreOfficeUIActivity extends AppCompatActivity implements Settings Intent i = this.getIntent(); if (i.hasExtra(CURRENT_DIRECTORY_KEY)) { try { - currentDirectory = documentProvider.createFromUri(new URI( + currentDirectory = documentProvider.createFromUri(this, new URI( i.getStringExtra(CURRENT_DIRECTORY_KEY))); } catch (URISyntaxException e) { - currentDirectory = documentProvider.getRootDirectory(); + currentDirectory = documentProvider.getRootDirectory(this); } Log.d(LOGTAG, CURRENT_DIRECTORY_KEY); } @@ -883,10 +886,10 @@ public class LibreOfficeUIActivity extends AppCompatActivity implements Settings .getProvider(savedInstanceState.getInt(DOC_PROVIDER_KEY)); } try { - currentDirectory = documentProvider.createFromUri(new URI( + currentDirectory = documentProvider.createFromUri(this, new URI( savedInstanceState.getString(CURRENT_DIRECTORY_KEY))); } catch (URISyntaxException e) { - currentDirectory = documentProvider.getRootDirectory(); + currentDirectory = documentProvider.getRootDirectory(this); } filterMode = savedInstanceState.getInt(FILTER_MODE_KEY, FileUtilities.ALL); viewMode = savedInstanceState.getInt(EXPLORER_VIEW_TYPE_KEY, GRID_VIEW); -- cgit