Page MenuHome

Blender file explorer hard deletes data
Closed, ResolvedPublic

Description

Operating system: Windows 10
Graphics card: GTX1080Ti

Blender Version
Broken: 2.8

Short description of error
Deleting a file in Blender's file explorer on Windows hard deletes the file. I don't think that's how any Windows app should behave. Common standard for deleting files within all Windows applications I know of should be soft deletion, which means moving the files to recycle bin. One accidental deletion can be completely destructive.

Exact steps for others to reproduce the error
1, On Windows, open Blender
2, In Blender, open File Browser and delete any file or folder

Result: The file has been hard deleted (not present in recycle bin)

Expected: The file has been soft deleted (moved to recycle bin)

Details

Event Timeline

Yeah this is really scary. Currently, if you are not careful, you can completely screw up your system from the Blender File Browser. This is not acceptable behaviour.

IMO we should either remove the ability to delete stuff from the File Browser completely, or make it do what you suggest - to use the system Trash / Recycle Bin.

It's not a bug though - but a critical issue. Not sure how to triage.

It's not a bug though - but a critical issue. Not sure how to triage.

Yes, I wasn't sure about this either, but at the same time, I can't imagine putting this on rightclickselect. That's where ideas go to die. I think Blender needs some more official channel for things that are not necessarily new feature requests, but not bugs either. Something called "Issues" with some voting system. That would be ideal place for things like this, or other things that are broken (such as texture baking) but not technically bugged.

William Reynish (billreynish) triaged this task as Confirmed, Medium priority.EditedFeb 11 2019, 1:00 PM

I'll mark it as a confirmed bug because of the severity of the danger of this issue. We could consider it a bug that it doesn't use the system Trash, even though it's not a normal bug in the traditional sense.

At the very least, it's misguided behaviour that can cause real destructive harm.

This raises some questions - should a file selector allow destructive operations at all (I quite like that Gnome/GTK file selector is readonly).

Even if this is done, Blender has a file-space type - where renaming/deleting is something we probably want to keep.

Setting as a TODO.

I vote for ability to retain all the functionality, just changing what delete does from hard delete to moving to recycle bin. I am not familiar how hard it is to implement on Windows, but if it's not too hard, that would be the way to go. All the other Windows software has either its own file browser or uses the Windows one, but in both cases, deleting is expected to mean moving to recycle bin.

@Kris (Metricity) SHFileOperation is the legacy API which doesn't support paths longer than 255 characters. If XP support is required we could use the VersionHelper to determine the OS version running and use IFileOperation if it's Vista or later. Otherwise I'd go with IFileOperation , because that is the the current API.

Looks like Windows made this one a bit more difficult. The FOFX_RECYCLEONDELETE was introduced for Windows 8, so I'm not sure how that functionality could be achieved on Vista using the new API. Perhaps we should use SHFileOperation for Vista, Windows 7 and IFileOperation for newer versions?

I'm doing some test to see if my idea with IFileOperation works.

You don't have to worry about XP, we don't support it.

I had a quick look at IFileOperation and was unsure regarding the single threaded/multi threaded apartment notes so went with older method, I hadn't considered the path limitation.

@Kris (Metricity) I have implemented a first version that works, but I'll have to do more testing. The API's documentation is C++ only, which made it a bit difficult. I'm also not sure whether the choice between COINIT_APARTMENTTHREADED vs COINIT_MULTITHREADED makes a big difference. For simplicity I chose COINIT_APARTMENTTHREADED which is what the official examples use and by the description in the documentation it won't cause us any headaches with concurrency.

@Kris (Metricity) I'm not sure that the BLI_delete_recycle is a good idea, since the Linux version does not recycle. I see that there are use cases where blender wants to delete without moving to the recycling bin, however the name is not suited for the Linux delete function

Perhaps we could do the following: We could implement BLI_delete_recycle, but use #ifdef WIN32 in file_delete_exec to call this function only if it's Windows. This would avoid confusion about the Linux function call, which in your patch is just a wrapper for the regular BLI_delete.

I've got the patch done, doing some last tests and then I'll submit a diff based on the code of @Kris (Metricity)

@Kris (Metricity) thanks for implementing this! I hope you don't mind my improvement suggestions.

I was expecting feedback as this was my first time working with the blender codebase. The intention behind the separate function was if it could be used in other places and I wondered if linux or mac should have an implementation. It may also be worth reviewing if the regular delete functionality handles long paths too.

@Kris (Metricity) I agree, the separate function makes sense. It's a good idea to have this functionality for Linux and MacOS, too. I'm not sure if Linux has a unified way of handling the trash directory. If Linux and MacOS have the path set in an environment variable it should be a simple mv operation.

It may also be worth reviewing if the regular delete functionality handles long paths too.

Yes it can handle long paths. See the 'tip' box. The difference between DeleteFileA and DeleteFileW is that the former uses ANSI characters, the latter Unicode/UTF-16.

Sorry for hijacking your first contribution. I just thought this was something I could contribute to as well. Please don't feel discourage if the devs choose to not include your implementation using the legacy API. There is nothing wrong your implementation. If I would've known that we wouldn't need both the legacy and the new API, I would've left this for you to implement.

@William Reynish (billreynish) I guess this should be addressed for Linux and MacOS as well. Currently remove() and rmdir() are used for Linux and MacOS which are equally destructive.

@Robert Guetzkow (rjg) Yes, absolutely. This should be done for all the OS’s.

Moving a file to the trash is actually quite a bit harder than mv:
https://specifications.freedesktop.org/trash-spec/trashspec-1.0.html

On Linux command line tools can be used, like this:
https://github.com/electron/electron/blob/master/atom/common/platform_util_linux.cc#L100

On macOS there are native API functions.

@Brecht Van Lommel (brecht) You're right I was oversimplifying. What I meant was if $XDG_DATA_HOME is set we know where to move the files to (+handle metadata for restoring). The bigger challenge may be that some distributions don't follow the spec and $XDG_DATA_HOME isn't set. For instance on my Ubuntu VMs 16.04 and 18.04 both don't have $XDG_DATA_HOME set. Your linked code from Electron probably already solves this by using the cli tools, which will default to gio trash for Ubuntu with Gnome as far as I can tell.

Marking as resolved for now. Now you cannot easily hard delete data at least.

Later we should add support for the system trash via native API's.