341803 - Allow the CCK to specify a default searchengine

Status: RESOLVED FIXED

(Other Applications :: CCK, defect)

Description

[Benjamin Smedberg]

Currently you can add search engines using the CCK, but you can't specify that a search engine should be the default. The upcoming patch will fix that.

Comment 1

[Benjamin Smedberg]

Attachment: Specify default searchengine, rev. 1

Comment 2

[Mike Kaply [:mkaply]]

Should we do something with search engine number 2 as well? Like move 1 to 2 and 2 to 3? Or just replace the default with the new one?

In a current Firefox, this would leave Yahoo 2nd and relegate google to alphabetical with everyone else.

Incidentally, I need to recheck on current builds, but last time I checked, search engines added by the CCK were automatically put at the top of the list (although they weren't made default.

Comment 3

[Benjamin Smedberg]

(In reply to comment #2)
> Should we do something with search engine number 2 as well? Like move 1 to 2
> and 2 to 3? Or just replace the default with the new one?

I don't think we should worry about #2. Unfortunately, we don't know in advance what #1 and #2 were (depends on the locale), and there's not a good way to "bump everything down a notch".

Comment 4

[Mike Kaply [:mkaply]]

setting the pref to data:text/plain,browser.search.defaultenginename=Foo

or just

Foo

Is browser.search.defaultenginename a complex value?

Same with browser.search.order.1

Also, should we be smarter and actually query the names out of the src files so we can give the user a list?

Finally, it appears that browser.search.selectedEngine gets set the first time you open the browser which is rather frustrating, so setting the default search engine only works on a global install with a new profile.

I'd almost be tempted to put this in my FAQ as "how to do this by setting prefs" vs. putting it in the UI, unless we can make the UI smarter.

Comment 5

[Benjamin Smedberg]

> Is browser.search.defaultenginename a complex value?

Yes: http://lxr.mozilla.org/mozilla1.8.0/source/browser/app/profile/firefox.js#203

> Same with browser.search.order.1

Yes, http://lxr.mozilla.org/mozilla1.8.0/source/browser/app/profile/firefox.js#207

> Also, should we be smarter and actually query the names out of the src files so
> we can give the user a list?

We should, but I'd like to do that as a second pass.

> I'd almost be tempted to put this in my FAQ as "how to do this by setting
> prefs" vs. putting it in the UI, unless we can make the UI smarter.

Setting the prefs by hand is tricky because of the data-URL bits; I'd prefer to add this directly (and add additional UI as needed).

Comment 6

[Mike Kaply [:mkaply]]

Attachment: Specify default searchengine, rev. 2

Here's a patch that reads the name of the default searchengine from the file and uses it.

I also made the listbox prettier - it uses the real name of the searchengine, not the path.

One thing still missing is text for "no default engine" I need to come up with that.

Comment 7

[Mike Kaply [:mkaply]]

Attachment: Specify default searchengine, rev. 3

OK, this is a final patch.

I went ahead and moved the values themselves into the properties file mainly because they might involve translation in the future.

I added error checking for the SRC files which I like a lot.

Note this only affects new profiles since browser.search.selectedEngine is always set in an existing profile. I thought about setting that as well in the CCK, but that seemed evil.

Comment 8

[Benjamin Smedberg]

> I went ahead and moved the values themselves into the properties file mainly
> because they might involve translation in the future.

What!?

If it lives in a .properties file, it should be in a content package, not a locale package: I don't think we should be messing with localized defaults for this.

Comment 9

[Mike Kaply [:mkaply]]

(In reply to comment #8)
> > I went ahead and moved the values themselves into the properties file mainly
> > because they might involve translation in the future.
> 
> What!?
> 
> If it lives in a .properties file, it should be in a content package, not a
> locale package: I don't think we should be messing with localized defaults for
> this.
> 

My only point was that if at some future point, we modified the CCK to contain multiple languages in one CCK, we would want any strings that might get translated in properties files, not in the JS files, and there would be less work if the strings were already there.

The main reason I did it this way instead of data/text URLs is to be consistent with the way things are done in Firefox today:

pref("browser.search.defaultenginename",      "chrome://browser-region/locale/region.properties");
pref("browser.search.order.1",                "chrome://browser-region/locale/region.properties");

So for the CCK, it will be:

pref("browser.search.defaultenginename",      "chrome://cck/content/cck.properties");
pref("browser.search.order.1",                "chrome://cck/content/cck.properties");

which is prettier than using the data/text URL.

That's how I have done things in the past.

Comment 10

[Mike Kaply [:mkaply]]

latest patch checked in.