mimehandling plugin tweaks

[dmoore]

"Use Associated App" currently fails for paths with spaces:

Code Select Expand


Index: src/plugins/defaultmimehandler/defaultmimehandler.cpp
===================================================================
--- src/plugins/defaultmimehandler/defaultmimehandler.cpp (revision 5719)
+++ src/plugins/defaultmimehandler/defaultmimehandler.cpp (working copy)
@@ -301,10 +301,10 @@
         ShellExecute(0, wxString(_T("open")).c_str(), filename.c_str(), 0, 0, SW_SHOW);
         #endif
         #ifdef __WXGTK__
-        wxExecute(wxString::Format(_T("xdg-open %s"), filename.c_str()));
+        wxExecute(wxString::Format(_T("xdg-open \"%s\""), filename.c_str()));
         #endif
         #ifdef __WXMAC__
-        wxExecute(wxString::Format(_T("open %s"), filename.c_str()));
+        wxExecute(wxString::Format(_T("open \"%s\""), filename.c_str()));
         #endif
         return 0;
     }


ok?

[MortenMacFly]

ok?

If that is working like that on the MAC and GTK - sure, go ahead.

[oBFusCATed]

And what about escaping the double quotes in the filename?

[dmoore]

And what about escaping the double quotes in the filename?

what do we currently use to escape filenames? alternatively, i'll just call the wxExecute variant that takes the argv array directly, which should handle the paths without escaping them.

[oBFusCATed]

I'm not 100% sure, but replacing " with \" in the filename would do the trick, because that is the purpose of the quotes around the file name.

[dmoore]

I'm not 100% sure, but replacing " with \" in the filename would do the trick, because that is the purpose of the quotes around the file name.

Are there other special character corner cases? I'm not happy about hardcoding a filename escaping routine into the plugin (I was pretty sure we already have this code floating around -- maybe in the compiler plugin?).

For now, I think I'm going to test out the argv variant of wxExecute...

[dmoore]

I'm not 100% sure, but replacing " with \" in the filename would do the trick, because that is the purpose of the quotes around the file name.

Are there other special character corner cases? I'm not happy about hardcoding a filename escaping routine into the plugin (I was pretty sure we already have this code floating around -- maybe in the compiler plugin?).

For now, I think I'm going to test out the argv variant of wxExecute...

aargh... the wxExecute variant I was thinking of takes a char **argv -- what encoding is that supposed to be !? Rather than guess, I'll leave it to wxWidgets to sort out and go back to the commandline wxExecute variant...

I see we have a QuoteStringIfNeeded in globals.h but it doesn't substitute \" for already present ". Shall I fix this as surely it breaks on any filenames containing " characters? (I'd like to think that fixing is unecessary, because " should be rare and unsupported on most OSes and something people capable of writing code wouldn't use)

anyway, assuming yes to fix, the current patch is

Code Select Expand


Index: src/sdk/globals.cpp
===================================================================
--- src/sdk/globals.cpp (revision 5723)
+++ src/sdk/globals.cpp (working copy)
@@ -191,7 +191,10 @@
bool hasSpace = str.Find(_T(' ')) != -1;
bool hasParen = !platform::windows && (str.Find(_T('(')) != -1 || str.Find(_T(')')) != -1);
     if (!str.IsEmpty() && str.GetChar(0) != _T('"') && (hasSpace || hasParen))
+    {
+        str.Replace(_T("\""),_T("\\\""));
         str = wxString(_T("\"")) + str + _T("\"");
+    }
}

wxString EscapeSpaces(const wxString& str)
Index: src/plugins/defaultmimehandler/defaultmimehandler.cpp
===================================================================
--- src/plugins/defaultmimehandler/defaultmimehandler.cpp (revision 5723)
+++ src/plugins/defaultmimehandler/defaultmimehandler.cpp (working copy)
@@ -301,10 +301,14 @@
         ShellExecute(0, wxString(_T("open")).c_str(), filename.c_str(), 0, 0, SW_SHOW);
         #endif
         #ifdef __WXGTK__
-        wxExecute(wxString::Format(_T("xdg-open %s"), filename.c_str()));
+        wxString escaped_filename(filename);
+        QuoteStringIfNeeded(escaped_filename);
+        wxExecute(wxString::Format(_T("xdg-open %s"), escaped_filename.c_str()));
         #endif
         #ifdef __WXMAC__
-        wxExecute(wxString::Format(_T("open %s"), filename.c_str()));
+        wxString escaped_filename(filename);
+        QuoteStringIfNeeded(escaped_filename);
+        wxExecute(wxString::Format(_T("open %s"), escaped_filename.c_str()));
         #endif
         return 0;
     }