Closed Bug 654420 Opened 9 years ago Closed 9 years ago

"Get more search engines..." in Engine Manager does nothing if no browser open

Categories

(SeaMonkey :: Search, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: kairo, Assigned: neil)

References

Details

Attachments

(1 file, 1 obsolete file)

To get something in fast for SeaMonkey 2.1, bug 595246 punted on the case of a missing browser window when "Get more search engines..." is called from Engine Manager. We should implement a working solution here.
My job just got easier. It turns out that preferences.xul already includes utilityOverlay, so I can call opener.top.openUILinkIn from engineManager.js
Attached patch Proposed patch (obsolete) — Splinter Review
The window.arguments stuff is to fix a problem opening the load add search engine page from the engine manager when opened from preferences when an existing navigator window is open - because the dialog is modal it refocuses preferences when it closes thus we can't open and focus the tab until then.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #532323 - Flags: review?(iann_bugzilla)
Attachment #532323 - Flags: feedback?(kairo)
Comment on attachment 532323 [details] [diff] [review]
Proposed patch

When you first start a browser you get in the error console:
Error: loadAddSearchEngines is not defined
Source File: chrome://navigator/content/navigator.js
Line: 1230

I know it is just moving code, but is best to define a const instead of just using 3 in loadAddSearchEngines()?
Attachment #532323 - Flags: review?(iann_bugzilla) → review-
(In reply to comment #3)
> When you first start a browser you get in the error console:
> Error: loadAddSearchEngines is not defined
> Source File: chrome://navigator/content/navigator.js
> Line: 1230
Oops, the files must load in the "wrong" order.

> I know it is just moving code, but is best to define a const instead of just
> using 3 in loadAddSearchEngines()?
Better still, I'll use the existing nsIBrowserDOMWindow.OPEN_NEWTAB const :-)
Attached patch Fixed patchSplinter Review
Even better, uses the kNewTab constant. Also fixes a search panel bug (oops).
Attachment #532323 - Attachment is obsolete: true
Attachment #532323 - Flags: feedback?(kairo)
Attachment #532442 - Flags: review?(iann_bugzilla)
Comment on attachment 532442 [details] [diff] [review]
Fixed patch

r=me

Not sure if it is just on linux but in the search pref pane there is never a search engine showing as selected.
Attachment #532442 - Flags: review?(iann_bugzilla) → review+
(In reply to comment #6)
> Not sure if it is just on linux but in the search pref pane there is never a
> search engine showing as selected.
That's a separate bug (also applies to 2.1, sigh...)
Pushed changeset 85db216cd817 to comm-central.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Depends on: 825586
You need to log in before you can comment on or make changes to this bug.