Add support for Firefox's -search command line option

RESOLVED FIXED in seamonkey2.1final

Status

SeaMonkey
OS Integration
--
enhancement
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: InvisibleSmiley, Assigned: InvisibleSmiley)

Tracking

(Blocks: 1 bug)

Trunk
seamonkey2.1final

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
The FF implementation is in nsBrowserContentHandler.js, handleFlagWithParam("search" and function doSearch. SMOP with no l10n impact.
(Assignee)

Updated

6 years ago
Severity: normal → enhancement
(Assignee)

Comment 1

6 years ago
Created attachment 524877 [details] [diff] [review]
patch

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)

Comment 2

6 years ago
(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.)
(Assignee)

Comment 3

6 years ago
(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?

Comment 4

6 years ago
(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 5

6 years ago
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).
(Assignee)

Comment 6

6 years ago
Created attachment 524990 [details] [diff] [review]
patch v2 [Checkin: comment 8]

(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)

Comment 7

6 years ago
(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.

Updated

6 years ago
Attachment #524990 - Flags: review?(neil) → review+
(Assignee)

Comment 8

6 years ago
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]
(Assignee)

Updated

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1final
You need to log in before you can comment on or make changes to this bug.