Bug 256650

Summary: Move in column view gives files wrong names and wrong folders
Product: [Unmaintained] kio Reporter: Todd <toddrme2178>
Component: generalAssignee: David Faure <faure>
Status: RESOLVED FIXED    
Severity: major CC: adawit, faure, peter.penz19
Priority: NOR    
Version: SVN   
Target Milestone: ---   
Platform: openSUSE   
OS: Linux   
Latest Commit: Version Fixed In: 4.6.4
Sentry Crash Report:
Attachments: Fix for the problem

Description Todd 2010-11-12 02:51:21 UTC 
Version:           unspecified (using KDE 4.5.3) 
OS:                Linux

When I am in column view and I try to drop files on a higher-level folder that already has files with that name, I am of course given the option to rename, overwrite, or skip.  However, if I pick rename, instead of renaming them based on the original name and putting them in the destination folder, it renames all but the first one based on the destination folder name and puts them in the next level up.  This means the original file names are irrevocably lost and the files end up in the wrong location.

Reproducible: Always

Steps to Reproduce:
1. Make a folder, call it "test"
2. Enter the folder
3. Make 2 sub-folders, test1 and test2
4. Enter test1, make 5 blank text files, file_1.txt, file_2.txt, file_3.txt, file_4.txt, and file_5.txt
5. Copy those files to test2.
6. Go to test, and change to columns view.
7. Enter test1.  You should still see the contents of test in the first column.
8. Select all the files in test1.  
9. Drag and drop the files to test2 in the test column
10. Select "Move"
11. Click "apply to all" then "rename"

Actual Results:  
You get 4 files in test, test2_1, test2_2, test2_3, and test2_4, and one file in test2, file_6.txt

Expected Results:  
You get 5 files in test2, file_6.txt, file_7.txt, file_8.txt, file_9.txt, file_10.txt

This does not happen when copying files, only moving them.
Comment 1 Peter Penz 2010-11-12 21:38:50 UTC 
Thanks for the report, I could reproduce this issue with KDE SC 4.5.x and trunk. But it is unrelated to the column-view in Dolphin (tested also with other views) and seems to be an issue in the dialog in kdelibs that offers the "Apply to all" switch for renaming (that copying files works, indicates that Dolphin passes the correct data).
Comment 2 Todd 2011-02-01 02:45:43 UTC 
This is still a problem in KDE 4.6 final.  Any progress on a fix?  

I think this bug probably warrants a "cirtical" priority.  At least in my opinion, considering an extremely common operating (moving files from one folder to another when there are name conflicts) will invariably lead to radically altered file names without any chance of recovery and, if the file names contain important information which they often do, unrecoverable data loss probably warrants a much higher priority than "normal".
Comment 3 Todd 2011-02-18 00:02:00 UTC 
Created attachment 57340 [details]
Fix for the problem

This patch fixes the problem.
Comment 4 Peter Penz 2011-02-18 14:25:30 UTC 
@Todd: Thanks for the patch. Would it be possible that you add this patch to git.reviewboard.kde.org? David gets so many notifications through bugs.kde.org that it is very easy for him to miss a patch like this...
Comment 5 Todd 2011-02-18 15:38:17 UTC 
I don't have an account.

This is a patch for a data-loss bug.  It should not be "easy" to miss this.  If it is then there would seem there is a problem with how bugzilla sends notifications that should be fixed.
Comment 6 David Faure 2011-02-21 21:59:35 UTC 
Well, there are currently 1661 unread emails in my kde-bugzilla/kio subfolder :-(

I'm completely swamped and unable to cope with the amount of bug reports.
I read in priority the bugs (e.g. kdelibs) where I'm personally cc'ed (different email folder), while this one was "just" a kio bug among 1661 others.

Thanks for the personal email notification, I'm having a look now.
Comment 7 David Faure 2011-02-21 22:57:41 UTC 
I'm surprised that this patch is enough to fix the bug, because I'm writing a unittest for it, and I'm finding many more issues. In particular, when moving to another partition (as opposed to doing it in the same partition), the faulty code is elsewhere (around line 1845, fixed locally) and on top of that, suggestName() is called for each file but before other files get moved, so it can't yet see the future files, and I end up with all auto-renamed files named exactly the same... (file_6.txt in your example).

This requires more work -> tomorrow.
Comment 8 Todd 2011-02-21 23:26:18 UTC 
For the test case I listed at the beginning the patch fixes.  There may be other problems that aren't fixed by this, though.  

That being said, this patch is necessary.  The way suggestName() was being called was simply incorrect, it needs the target directory for a file but was given the target path (including the file name) instead.  So whatever other problems there might be, this patch is still needed.

As for remote partitions, suggestName() by design does not work properly for those (you can see that in a code comment, line 433 and 434 in kio/kio/renamedialog.cpp).  That is certainly a major issue, but is independent of the problem described in this bug report.
Comment 9 David Faure 2011-02-22 09:16:24 UTC 
Git commit cfaa411382b2c2f8e965bca2dbdf9d6f31b65e29 by David Faure.
Committed on 22/02/2011 at 08:50.
Pushed by dfaure into branch 'KDE/4.6'.

Fix autorename feature so that it calls suggestName() correctly.

It was moving files into other directories unexpectedly.
Based on toddrme's patch, but also handles other cases (same partition
vs different partition), and a fix for non-interactive mode in order to
test this feature from a unittest (trunk only, needs new API)

BUG: 256650
FIXED-IN: 4.6.1
(cherry picked from commit fa1e90f4ba42f1267c12d1a28de3acf9fcbebb01)

M  +26   -14   kio/kio/copyjob.cpp     

http://commits.kde.org/kdelibs/cfaa411382b2c2f8e965bca2dbdf9d6f31b65e29
Comment 10 David Faure 2011-02-22 09:17:42 UTC 
Git commit 3f8b2f4da5cf5e122ed34471a214c5b7462b3e82 by David Faure.
Committed on 22/02/2011 at 08:50.
Pushed by dfaure into branch 'master'.

Fix autorename feature so that it calls suggestName() correctly.

It was moving files into other directories unexpectedly.
Based on toddrme's patch, but also handles other cases (same partition
vs different partition), and a fix for non-interactive mode in order to
test this feature from a unittest (trunk only, needs new API)

BUG: 256650
FIXED-IN: 4.6.1

M  +26   -14   kio/kio/copyjob.cpp     

http://commits.kde.org/kdelibs/3f8b2f4da5cf5e122ed34471a214c5b7462b3e82
Comment 11 Todd 2011-03-19 19:40:31 UTC 
I am using 4.6.1, and this bug is better, but it is not fixed.  The files have the correct name now, and I am able to undo, but files still consistently end up in the wrong directory (the parent of the directory they should end up in).  I am not sure why this is happening.
Comment 12 Dawit Alemayehu 2011-05-08 23:59:09 UTC 
(In reply to comment #11)
> I am using 4.6.1, and this bug is better, but it is not fixed.  The files have
> the correct name now, and I am able to undo, but files still consistently end
> up in the wrong directory (the parent of the directory they should end up in). 
> I am not sure why this is happening.

I can confirm that by using the test case outlined.
Comment 13 Dawit Alemayehu 2011-05-09 05:21:38 UTC 
(In reply to comment #12)
> (In reply to comment #11)
> > I am using 4.6.1, and this bug is better, but it is not fixed.  The files have
> > the correct name now, and I am able to undo, but files still consistently end
> > up in the wrong directory (the parent of the directory they should end up in). 
> > I am not sure why this is happening.
> 
> I can confirm that by using the test case outlined.

The issue is caused by the fact that setting a file name on a URL that contains a path without trailing slash means the last directory gets treated as if it was a filename.

In this case, the m_dest url did contained a file that does not end with a trailing slash "/", i.e. the path ends as "test/test2". So whenever m_dest.setFileName(newName) was called in kio/kio/copyjob.cpp  line#1872, it simply treated "test2" as if it was a filename and overwrote with the newName. This meant the all conflicting files that were set to be renamed except for the very first one ended up one directory up. Anyhow, I will commit a fix for it shortly.
Comment 14 Dawit Alemayehu 2011-05-09 05:23:52 UTC 
Git commit 76e53125d22db9151c345fa1d3e3b467eab512af by Dawit Alemayehu.
Committed on 09/05/2011 at 05:10.
Pushed by adawit into branch 'KDE/4.6'.

Properly rename a file when it conflicts with an existing file during a move
operation. Fixes the remaining issue in bug# 256650.

BUG: 256650
FIXED-IN: 4.6.4

M  +1    -0    kio/kio/copyjob.cpp     

http://commits.kde.org/kdelibs/76e53125d22db9151c345fa1d3e3b467eab512af
Comment 15 Dawit Alemayehu 2011-05-09 05:24:59 UTC 
Git commit ba78676af7fe0a2469e8d4f17a944a3d30fa8209 by Dawit Alemayehu.
Committed on 09/05/2011 at 05:10.
Pushed by adawit into branch 'master'.

Properly rename a file when it conflicts with an existing file during a move
operation. Fixes the remaining issue in bug# 256650.

CCBUG: 256650
FIXED-IN: 4.6.4

(cherry picked from commit 76e53125d22db9151c345fa1d3e3b467eab512af)

M  +1    -0    kio/kio/copyjob.cpp     

http://commits.kde.org/kdelibs/ba78676af7fe0a2469e8d4f17a944a3d30fa8209