| Summary: | Moving the desktop directory doesn't check if the destination directory is a subdirectory of the orginial | ||
|---|---|---|---|
| Product: | [Applications] systemsettings | Reporter: | Martin Berglund <mabe02> |
| Component: | kcm_desktoppath | Assignee: | Unassigned bugs mailing-list <unassigned-bugs> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | aacid, finex, kde, mabe02 |
| Priority: | NOR | ||
| Version: | 4.1 | ||
| Target Milestone: | --- | ||
| Platform: | Compiled Sources | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
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 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? 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 ;.) There are now a lot of checks to make sure we never move the home directory. |
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() )