Closed Bug 598623 Opened 14 years ago Closed 14 years ago

addEngine* APIs are broken in multiple ways

Categories

(Firefox :: Search, defect)

Other
All
defect
Not set
normal

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*.
See bug 598630 for a workaround: copy files and reload().
There's a patch here for some of these problems. https://bugzilla.mozilla.org/show_bug.cgi?id=493051
(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.
Actually, the patch there solves all the problems listed above for addEngine(), as far as I read it. Marking DUP.
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.