fix(macos): Improve VFS trash behavior and add deletion warning #9241
Conversation
This commit addresses issues with the macOS Virtual Files (FileProvider)
trash handling to prevent accidental permanent data loss:
1. Filter server-trashed items from materialised enumeration
- Prevents items that are already in the server's trash from appearing
in the macOS Trash, avoiding confusion and potential data conflicts
- Server trash items should only be managed through the Nextcloud web
interface or desktop client
2. Add confirmation dialog when enabling permanent deletion
- Shows a clear warning dialog explaining the consequences before
enabling the "permanently delete files" option
- Requires explicit user confirmation (Yes/No) with "No" as default
- Resets UI state if user cancels
3. Improve UI clarity for trash deletion setting
- Renamed checkbox to "Permanently delete files when removed from
virtual drive" for clarity
- Added visible warning label (red, italic) when the option is enabled
- Added Connections handler to properly sync UI state with controller
Signed-off-by: Florian Staske <florian@Air-von-Florian.staske.lan>
Flo-129
requested review from
Aiiaiiio,
camilasan,
i2h3,
mgallien and
nilsding
as code owners
i2h3
left a comment
i2h3
left a comment
There was a problem hiding this comment.
Thank you for your pull request!
I am afraid there is a conceptual misunderstanding on what the existing feature is expected to do and how the trash works with file providers.
Current Implementation
The currently available switch to enable deletion of items in the trash controls whether items can actually be deleted (see FileProviderExtension.swift:455).
In summary: If trash synchronization is supported and enabled and the item already located in the trash is supposed to finally be deleted for real, then the deletion can be rejected with this toggle.
If trash synchronization is not supported or enabled or the item to delete not inside the trash, then deletion will take place.
I have to admit: We do not know why this feature is there in the first place and it was scheduled to be removed soon with the next major release. Just like its related user interface already has been removed in the development branch.
File Providers and Trash
File provider domains can advertise their support for synchronizing trash content or their lack thereof (see NSFileProviderDomain. supportsSyncingTrash). Depending o
n the value, macOS behaves differently.
If a file provider domain does not support trash content synchronization, then macOS will not move items into the trash but ask the user for confirmation of immediate and permanent deletion. We are informed about that through the deleteitem(identifier:baseversion:options:request:completionhandler:) call.
If a file provider domain does support trash content synchronization, then macOS will move an item to the trash and inform us through the modifyitem(_:baseversion:changedfields:contents:options:request:completionhandler:) call about the new parent identifier for the trashed item which then is trashContainer. Only when the trash is emptied, we get the deleteItem call for the trashed item.
Conclusion
Because you might have expected the feature to work differently and its scheduled removal and the general scheduled and conflicting improvement of the trash support, I recommend not to invest further effort and close this pull request.
|
@i2h3 I have a lot of files in the trash on the Nextcloud server, and I don't want them to appear in my Mac's trash folder, or for the server's trash to be emptied when I empty my Mac's trash folder. How do I configure this? |
Currently, there is no way to configure this from user side. The enumeration of remote trash items was subject of internal discussion because it is even more questionable in use cases of organizations which have a lot of group folders and where even items deleted by other users appear in the trash. The plan for the next bigger client release includes to reduce the synchronized items in the trash to those which actually were deleted locally (exclude items deleted by other users or in the web user interface). |
|
I'm closing the pull request and referring to my new tested version. |
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
2 participants
Summary
Test plan