Closed
Bug 598623
Opened 14 years ago
Closed 14 years ago
addEngine* APIs are broken in multiple ways
Categories
(Firefox :: Search, defect)
Tracking
()
RESOLVED
DUPLICATE
of bug 493051
People
(Reporter: BenB, Unassigned)
Details
(This bug technically belongs in Toolkit, the component is toolkit/components/search/nsSearchService.js and the API is netwerk/base/public/nsIBrowserSearchService.idl, but there's only the bugzilla Search component in Firefox product.)
The objective is:
* Extension installs 3 search plugins as OSD files.
(They are an important part of the extension.)
The install happens unconditionally without user confirmation,
but the default search engine must not be changed.
* Extension puts up UI, asks user whether the new search engine may be the
default. If granted, the extension changes the default to one of the
new engines.
This seems impossible with the current nsIBrowserSearchService API and implementation. It has several function that seem to *almost* do that, but all of them have so serious problems that we had 3 engineers trying to solve it, for 1-2 days, without success and proper code. We were all very frustrated.
* addEngine
This seems entirely appropriate. It takes an URL, reads all values
from our OSD files, exactly what we want.
It even has a |confirm| param and says it won't change the default
in that case, exactly what we want. However, that doesn't work.
* It does change the default, even with confirm = false. That alone
is a serious problem when using the function. We'd have to store
the currentEngine value, call addEngine, and reset currentEngine.
Unfortunately, that leads to the second problem.
* addEngine is async, but has no callback. It doesn't tell me when
it finished! So, doing
var default = ss.currentEngine;
ss.addEngine(...);
ss.currentEngine = default;
will fail to work. There's only an "engine-added" message that's
sent around globally . That's broken API design, but worse,
it leads to the third problem:
* if I call addEngine() several times, to add several engines,
I don't know which "engine-added" means that it's really done.
Also, I can't be sure that there isn't coincidentally some other
code which does addEngine() at the same time (after restart) which
sends me "engine-added". Basically, I need to know when *my* engine
was added, not when *some* engine was added. The connection between
my addEngine() call and the callback is entirely missing.
* addEngine*() does not return the new nsISearchEngine, which I'll
need to set currentEngine. I have to just "know" the *name* of it
and use getEngineByName/Alias. So, that means I have to add another
constant variable in my code (or parse the OSD myself) just to know
the name. Worse, it's user-visible and therefore translatable. I just
want to get the nsISearchEngine for my OSD file.
* So, what's necessary is a
addEngine(url, successCallback, errorCallback)
signature, and a proper implementation that calls the callback only
for *my* engine. The successCallback(newEngine) would have one
parameter, the nsISearchEngine that has been added, i.e. the searchserv
passes it back to me.
* addEngineWithDetails()
I'd have to parse the OSD file, pass the contents as params one-by-one.
The function would then re-construct an OSD file and save it. However,
there's no way to pass the suggestion URL, so I can't use that. More
generally, that's breaking whenever the OSD file contains new
params/features. That's why this approach is broken and I want to pass
the whole OSD *file*.
Reporter | ||
Comment 1•14 years ago
|
||
See bug 598630 for a workaround: copy files and reload().
Comment 2•14 years ago
|
||
There's a patch here for some of these problems.
https://bugzilla.mozilla.org/show_bug.cgi?id=493051
Comment 3•14 years ago
|
||
(In reply to comment #0)
> * addEngineWithDetails()
> I'd have to parse the OSD file, pass the contents as params one-by-one.
> The function would then re-construct an OSD file and save it. However,
> there's no way to pass the suggestion URL, so I can't use that.
To clarify on this point: there is no way to modify the URLs used by the SearchEngine instance once it's created. It'd be nice if _urls were made accessible.
Reporter | ||
Comment 4•14 years ago
|
||
Actually, the patch there solves all the problems listed above for addEngine(), as far as I read it. Marking DUP.
Reporter | ||
Updated•14 years ago
|
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•