From d974b61c2615a6797a149c997651d847f04be8f6 Mon Sep 17 00:00:00 2001 From: alperozturk Date: Thu, 5 Dec 2024 09:43:18 +0100 Subject: [PATCH 1/8] fix infinite storage permission check Signed-off-by: alperozturk --- .../ui/activity/SyncedFoldersActivity.kt | 4 +- .../owncloud/android/utils/PermissionUtil.kt | 47 ++++++++++--------- app/src/main/res/values/strings.xml | 2 +- 3 files changed, 28 insertions(+), 25 deletions(-) diff --git a/app/src/main/java/com/owncloud/android/ui/activity/SyncedFoldersActivity.kt b/app/src/main/java/com/owncloud/android/ui/activity/SyncedFoldersActivity.kt index 23486394a2ca..06a7267adf04 100644 --- a/app/src/main/java/com/owncloud/android/ui/activity/SyncedFoldersActivity.kt +++ b/app/src/main/java/com/owncloud/android/ui/activity/SyncedFoldersActivity.kt @@ -198,6 +198,7 @@ class SyncedFoldersActivity : setTheme(R.style.FallbackThemingTheme) } binding.emptyList.emptyListViewAction.setOnClickListener { showHiddenItems() } + PermissionUtil.requestExternalStoragePermission(this, viewThemeUtils, true) } override fun onCreateOptionsMenu(menu: Menu): Boolean { @@ -810,9 +811,6 @@ class SyncedFoldersActivity : if (grantResults.isNotEmpty() && grantResults[0] == PackageManager.PERMISSION_GRANTED) { // permission was granted load(getItemsDisplayedPerFolder(), true) - } else { - // permission denied --> request again - PermissionUtil.requestExternalStoragePermission(this, viewThemeUtils, true) } } else -> super.onRequestPermissionsResult(requestCode, permissions, grantResults) diff --git a/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt b/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt index 1d85050e187f..3f964cf7f219 100644 --- a/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt +++ b/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt @@ -99,6 +99,7 @@ object PermissionUtil { ActivityCompat.checkSelfPermission(context, it) == PackageManager.PERMISSION_GRANTED } + // TODO rename function name to requestExternalStoragePermissionIfNeeded to prevent confusion /** * Request relevant external storage permission depending on SDK, if needed. * @@ -132,10 +133,6 @@ object PermissionUtil { } } - /** - * Request a storage permission - */ - // TODO inject this class to avoid passing ViewThemeUtils around private fun requestStoragePermission( activity: Activity, readOnly: Boolean, @@ -145,10 +142,8 @@ object PermissionUtil { val preferences: AppPreferences = AppPreferencesImpl.fromContext(activity) if (permissionRequired || !preferences.isStoragePermissionRequested) { - // determine required permissions val permissions = if (readOnly && Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { - // use granular media permissions arrayOf( Manifest.permission.READ_MEDIA_IMAGES, Manifest.permission.READ_MEDIA_VIDEO @@ -160,29 +155,39 @@ object PermissionUtil { arrayOf(Manifest.permission.WRITE_EXTERNAL_STORAGE) } - fun doRequest() { - ActivityCompat.requestPermissions( - activity, - permissions, - PERMISSIONS_EXTERNAL_STORAGE - ) - preferences.isStoragePermissionRequested = true + val grantedPermissions = permissions.all { + ContextCompat.checkSelfPermission(activity, it) == PackageManager.PERMISSION_GRANTED } - // Check if we should show an explanation - if (permissions.any { shouldShowRequestPermissionRationale(activity, it) }) { - // Show explanation to the user and then request permission + if (grantedPermissions) { + // Permissions already granted, proceed with functionality + return + } + + val permanentlyDeniedPermissions = permissions.filter { + !ActivityCompat.shouldShowRequestPermissionRationale(activity, it) && + ContextCompat.checkSelfPermission(activity, it) != PackageManager.PERMISSION_GRANTED + } + + if (permissions.any { ActivityCompat.shouldShowRequestPermissionRationale(activity, it) } || + permanentlyDeniedPermissions.isNotEmpty() + ) { Snackbar.make( activity.findViewById(android.R.id.content), R.string.permission_storage_access, - Snackbar.LENGTH_INDEFINITE - ).setAction(R.string.common_ok) { - doRequest() + Snackbar.LENGTH_LONG + ).setAction(R.string.common_settings) { + val intent = Intent(Settings.ACTION_APPLICATION_DETAILS_SETTINGS).apply { + data = Uri.fromParts("package", activity.packageName, null) + } + activity.startActivity(intent) }.also { viewThemeUtils.material.themeSnackbar(it) }.show() } else { - // No explanation needed, request the permission. - doRequest() + ActivityCompat.requestPermissions(activity, permissions, PERMISSIONS_EXTERNAL_STORAGE) } + + // Only mark as requested after actual request + preferences.isStoragePermissionRequested = true } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index e541d34a9c19..bc2504245bf4 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -16,6 +16,7 @@ Details Send Sort + Settings Sort by A - Z Z - A @@ -486,7 +487,6 @@ to create this file to upload to this folder The file is no longer available on the server - Updating data storage folder Preparing migration… Checking destination… From f5bda28ae8c00d67f836e6a36754e45f39dbfa14 Mon Sep 17 00:00:00 2001 From: alperozturk Date: Thu, 5 Dec 2024 09:45:19 +0100 Subject: [PATCH 2/8] todo comment Signed-off-by: alperozturk --- app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt b/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt index 3f964cf7f219..7be2e1b6dc4a 100644 --- a/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt +++ b/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt @@ -99,7 +99,7 @@ object PermissionUtil { ActivityCompat.checkSelfPermission(context, it) == PackageManager.PERMISSION_GRANTED } - // TODO rename function name to requestExternalStoragePermissionIfNeeded to prevent confusion + // TODO: Rename the function to requestExternalStoragePermissionIfNeeded to avoid confusion. /** * Request relevant external storage permission depending on SDK, if needed. * From 79251ddc69e290c754b9365a21abc2580f7ff3b7 Mon Sep 17 00:00:00 2001 From: alperozturk Date: Thu, 5 Dec 2024 09:46:06 +0100 Subject: [PATCH 3/8] fix dco Signed-off-by: alperozturk --- app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt b/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt index 7be2e1b6dc4a..bbe05107e578 100644 --- a/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt +++ b/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt @@ -99,6 +99,7 @@ object PermissionUtil { ActivityCompat.checkSelfPermission(context, it) == PackageManager.PERMISSION_GRANTED } + // TODO: Rename the function to requestExternalStoragePermissionIfNeeded to avoid confusion. /** * Request relevant external storage permission depending on SDK, if needed. From 61f8ba835978488c6b55a92399f82e9ed5c8a213 Mon Sep 17 00:00:00 2001 From: alperozturk Date: Thu, 5 Dec 2024 09:46:49 +0100 Subject: [PATCH 4/8] fix dco Signed-off-by: alperozturk --- app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt b/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt index bbe05107e578..7be2e1b6dc4a 100644 --- a/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt +++ b/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt @@ -99,7 +99,6 @@ object PermissionUtil { ActivityCompat.checkSelfPermission(context, it) == PackageManager.PERMISSION_GRANTED } - // TODO: Rename the function to requestExternalStoragePermissionIfNeeded to avoid confusion. /** * Request relevant external storage permission depending on SDK, if needed. From b8d1c5f170ebdb15ab4e7254e6c51129a0f17258 Mon Sep 17 00:00:00 2001 From: alperozturk Date: Thu, 5 Dec 2024 09:54:41 +0100 Subject: [PATCH 5/8] change comment Signed-off-by: alperozturk --- app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt b/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt index 7be2e1b6dc4a..e18222f12a9b 100644 --- a/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt +++ b/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt @@ -160,7 +160,7 @@ object PermissionUtil { } if (grantedPermissions) { - // Permissions already granted, proceed with functionality + // Permissions already granted return } From 134d49a7426b2c807b2f6e01efeb720da0d3289f Mon Sep 17 00:00:00 2001 From: alperozturk Date: Thu, 5 Dec 2024 09:59:51 +0100 Subject: [PATCH 6/8] change comment Signed-off-by: alperozturk --- app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt b/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt index e18222f12a9b..930bd192d30c 100644 --- a/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt +++ b/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt @@ -99,7 +99,7 @@ object PermissionUtil { ActivityCompat.checkSelfPermission(context, it) == PackageManager.PERMISSION_GRANTED } - // TODO: Rename the function to requestExternalStoragePermissionIfNeeded to avoid confusion. + // TODO Rename the function to requestExternalStoragePermissionIfNeeded to avoid confusion. /** * Request relevant external storage permission depending on SDK, if needed. * From 9604c992f18ddb62f6c5cd390dc110e548123c8e Mon Sep 17 00:00:00 2001 From: alperozturk Date: Thu, 5 Dec 2024 11:57:34 +0100 Subject: [PATCH 7/8] revert logic changes Signed-off-by: alperozturk --- .../owncloud/android/utils/PermissionUtil.kt | 59 ++++++++++--------- app/src/main/res/values/strings.xml | 1 - 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt b/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt index 930bd192d30c..71c2217d5fde 100644 --- a/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt +++ b/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt @@ -99,7 +99,6 @@ object PermissionUtil { ActivityCompat.checkSelfPermission(context, it) == PackageManager.PERMISSION_GRANTED } - // TODO Rename the function to requestExternalStoragePermissionIfNeeded to avoid confusion. /** * Request relevant external storage permission depending on SDK, if needed. * @@ -133,6 +132,10 @@ object PermissionUtil { } } + /** + * Request a storage permission + */ + // TODO inject this class to avoid passing ViewThemeUtils around private fun requestStoragePermission( activity: Activity, readOnly: Boolean, @@ -142,8 +145,10 @@ object PermissionUtil { val preferences: AppPreferences = AppPreferencesImpl.fromContext(activity) if (permissionRequired || !preferences.isStoragePermissionRequested) { + // determine required permissions val permissions = if (readOnly && Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { + // use granular media permissions arrayOf( Manifest.permission.READ_MEDIA_IMAGES, Manifest.permission.READ_MEDIA_VIDEO @@ -155,39 +160,29 @@ object PermissionUtil { arrayOf(Manifest.permission.WRITE_EXTERNAL_STORAGE) } - val grantedPermissions = permissions.all { - ContextCompat.checkSelfPermission(activity, it) == PackageManager.PERMISSION_GRANTED - } - - if (grantedPermissions) { - // Permissions already granted - return - } - - val permanentlyDeniedPermissions = permissions.filter { - !ActivityCompat.shouldShowRequestPermissionRationale(activity, it) && - ContextCompat.checkSelfPermission(activity, it) != PackageManager.PERMISSION_GRANTED + fun doRequest() { + ActivityCompat.requestPermissions( + activity, + permissions, + PERMISSIONS_EXTERNAL_STORAGE + ) + preferences.isStoragePermissionRequested = true } - if (permissions.any { ActivityCompat.shouldShowRequestPermissionRationale(activity, it) } || - permanentlyDeniedPermissions.isNotEmpty() - ) { + // Check if we should show an explanation + if (permissions.any { shouldShowRequestPermissionRationale(activity, it) }) { + // Show explanation to the user and then request permission Snackbar.make( activity.findViewById(android.R.id.content), R.string.permission_storage_access, - Snackbar.LENGTH_LONG - ).setAction(R.string.common_settings) { - val intent = Intent(Settings.ACTION_APPLICATION_DETAILS_SETTINGS).apply { - data = Uri.fromParts("package", activity.packageName, null) - } - activity.startActivity(intent) + Snackbar.LENGTH_INDEFINITE + ).setAction(R.string.common_ok) { + doRequest() }.also { viewThemeUtils.material.themeSnackbar(it) }.show() } else { - ActivityCompat.requestPermissions(activity, permissions, PERMISSIONS_EXTERNAL_STORAGE) + // No explanation needed, request the permission. + doRequest() } - - // Only mark as requested after actual request - preferences.isStoragePermissionRequested = true } } @@ -256,10 +251,16 @@ object PermissionUtil { activity, listener ) - } - val dialogFragment = StoragePermissionDialogFragment.newInstance(permissionRequired) - dialogFragment.show(activity.supportFragmentManager, PERMISSION_CHOICE_DIALOG_TAG) + // Check if the dialog is already added to the FragmentManager. + val existingDialog = activity.supportFragmentManager.findFragmentByTag(PERMISSION_CHOICE_DIALOG_TAG) + + // Only show the dialog if it's not already shown. + if (existingDialog == null) { + val dialogFragment = StoragePermissionDialogFragment.newInstance(permissionRequired) + dialogFragment.show(activity.supportFragmentManager, PERMISSION_CHOICE_DIALOG_TAG) + } + } } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index bc2504245bf4..62d70524205a 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -16,7 +16,6 @@ Details Send Sort - Settings Sort by A - Z Z - A From a9109b44d2e275dfba3197757472b70f2254cf37 Mon Sep 17 00:00:00 2001 From: alperozturk Date: Thu, 5 Dec 2024 11:58:19 +0100 Subject: [PATCH 8/8] revert changes Signed-off-by: alperozturk --- app/src/main/res/values/strings.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 62d70524205a..e541d34a9c19 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -486,6 +486,7 @@ to create this file to upload to this folder The file is no longer available on the server + Updating data storage folder Preparing migration… Checking destination…