Thunar allows copying folder into itself

Thunar 4.15.1

Thunar seems to have protection against copying a folder into itself, but this seems to only work for the first 100 folders.

Repro:

• Create 150 folders next to each other (for i in {0..149}; do mkdir $i; done)
• Select all in Thunar with Ctrl+a
• Initiate Drag'n'drop

-> The selection cannot be dropped into the first 100 selected folders, but it can be dropped into the 101st and subsequent ones. Of course, doing so breaks, and Thunar shows an "Invalid filename" message.

Cheers!

changed the description

changed the description

I can reproduce that strange behavior, you just earned the obscure bug award :P

Searching for "100" in the codebase reveals 'thunar_file_accepts_drop':

  /* check up to 100 of the paths (just in case somebody tries to
   * drag around his music collection with 5000 files).
   */
  for (lp = file_list, n = 0; lp != NULL && n < 100; lp = lp->next, ++n)

Patches are welcome !

Had a look at this, these were my changes to get the correct functionality:

diff --git a/thunar/thunar-file.c b/thunar/thunar-file.c
index 279e8918..2d5c1acf 100644
--- a/thunar/thunar-file.c
+++ b/thunar/thunar-file.c
@@ -2071,10 +2071,7 @@ thunar_file_accepts_drop (ThunarFile     *file,
       if (thunar_file_is_trashed (file))
         actions &= ~(GDK_ACTION_COPY | GDK_ACTION_LINK);

-      /* check up to 100 of the paths (just in case somebody tries to
-       * drag around his music collection with 5000 files).
-       */
-      for (lp = file_list, n = 0; lp != NULL && n < 100; lp = lp->next, ++n)
+      for (lp = file_list; lp != NULL; lp = lp->next)
         {
           /* we cannot drop a file on itself */
           if (G_UNLIKELY (g_file_equal (file->gfile, lp->data)))
@@ -2107,8 +2104,7 @@ thunar_file_accepts_drop (ThunarFile     *file,
           /* default to move as suggested action */
           suggested_action = GDK_ACTION_MOVE;

-          /* check for up to 100 files, for the reason state above */
-          for (lp = file_list, n = 0; lp != NULL && n < 100; lp = lp->next, ++n)
+          for (lp = file_list; lp != NULL; lp = lp->next)
             {
               /* dropping from the trash always suggests move */
               if (G_UNLIKELY (thunar_g_file_is_trashed (lp->data)))

I did a quick test with dragging 5000 files and didn't notice any obvious performance impact on my pretty slow machine (N3710 - 4 core @ 1.6 GHz with 4 GB memory and 5400 RPM spinning disk).

That said, timing the thunar_file_accepts_drop function, there's pretty large performance impact.

Time per call to thunar_file_accepts_drop while dragging 5000 files, times in milliseconds:

Test	Max	Median	90th %ile
Current	2	0.5	1
Patch	17	10	14

I guess the good news is it could be worse if the time increased linearly (50x worse for a list of 5000 vs. 100).

Something I noticed while testing is that thunar_file_accepts_drop is called redundantly, a lot. It is called each time the mouse moves while files are selected. So most of the time we are calling it with the same drop destination file and selected file list.

Caching the previous result is possible so that we only call thunar_file_accepts_drop for a different destination file and/or selected file list. I have a rough prototype that I could polish if that sounds like something worth pursuing. My thought is that it's probably not worth the added complexity given the perceived performance impact is negligible and dragging around 1000s of files is probably not a common use case.

Thank you for the investigations !

Could you please open a merge request or provide a patch via git format-patch ? That simplifies testing and giving credit for me!

n is now unused, you can drop it !

thunar-file.c:2045:17: warning: unused variable ‘n’ [-Wunused-variable] 2045 | guint n;

That line need to be dropped:

/* check for up to 100 files, for the reason state above */

Time per call to thunar_file_accepts_drop while dragging 5000 files, times in milliseconds:

Just to get this right: If you dont move the mouse, it is not called at all ? Does the mouse lag because of that ? If not, it's fine for me.

If you want to tweak performance, probably some rate limiting for the method would be a simple fix. E.g. always wait 100ms before executing the code and drop all calls which arrived meanwhile, use the data of the latest call after the 100ms.

Before applying the patch, it would be great if you could check the impact for remote loactions, e.g. for sftp://something .. just to be sure we dont talk about seconds there.

Just to get this right: If you dont move the mouse, it is not called at all ? Does the mouse lag because of that ? If not, it's fine for me.

That's right, it only appears to be called when moving the mouse (dragging the selection). I observe no mouse lag.

Before applying the patch, it would be great if you could check the impact for remote loactions, e.g. for sftp://something .. just to be sure we dont talk about seconds there.

I ran a quick test using an ssh server on my phone. Dragging a selection of 5000 files showed no noticeable lag.

Attaching the patch: 0001-Remove-limit-on-number-of-paths-to-check-in-thunar_f.patch

closed via commit 82fc3299

Thank you for your contribution ! Will be released in thunar 4.15.2 / 4.16.0

Thanks! I can confirm that 82fc3299 fixes the test case. There's some minor CPU usage while dragging around the folders, but that's acceptable.