Hello Jules!
FileChooser with useOSNativeDialogBox=true doesn’t work correctly on Linux:
- dialogBoxTitle is displayed with surrounding double quotes
- dialogBoxTitle breaks if you send it embedded double quotes
- initialFileOrDirectory is not working, ie the file or directory is not selected in the dialog
Reasons for this are:
-
zenity, (the gtk dialog program which is used to display the file dialog), doesn’t take a full path to --filename=. One must chdir to the relevant directory, then give only the file basename as argument. (cd /etc/; zenity --file-selection --filename=passwd).
-
FileChooser::showPlatformDialog escapes the title and file arguments with double quotes, as if preparing to send them to a shell like /bin/sh. However, the assembled command is sent to ChildProcess::start() which uses execvp() to execute the command. execvp() sends the arguments verbatim to the new process, hence they should not be escaped or quoted.
This example produces a file-not-found-error, since there is no file called “/etc/passwd” (including the quotes):
char* arglist_quoted[] = {"/bin/ls", "-l", "\"/etc/passwd\"", NULL};
execvp ("/bin/ls", arglist_quoted);ChildProcess::start() also splits the command on whitespace, producing the argument list for execvp(). This causes an error either way: if arguments are quoted, the split is correct but the quotes remain wrongly in the argument; if arguments are not quoted they are broken when containing whitespace. ChildProcess::start() tries to emulate commandline parsing but fails.
Some ways to go about this:
- Call /bin/sh -c with the arguments, ie keep the quoting/escaping and pass it on to the shell
- Alter ChildProcess::start() to accept a StringArray. This avoids the split on whitespace, and all arguments are sent verbatim to the new process.
- Add an optional separator-argument: ChildProcess::start(const String &command, const String &commandSeparator=String(" \t")). Use commandSeparator to split the argument list, and trust the caller to choose something which doesn’t interfer with the actual arguments.
- A variable argument list ChildProcess::start("/bin/ls", “-l”, “/etc/passwd”), but this is frowned upon in C++.
Of the above, 1 is the worst choice. The caller should not have to worry about correct quoting and escaping for /bin/sh. I prefer 2, it is to me the cleanest approach. Compose your StringArray with the exact arguments needed - no quoting/escaping - and you’re done.
The patch below, to current git, overloads start() with a StringArray argument. It also modifies FileChooser to use it, and fixes the other errors mentioned above. Code like this will now work correctly:
$ touch "/tmp/the file"
FileChooser("test \"a b c\" ; --q ", File("/tmp/the file")).browseForFileToOpen();diff --git a/modules/juce_core/native/juce_posix_SharedCode.h b/modules/juce_core/native/juce_posix_SharedCode.h
index 80c2faf..aa57438 100644
--- a/modules/juce_core/native/juce_posix_SharedCode.h
+++ b/modules/juce_core/native/juce_posix_SharedCode.h
@@ -1069,7 +1069,13 @@ bool ChildProcess::start (const String& command)
{
StringArray tokens;
tokens.addTokens (command, true);
- tokens.removeEmptyStrings (true);
+ return start (tokens);
+}
+
+bool ChildProcess::start (const StringArray& command)
+{
+ StringArray tokens (command);
+ tokens.removeEmptyStrings (false);
if (tokens.size() == 0)
return false;
diff --git a/modules/juce_core/threads/juce_ChildProcess.h b/modules/juce_core/threads/juce_ChildProcess.h
index 454a6c6..7aa94b5 100644
--- a/modules/juce_core/threads/juce_ChildProcess.h
+++ b/modules/juce_core/threads/juce_ChildProcess.h
@@ -56,6 +56,7 @@ public:
occurs, the method will return false.
*/
bool start (const String& command);
+ bool start (const StringArray& command);
/** Returns true if the child process is alive. */
bool isRunning() const;
diff --git a/modules/juce_gui_basics/native/juce_linux_FileChooser.cpp b/modules/juce_gui_basics/native/juce_linux_FileChooser.cpp
index 39da863..ce0fa1c 100644
--- a/modules/juce_gui_basics/native/juce_linux_FileChooser.cpp
+++ b/modules/juce_gui_basics/native/juce_linux_FileChooser.cpp
@@ -45,15 +45,33 @@ void FileChooser::showPlatformDialog (Array<File>& results,
FilePreviewComponent* previewComponent)
{
const String separator (":");
- String command ("zenity --file-selection");
+ StringArray command (String("zenity"));
+ command.add (String("--file-selection"));
- if (title.isNotEmpty()) command << " --title=\"" << title << "\"";
- if (file != File::nonexistent) command << " --filename=\"" << file.getFullPathName () << "\"";
- if (isDirectory) command << " --directory";
- if (isSave) command << " --save";
- if (selectMultipleFiles) command << " --multiple --separator=" << separator;
+ if (title.isNotEmpty()) command.add (String("--title=") + title);
+ if (isDirectory) command.add (String("--directory"));
+ if (isSave) command.add (String("--save"));
+ if (selectMultipleFiles)
+ {
+ command.add (String("--multiple"));
+ command.add (String("--separator=") + separator);
+ }
- command << " 2>&1";
+ File previousWorkingDirectory = File::getCurrentWorkingDirectory();
+ if (file.exists())
+ {
+ if (file.isDirectory())
+ {
+ file.setAsCurrentWorkingDirectory();
+ }
+ else
+ {
+ file.getParentDirectory().setAsCurrentWorkingDirectory();
+ command.add (String("--filename=") + file.getFileName());
+ }
+ }
+
+ command.add (String("2>&1"));
ChildProcess child;
if (child.start (command))
@@ -75,4 +93,6 @@ void FileChooser::showPlatformDialog (Array<File>& results,
child.waitForProcessToFinish (60 * 1000);
}
+
+ previousWorkingDirectory.setAsCurrentWorkingDirectory();
}