feat: Allow disabling kDrive display in files#2015
feat: Allow disabling kDrive display in files#2015aymericmariaux wants to merge 4 commits intomainfrom
Conversation
aymericmariaux
force-pushed
the
disable-kdrive-in-file-app
branch
from
fb2ddb0 to
1cb1e52
Compare
aymericmariaux
force-pushed
the
disable-kdrive-in-file-app
branch
2 times, most recently
from
0480d50 to
1cb1e52
Compare
aymericmariaux
requested review from
NicolasBourdin88 and
Copilot
and removed request for
NicolasBourdin88 and
Copilot
There was a problem hiding this comment.
Pull request overview
Adds a new Security settings screen that lets users toggle whether kDrive’s DocumentsProvider is exposed to the system “Files” app (and other third‑party file pickers), and wires that screen into the existing Settings navigation.
Changes:
- Added “Show in Files” strings (with multiple localizations) and a new Security settings screen UI.
- Added a new
SecuritySettingsFragmentand navigation route fromSettingsFragment. - Implemented a persisted “provider disabled” flag in
CloudStorageProviderand enforced it inqueryChildDocuments()/openDocument().
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/res/values/strings.xml | Adds new strings for the “Show in Files” toggle and provider-disabled error. |
| app/src/main/res/values-*/strings.xml | Adds translations for the new provider toggle/error strings (sv/pt/pl/nl/nb/it/fr/fi/es/el/de/da). |
| app/src/main/res/navigation/main_navigation.xml | Adds navigation action from Settings to the new Security settings fragment; registers the fragment destination. |
| app/src/main/res/layout/fragment_settings_security.xml | New Security settings screen layout containing app lock and provider toggle. |
| app/src/main/res/layout/fragment_settings.xml | Replaces the previous “App Security” row with a chevron row leading to Security settings. |
| app/src/main/java/com/infomaniak/drive/ui/menu/settings/SettingsFragment.kt | Updates Settings click handling to navigate to the new Security settings screen. |
| app/src/main/java/com/infomaniak/drive/ui/menu/settings/SecuritySettingsFragment.kt | New fragment controlling app-lock entry and “Show in Files” provider toggle. |
| app/src/main/java/com/infomaniak/drive/ui/menu/settings/AppSecuritySettingsActivity.kt | Forces provider disable when enabling app lock. |
| app/src/main/java/com/infomaniak/drive/data/documentprovider/CloudStorageProvider.kt | Adds shared-pref flag + enforcement for disabled provider behavior and root refresh notifications. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
aymericmariaux
force-pushed
the
disable-kdrive-in-file-app
branch
from
1dc00e4 to
473369a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Prevents contentProviderSwitch from re-triggering its listener in onResume, avoiding redundant calls to CloudStorageProvider.setDisabled() and notifyRootsChanged().
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| fun setDisabled(context: Context, disabled: Boolean) { | ||
| context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE).edit { putBoolean(KEY_PROVIDER_DISABLED, disabled) } |
There was a problem hiding this comment.
SharedPreferences.edit { ... } uses apply() by default (async). Calling notifyRootsChanged(context) immediately after can race with persistence, so the provider may be re-queried before the new flag is stored. Make the write synchronous (e.g., commit=true / commit()) or otherwise ensure the preference is persisted before notifying roots changed.
| context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE).edit { putBoolean(KEY_PROVIDER_DISABLED, disabled) } | |
| context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE).edit(commit = true) { | |
| putBoolean(KEY_PROVIDER_DISABLED, disabled) | |
| } |
| override fun openDocument(documentId: String, mode: String, signal: CancellationSignal?): ParcelFileDescriptor? { | ||
| SentryLog.d(TAG, "openDocument(), id=$documentId, mode=$mode, signalIsCancelled: ${signal?.isCanceled}") | ||
| if (isProviderDisabled()) { | ||
| throw SecurityException(context?.getString(R.string.fileProviderExtensionError)) | ||
| } | ||
| val context = context ?: return null |
There was a problem hiding this comment.
When the provider is disabled by a user preference, throwing SecurityException can lead to harsher client behavior than necessary (some clients treat it as an authorization failure). Consider throwing FileNotFoundException (or an appropriate DocumentsContract-aligned exception) with the same message to better reflect 'content unavailable' rather than 'permission denied'.
| contentProviderSwitch.isChecked = !CloudStorageProvider.isDisabled(appCtx) | ||
|
|
||
| contentProviderSwitch.setOnCheckedChangeListener { _, isChecked -> | ||
| CloudStorageProvider.setDisabled(appCtx, disabled = !isChecked) | ||
| } |
There was a problem hiding this comment.
In onResume(), assigning contentProviderSwitch.isChecked may trigger the existing setOnCheckedChangeListener, causing redundant preference writes and repeated notifyRootsChanged() calls. A common fix is to temporarily clear/reinstall the listener around the assignment, or only assign when the value actually changes.
|
No description provided.