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)
SeaMonkey
OS Integration
Tracking
(Not tracked)
RESOLVED
FIXED
seamonkey2.1final
People
(Reporter: InvisibleSmiley, Assigned: InvisibleSmiley)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
4.67 KB,
patch
|
neil
:
review+
|
Details | Diff | Splinter Review |
The FF implementation is in nsBrowserContentHandler.js, handleFlagWithParam("search" and function doSearch. SMOP with no l10n impact.
Assignee | ||
Updated•13 years ago
|
Severity: normal → enhancement
Assignee | ||
Comment 1•13 years ago
|
||
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.]
Comment 2•13 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•13 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•13 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•13 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•13 years ago
|
||
(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•13 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•13 years ago
|
Attachment #524990 -
Flags: review?(neil) → review+
Assignee | ||
Comment 8•13 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•13 years ago
|
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.
Description
•