361061 - params added nsISearchEngine::addParam() are sometimes forgotten after a restart (especially for engines added without icons)

Status: RESOLVED FIXED

(Firefox :: Search, defect)

Description

[Malte Kraus]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061116 BonEcho/2.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061116 BonEcho/2.0

When a search engine is created using addEngineWithDetails and no or an invalid icon URI is specified, the parameters added later using nsISearchEngine::addEngine for this engine are not serialized to the file. This means, that the created engines only work until Firefox is restarted.

Reproducible: Always

Steps to Reproduce:
1. execute this javascript code with privileges (from js console or something):
var iconURL = "";
/* var iconURL = "http://www.google.com/favicon.ico"; */
var ss = Cc["@mozilla.org/browser/search-service;1"]
           .getService(Ci.nsIBrowserSearchService);
var name = "Google 2";
ss.addEngineWithDetails(name, iconURL, "", "", "GET", "http://www.google.com/search");
var addedEngine = ss.getEngineByName(name);
addedEngine.addParam("q", "{searchTerms}", null);

2. try the newly created engine, see that it works
2. restart Firefox
3. notice that the engine does not work anymore

(check that this has something to do with the icon:)
4. delete the engine
5. execute the above code again but with the first instead of the second line commented out
6. see that the engine works even after restarting a hundred times

Comment 1

[Malte Kraus]

After some further debugging I think I found the actual reason for this bug:
_serializeToFile() is only called when the icon download finishes. The reason it works for engines with a icon is that the script can add the params before the download finishes. If you wait long enough, it doesn't work for engines with icons either.

var ss = Cc["@mozilla.org/browser/search-service;1"]
           .getService(Ci.nsIBrowserSearchService);
var name = "Google 2";
ss.addEngineWithDetails(name, "http://www.google.com/favicon.ico", "", "", "GET",
"http://www.google.com/search");
var addedEngine = ss.getEngineByName(name);
window.setTimeout(function() { addedEngine.addParam("q", "{searchTerms}", null); }, 1000);

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch

This uses a lazy-write timer technique (almost identical to what cookies do, see nsCookieService::LazyWrite()) to avoid multiple safeoutputstream writes in a row, which is a common case when adding an engine (add multiple params at a time).

Comment 3

[Asaf Romano (gone)]

Comment on attachment 247922 [details] [diff] [review]
patch

>+  _lazySerializeToFile: function SRCH_ENG_serializeToFile() {
>+    if (this._serializeTimer)
>+      this._serializeTimer.delay = LAZY_SERIALIZE_DELAY;

Please comment here that this re-initializes the timer.

r=mano.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Attachment: with comment

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Checked in on the trunk. Thanks for reporting this, Malte!

mozilla/browser/components/search/nsSearchService.js 	1.91