winebrowser and multi-word commands

[Yuriy]

Hi,

I noticed that winebrowser doesn't work with browsers/commands that
are more than one word.  For example, the command to run konqueror
with the webbrowsing profile on kubuntu is:

kfmclient openProfile webbrowsing [url]

Setting that to be the first attempted webbrowser in the registry:

[HKEY_USERS\S-1-5-4\Software\Wine\WineBrowser]
"Browsers"="kfmclient openProfile
webbrowsing,konqueror,firefox,mozilla,netscape,galeon,opera,dillo"

and running:
winebrowser http://www.winehq.org

yields this:
Considering: kfmclient openProfile webbrowsing
argv[1]: http://www.winehq.org
Considering: firefox
argv[1]: http://www.winehq.org

then it opens up with firefox.

Is this a bug?  Would it just be a matter of modifying the winebrowser
script to fix?

~Yuriy

[Yuriy]

Hello,

Ok, so I realized that winebrowser is an actual program, not a script.
 I fixed the bug, but I'm sending the patch here first so somebody can
look over it because I've never written more than a couple lines of C
before.

This patch allows commands with up to 4 arguments, such as "kfmclient
exec" to be used for winebrowser:

diff --git a/programs/winebrowser/main.c b/programs/winebrowser/main.c
index 690f931..600d06b 100644
--- a/programs/winebrowser/main.c
+++ b/programs/winebrowser/main.c
@@ -51,20 +51,39 @@ typedef LPSTR (*wine_get_unix_file_name_
 /* try to launch an app from a comma separated string of app names */
 static int launch_app( char *candidates, const char *argv1 )
 {
-    char *app;
-    const char *argv_new[3];
+    char *app, *appname;
+    const char *argv_new[6];
+    int i1 = 0, i2 = 0, i3 = 0, len = 0;

app = strtok( candidates, "," );
     while (app)
     {
-        argv_new[0] = app;
-        argv_new[1] = argv1;
-        argv_new[2] = NULL;
+        i2 = 0;
+        i3 = 0;
+        len = strlen(app);
+        appname = (char *)malloc(len);
+        for(i1 = 0; i1 < len && i3 < 4; i1++)
+        {
+            appname[i1] = app[i1];
+            if (app[i1] == ' ')
+            {
+                app[i1] = '\0';
+                argv_new[i3] = app + i2;
+                i3++;
+                i2 = i1 + 1;
+            }
+        }
+
+        appname[i1] = '\0';
+        argv_new[i3] = app+i2;
+        argv_new[i3+1] = argv1;
+        argv_new[i3+2] = NULL;

-        fprintf( stderr, "Considering: %s\n", app );
+        fprintf( stderr, "Considering: %s\n", appname );
         fprintf( stderr, "argv[1]: %s\n", argv1 );

spawnvp( _P_OVERLAY, app, argv_new );  /* only returns on error */
+        free(appname);
         app = strtok( NULL, "," );  /* grab the next app */
     }
     fprintf( stderr, "winebrowser: could not find a suitable app to run\n" );

On 8/2/06, Yuriy  wrote:
> Hi,
>
> I noticed that winebrowser doesn't work with browsers/commands that
> are more than one word.  For example, the command to run konqueror
> with the webbrowsing profile on kubuntu is:
>
> kfmclient openProfile webbrowsing [url]
>
> Setting that to be the first attempted webbrowser in the registry:
>
> [HKEY_USERS\S-1-5-4\Software\Wine\WineBrowser]
> "Browsers"="kfmclient openProfile
> webbrowsing,konqueror,firefox,mozilla,netscape,galeon,opera,dillo"
>
> and running:
> winebrowser http://www.winehq.org
>
> yields this:
> Considering: kfmclient openProfile webbrowsing
> argv[1]: http://www.winehq.org
> Considering: firefox
> argv[1]: http://www.winehq.org
>
> then it opens up with firefox.
>
> Is this a bug?  Would it just be a matter of modifying the winebrowser
> script to fix?
>
> ~Yuriy

[Yuriy]

Hi,

I sent this to wine-patches last week and it hasn't been accepted yet,
so I'm wondering if there is something wrong with the patch.  Here it
is again:

index 690f931..81b2f42 100644
--- a/programs/winebrowser/main.c
+++ b/programs/winebrowser/main.c
@@ -51,20 +51,39 @@ typedef LPSTR (*wine_get_unix_file_name_
 /* try to launch an app from a comma separated string of app names */
 static int launch_app( char *candidates, const char *argv1 )
 {
-    char *app;
-    const char *argv_new[3];
+    char *app, *appname;
+    const char *argv_new[7];
+    int i1 = 0, i2 = 0, i3 = 0, len = 0;

app = strtok( candidates, "," );
     while (app)
     {
-        argv_new[0] = app;
-        argv_new[1] = argv1;
-        argv_new[2] = NULL;
+        i2 = 0;
+        i3 = 0;
+        len = strlen(app);
+        appname = (char *)malloc(len);
+        for(i1 = 0; i1 < len && i3 < 4; i1++)
+        {
+            appname[i1] = app[i1];
+            if (app[i1] == ' ')
+            {
+                app[i1] = '\0';
+                argv_new[i3] = app + i2;
+                i3++;
+                i2 = i1 + 1;
+            }
+        }
+
+        appname[i1] = '\0';
+        argv_new[i3] = app+i2;
+        argv_new[i3+1] = argv1;
+        argv_new[i3+2] = NULL;

-        fprintf( stderr, "Considering: %s\n", app );
+        fprintf( stderr, "Considering: %s\n", appname );
         fprintf( stderr, "argv[1]: %s\n", argv1 );

spawnvp( _P_OVERLAY, app, argv_new );  /* only returns on error */
+        free(appname);
         app = strtok( NULL, "," );  /* grab the next app */
     }
     fprintf( stderr, "winebrowser: could not find a suitable app to run\n" );

On 8/7/06, Yuriy  wrote:
> Hello,
>
> Ok, so I realized that winebrowser is an actual program, not a script.
>  I fixed the bug, but I'm sending the patch here first so somebody can
> look over it because I've never written more than a couple lines of C
> before.
>
> This patch allows commands with up to 4 arguments, such as "kfmclient
> exec" to be used for winebrowser:

[Yuriy]

Hi,

I got back to working on this, and the redone patch works now.  This
patch does the following (advantages over the previous attempt):
- changes the browser list to a REG_MULTI_SZ value
- if it is not a REG_MULTI_SZ, the old behavior remains for now
- if there are arguments, the name of the browser should be in quotes
- if there is a %s, the url is placed there rather than at the end
- any number of arguments.

Patch attached, thoughts?

~Yuriy

On 8/17/06, Yuriy  wrote:
> Hi,
>
> I sent this to wine-patches last week and it hasn't been accepted yet,
> so I'm wondering if there is something wrong with the patch.  Here it
> is again: