Bug 61858 - [GTK] run-launcher should run MiniBrowser in WebKit2
Summary: [GTK] run-launcher should run MiniBrowser in WebKit2
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2011-06-01 06:42 PDT by Carlos Garcia Campos
Modified: 2011-06-01 09:08 PDT (History)
0 users

See Also:


Attachments
Patch (1.18 KB, patch)
2011-06-01 06:44 PDT, Carlos Garcia Campos
mrobinson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2011-06-01 06:42:42 PDT
Since r87760 we are passing -2 command line option to run-launcher script for wk2, but we are still executing GtkLauncher and it fails because -2 option is passed to GtkLauncher too. We should check whether -2 option has been passed, so that it's removed, and run MiniBrowser instead.
Comment 1 Carlos Garcia Campos 2011-06-01 06:44:46 PDT
Created attachment 95594 [details]
Patch
Comment 2 Martin Robinson 2011-06-01 08:33:56 PDT
Comment on attachment 95594 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=95594&action=review

> Tools/Scripts/run-launcher:72
> +            # isWK2() removes -2 option from @ARGV.
> +            @args = @ARGV;

Why is it necessary to preserve "-2" here? I ask because Qt does not do this.
Comment 3 Carlos Garcia Campos 2011-06-01 08:37:40 PDT
(In reply to comment #2)
> (From update of attachment 95594 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=95594&action=review
> 
> > Tools/Scripts/run-launcher:72
> > +            # isWK2() removes -2 option from @ARGV.
> > +            @args = @ARGV;
> 
> Why is it necessary to preserve "-2" here? I ask because Qt does not do this.

I guess the Qt program ignores options that it doesn't handle, MiniBrowser returns with an error because -2 option is invalid. I added this, because I wasn't sure, so I wanted to make sure I didn't break other ports.
Comment 4 Martin Robinson 2011-06-01 08:43:38 PDT
Comment on attachment 95594 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=95594&action=review

>>> Tools/Scripts/run-launcher:72
>>> +            @args = @ARGV;
>> 
>> Why is it necessary to preserve "-2" here? I ask because Qt does not do this.
> 
> I guess the Qt program ignores options that it doesn't handle, MiniBrowser returns with an error because -2 option is invalid. I added this, because I wasn't sure, so I wanted to make sure I didn't break other ports.

Ah, I see. I misunderstood the comment. You might consider expanding it to something like: "isWK2() removes the -2 option from @ARGV, but not @args. We do not want to pass -2 to MiniBrowser."
Comment 5 Carlos Garcia Campos 2011-06-01 08:47:50 PDT
(In reply to comment #4)
> (From update of attachment 95594 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=95594&action=review
> 
> >>> Tools/Scripts/run-launcher:72
> >>> +            @args = @ARGV;
> >> 
> >> Why is it necessary to preserve "-2" here? I ask because Qt does not do this.
> > 
> > I guess the Qt program ignores options that it doesn't handle, MiniBrowser returns with an error because -2 option is invalid. I added this, because I wasn't sure, so I wanted to make sure I didn't break other ports.
> 
> Ah, I see. I misunderstood the comment. You might consider expanding it to something like: "isWK2() removes the -2 option from @ARGV, but not @args. We do not want to pass -2 to MiniBrowser."

Sure, thanks for reviewing!
Comment 6 Carlos Garcia Campos 2011-06-01 09:08:41 PDT
Committed r87808: <http://trac.webkit.org/changeset/87808>