Closed Bug 648781 Opened 13 years ago Closed 13 years ago

Add support for Firefox's -search command line option

Categories

(SeaMonkey :: OS Integration, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.1final

People

(Reporter: InvisibleSmiley, Assigned: InvisibleSmiley)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

The FF implementation is in nsBrowserContentHandler.js, handleFlagWithParam("search" and function doSearch. SMOP with no l10n impact.
Severity: normal → enhancement
Attached patch patch (obsolete) — Splinter Review
I'm not sure about bsmedberg's comment, maybe you can enlighten me.

While I was at it I also added some more options to helpInfo.

I left out "? searchterm" for Windows Vista start menu integration (FF nsBrowserContentHandler.js line 555 and following) for now since FF uses preprocessing to limit it to Windows and that's so frowned upon here. [And since I didn't port it I didn't test it.]
Assignee: nobody → jh
Status: NEW → ASSIGNED
Attachment #524877 - Flags: review?(neil)
(In reply to comment #1)
> I'm not sure about bsmedberg's comment, maybe you can enlighten me.
The -url parameter respects tabbed browsing preferences, but -search always opens a new window.

> I left out "? searchterm" for Windows Vista start menu integration (FF
> nsBrowserContentHandler.js line 555 and following) for now since FF uses
> preprocessing to limit it to Windows and that's so frowned upon here. [And
> since I didn't port it I didn't test it.]
Odd, I thought we'd already implemented something to support ? search term for Process Explorer integration. (But maybe that works slightly differently.)
(In reply to comment #2)
> (In reply to comment #1)
> > I'm not sure about bsmedberg's comment, maybe you can enlighten me.
> The -url parameter respects tabbed browsing preferences, but -search always
> opens a new window.

Heh, I guess I wasn't clear enough. That part I understood, it was the last bit that I didn't get: "use handURIToExistingBrowser ... but need nsIBrowserDOMWindow extensions". In other words, if handURIToExistingBrowser does the right thing, what exactly is the reason it wasn't used?
(In reply to comment #3)
> In other words, if handURIToExistingBrowser
> does the right thing, what exactly is the reason it wasn't used?
I looked more closely and it doesn't support post data. (nsIBrowserDOMWindow doesn't support post data either, but then again the internal caller only ever asks for about:blank anyway and just loads what it wants in the new tab.)
Comment on attachment 524877 [details] [diff] [review]
patch

>+  // fill our nsISupportsArray with uri-as-wstring, null, null, postData
>+  var sa = Components.classes["@mozilla.org/supports-array;1"]
>+                     .createInstance(Components.interfaces.nsISupportsArray);
nsISupportsArray is deprecated. Please use nsIArray.

>+  var wuri = Components.classes["@mozilla.org/supports-string;1"]
>+                       .createInstance(Components.interfaces.nsISupportsString);
>+  wuri.data = submission.uri.spec;
openWindow (not to be confused with wwatch.openWindow) calls this argstring.
So maybe this should be called uristring?

>     if (cmdLine.handleFlag("preferences", false)) {
>       openPreferences();
>       cmdLine.preventDefault = true;
>     }
> 
>     if (cmdLine.handleFlag("silent", false))
>       cmdLine.preventDefault = true;
> 
>+    var searchParam = cmdLine.handleFlagWithParam("search", false);
>+    if (searchParam) {
>+      doSearch(searchParam, features);
>+      cmdLine.preventDefault = true;
>+    }
Nit: I think I'd prefer search before preferences (and in helpinfo obviously).
(In reply to comment #5)
> nsISupportsArray is deprecated. Please use nsIArray.

ITYM nsIMutableArray. Some notes here:
0. I guess you assumed I knew. Someone less knowledgeable might have given up after searching around endlessly (the code in this phase isn't exactly easy to debug).
1. Your statement looks like copied from MDC, which isn't exactly precise either. Or rather *was*.
2. I have no idea whether the second parameter could be "true" as well. This back-end low-level stuff scares me. Bad enough that MDC expects everyone to know. I'm just glad I somehow made it through.
Attachment #524877 - Attachment is obsolete: true
Attachment #524877 - Flags: review?(neil)
Attachment #524990 - Flags: review?(neil)
(In reply to comment #6)
> 0. I guess you assumed I knew. Someone less knowledgeable might have given up
> after searching around endlessly (the code in this phase isn't exactly easy to
> debug).
Yes, sorry I just think of nsIArray first. Besides, MDC normally seems to mention them both in the same breath.

> 1. Your statement looks like copied from MDC, which isn't exactly precise
> either.
Probably the XPCOM Array Guide.

> 2. I have no idea whether the second parameter could be "true" as well. This
> back-end low-level stuff scares me. Bad enough that MDC expects everyone to
> know. I'm just glad I somehow made it through.
The page on nsIMutableArray seems to be clear enough on this point.
Attachment #524990 - Flags: review?(neil) → review+
Comment on attachment 524990 [details] [diff] [review]
patch v2 [Checkin: comment 8]

http://hg.mozilla.org/comm-central/rev/fca529126184
Attachment #524990 - Attachment description: patch v2 → patch v2 [Checkin: comment 8]
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1final
You need to log in before you can comment on or make changes to this bug.