Bug 171479 - Moving the desktop directory doesn't check if the destination directory is a subdirectory of the orginial
Summary: Moving the desktop directory doesn't check if the destination directory is a ...
Status: RESOLVED FIXED
Alias: None
Product: systemsettings
Classification: Applications
Component: kcm_desktoppath (show other bugs)
Version: 4.1
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Unassigned bugs mailing-list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-09-22 23:06 UTC by Martin Berglund
Modified: 2015-04-17 16:25 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Berglund 2008-09-22 23:06:24 UTC
Version:            (using Devel)
Compiler:          G++ 4.2.3 
OS:                Linux
Installed from:    Compiled sources

In System Settings -> About Me -> Paths, I tried to correct a previous mistake when I had earlier set the desktop (or was is Documents?) path to my home dir (~). I changed it to ~/Desktop and I was asked if I wanted to move my files in the desktop directory to the new location. I clicked "Move" before I realized I probably shouldn't and then suddenly KDE started behaving in weird ways. The reason was, of course, that my .kde4 and all my other files and directories in ~ had been moved to ~/Desktop until the system tried to move ~/Desktop to ~/Desktop/ which didn't work (and crashed there perhaps? I didn't check the logs).

Anyway, I thought a simple check making sure the new directory isn't a subdirectory of the previous one would be quite easy and here's a patch (see below).

I wasn't sure if I chose the right component for the bug reporting, for the user it's a part of System Settings, but I found the code in konqueror (took me a while to find it...). Still, reporting this as a konqueror bug seems a bit far fetched.

Oh, and, if this gets accepted, it will be my first contribution to any open source project. Anyway, here's my patch:

Index: apps/konqueror/settings/konq/globalpaths.cpp
===================================================================
--- apps/konqueror/settings/konq/globalpaths.cpp	(revision 863683)
+++ apps/konqueror/settings/konq/globalpaths.cpp	(working copy)
@@ -288,6 +288,12 @@
     if (!src.isLocalFile() || !dest.isLocalFile())
         return true;
     m_ok = true;
+
+    // If dest is a subfolder of src, NEVER move the files 
+    // and directories of src to dest. It won't work, so don't bother asking.
+    if ( src.isParentOf(dest) )
+        return m_ok;    // always true
+
     // Ask for confirmation before moving the files
     if ( KMessageBox::questionYesNo( this, i18n("The path for '%1' has been changed;\ndo you want the files to be moved from '%2' to '%3'?",
                               type, src.path(), dest.path()), i18n("Confirmation Required"),KGuiItem(i18n("Move")),KStandardGuiItem::cancel() )
Comment 1 Albert Astals Cid 2008-10-05 15:05:17 UTC
Moving a dir to one of its children works here, you get a warning saying the children could not be moved to itself, but everything else is moved, i would say that your problem is that you moved ~ to another dir and that's usually not something the user wants to do, so if src is the home i would:
a) Not move
b) Ask again if the user is really sure he wants to move
Comment 2 Martin Berglund 2008-10-08 15:20:39 UTC
Ok, I didn't get any warning when I did it, maybe someone added that after I made the report. I'll make another checkout and try again. However, moving the content of a directory to one of it's subdirectories really isn't correct by definition and shouldn't be allowed. Moving everything in ~ is certainly even more wrong. So, either just asking for confirmation or informing the user that the content couldn't be moved, I guess?
Comment 3 Albert Astals Cid 2008-10-09 00:33:39 UTC
So what about this:

 - First, do the question that is already on the code
 - If he says yes, after that, if src.isParentOf(dest) ask a new question telling the user this is unusual to do and ask him if he is sure
 - If he says yes, after that, if src is the home of the user ask a new question telling the user this might be disruptive and ask him if he is sure

Getting three messageboxes in a row is probably more than enough so you end up reading at least one of them and pressing Cancel on time if that's not exactly what you want to do.


On the other hand we might just do the check you do now plus a message saying the files would not be moved from foo to bar because bar is a child of foo, that the user should move any document himself manually.

I'm not really sure, you choose ;.)
Comment 4 David Edmundson 2015-04-17 16:25:00 UTC
There are now a lot of checks to make sure we never move the home directory.