Bug 281952 - KOpenWithDialog: Wrong handling of paths with spaces
Summary: KOpenWithDialog: Wrong handling of paths with spaces
Status: RESOLVED FIXED
Alias: None
Product: kio
Classification: Frameworks and Libraries
Component: kfile (show other bugs)
Version: unspecified
Platform: Ubuntu Linux
: NOR normal
Target Milestone: ---
Assignee: Ingomar Wesp
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-13 16:29 UTC by Victor Varvaryuk
Modified: 2012-01-27 13:20 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.8


Attachments
example of bug (58.52 KB, image/png)
2011-09-13 16:29 UTC, Victor Varvaryuk
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Victor Varvaryuk 2011-09-13 16:29:57 UTC
Created attachment 63621 [details]
example of bug

Version:           unspecified (using KDE 4.7.0) 
OS:                Linux

When adding an application launcher via Browse, it inserts path with non-escaped paths.

Reproducible: Always

Steps to Reproduce:
Try to add launcher browsing to a path with spaces (see the attachment)


Expected Results:  
Either selected path should be with spaces escaped with '\', or must handle paths without escaped spaces properly.
Comment 1 Etienne Charlier 2011-11-27 23:30:28 UTC
I just got the same behaviour on my system: kubuntu 11.10 ( kde 4.7.3)
Comment 2 Reza 2011-12-26 03:09:21 UTC
I can confirmed this on 4.8 master
Comment 3 Reza 2011-12-26 13:22:22 UTC
Seems the real bug is not inside quicklaunch.
Seem the checking inside KOpenWithDialog::checkAccept will not pass path with spaces.
Comment 4 Ingomar Wesp 2011-12-29 23:12:12 UTC
> Seems the real bug is not inside quicklaunch.
> Seem the checking inside KOpenWithDialog::checkAccept will not pass path with
> spaces.

Indeed. checkAccept calls KRun::binaryName(…) on the entered text, which treats the first space as a separator between an executable and its arguments. A reasonable solution would probably be to assume that whitespace is part of the path as long as the path denotes an existing file.

I'll try to come up with a patch for the KOpenWithDialog.
Comment 5 Ingomar Wesp 2012-01-16 14:50:27 UTC
Git commit 63c7ae800b22d0d9d6d5a43a21f8e4473994298d by Ingomar Wesp.
Committed on 16/01/2012 at 15:45.
Pushed by iwesp into branch 'KDE/4.8'.

Make sure that paths selected via the file dialog are properly quoted. Since the
line edit expects shell-style quoting, this patch avoids file-not-found errors
when selecting a path that contains whitespace or special characters.
FIXED-IN: 4.8
REVIEW: 103602

M  +9    -0    kio/kfile/kopenwithdialog.cpp
M  +1    -0    kio/kfile/kopenwithdialog.h

http://commits.kde.org/kdelibs/63c7ae800b22d0d9d6d5a43a21f8e4473994298d
Comment 6 Victor Varvaryuk 2012-01-16 16:11:40 UTC
edit->setText(KShell::quoteArg(edit->text()));

Would it be better to quote the path when passing it the part of the application which uses it, instead of quoting it after selecting a file?

This would allow manually pasting/entering a path with spaces via Ctrl-V to the line edit.
Also it's not user friendly to see 'Heroes\ of\ Might ...' in the line edit.
Also it would a problem to copy this path to another application, which doesn't expect '\' in the text.

IMO
Comment 7 Ingomar Wesp 2012-01-16 19:42:04 UTC
(In reply to comment #6)
> Would it be better to quote the path when passing it the part of the
> application which uses it, instead of quoting it after selecting a file?

Maybe. OTOH, how would you specify command line arguments then? Your proposed solution (as well as the one I initially suggested in comment #4) would require some logic that tries to guess which whitespaces are part of the path and which ones are separators of arguments. And you could still run into ambiguities:

Assume the following two executables:

"/bin/foo"
"/bin/foo bar"

When the user types "/bin/foo bar", should this be interpreted as executable "foo" with parameter "bar" or did the user mean the executable "foo bar"?

> Also it's not user friendly to see 'Heroes\ of\ Might ...' in the line edit.

Luckily, KShell::quoteArg returns 'Heroes of Might …', which is better IMHO. But I do see your point.

Anyways, that's just my opinion, so if you're not convinced, I would suggest filing a new report (severity: wishlist) against kio/kfile.
Comment 8 Ingomar Wesp 2012-01-16 19:44:34 UTC
> Luckily, KShell::quoteArg returns 'Heroes of Might …', which is better IMHO.

I should have written "'Heroes of Might …'" to make it clear that the single quotes are part of the string. Sorry.
Comment 9 Victor Varvaryuk 2012-01-17 06:39:16 UTC
Ok, just wanted to be sure that there is no better solution.

What about instead of having "path/to/My\ Application %u" after file selection put there "'path/to/My Application' %u" - smth like that?
Comment 10 Ingomar Wesp 2012-01-17 09:52:29 UTC
(In reply to comment #9)
> Ok, just wanted to be sure that there is no better solution.

I agree that this is not perfect. 

I'm also not the maintainer of this part of kdelibs, so I really suggest opening a new report (people will probably not see your comment if they're attached to a report that is already closed) in order to discuss this.
Comment 11 Sebastian Trueg 2012-01-27 13:20:54 UTC
Git commit cfc52e240ae5a15f049f578c50b9c58eadb3203a by Sebastian Trueg, on behalf of Ingomar Wesp.
Committed on 16/01/2012 at 15:45.
Pushed by trueg into branch 'KDE/4.8'.

Make sure that paths selected via the file dialog are properly quoted. Since the
line edit expects shell-style quoting, this patch avoids file-not-found errors
when selecting a path that contains whitespace or special characters.
FIXED-IN: 4.8
REVIEW: 103602

M  +9    -0    kio/kfile/kopenwithdialog.cpp
M  +1    -0    kio/kfile/kopenwithdialog.h

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