Closed Bug 344159 Opened 13 years ago Closed 13 years ago

Ensure FF2 search service interacts appropriately with CCK extensions for partner and corporate deployment

Categories

(Firefox :: Search, defect)

2.0 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 2 beta2

People

(Reporter: benjamin, Assigned: benjamin)

Details

(Keywords: fixed1.8.1)

Attachments

(2 files, 1 obsolete file)

In FF 1.5 the CCK (and extensions in general) can modify many aspects of search engine behavior, including changing the default search plugin, changing branding parameters, and reordering search plugins. We need to ensure that similar (and enhanced) capabilities are available in FF2.
Basil, can you post detailed requirements here?
Flags: blocking-firefox2?
Flags: blocking-firefox2? → blocking-firefox2+
Target Milestone: --- → Firefox 2 beta2
The product team has been using Michael Kaply's CCK Wizard (https://addons.mozilla.org/firefox/2553/) to generate an XPI that includes the branding and customization required for various distribution partners. Most (but not all) of the requirements listed below are available from the CCK Wizard.

With regard to search plugins in particular, the requirements are:

Defaults:
- Extensions (such as the XPI generated from the CCK Wizard) can include one or more search plugins
- Select a new default search engine (from the existing list packaged with the browser)
- Install a new search engine and select it as the new default search engine
- Even when adding new search engines, do not change the default search engine

Install/Uninstall/Ordering:
- Search engines can be added at a particular location (e.g. slot 2)
- Allow extensions to replace existing search engines (e.g. in order to point to a different URL or modify the URL parameters)
- Allow extensions to remove existing search engines
- When an extension is uninstalled, the search plugin will no longer be present

URL Parameters:
- Allow extensions to add URL parameters to existing search engine URL parameters
(For modifications to URL parameters, it is suggested to replace th entire search engine plugin, rather than patch the parameters) 

> branding and customization required for various distribution partners. Most
> (but not all) of the requirements listed below are available from the CCK
> Wizard.

But not for FF2, which will require a new version of the CCK ;-)
Removing existing search plugins could be problematic...

Also, the reordering is tricky as well.

The only thing that comes to mind here is a way in Firefox to say "ignore the installed search engines" and allow the CCK to do everything?
Whiteboard: [at risk]
Whiteboard: [at risk] → [at serious risk]
Is there a timeline on when we'll build a CCK for Fx2?

-> Benjamin for now, please reassign to the appropriate people if there are in fact issues to resolve.
Assignee: nobody → benjamin
Attachment #231607 - Flags: review? → review?(sspitzer)
Attachment #231625 - Flags: review?(gavin.sharp)
With these two patches we're at parity with FF1.5. Removal and "insertion" via prefs doesn't look that hard, I'll take a stab at that once these land.
I'd rather not have two prefs to change when localizers need to change the default. Can't you avoid adding another default localized pref by comparing this._currentEngine to this.defaultEngine before calling setLocalizedPref, and calling clearUserPref if they're the same? Something like:

if (this._currentEngine == this.defaultEngine) {
  if (hasUserPref(selectedEnginePref))
    clearUserPref(selectedEnginePref)
} else {
  setLocalizedPref(...)
}

in the currentEngine setter.
OS: Linux → All
Hardware: PC → All
Version: unspecified → 2.0 Branch
Put more clearly: move the check for equality to the setLocalizedPref caller, to avoid adding a default for browser.search.selectedEngine, and compare engines instead of engine names, since you have easy access to both engines.
Attachment #231625 - Attachment is obsolete: true
Attachment #231642 - Flags: review?(gavin.sharp)
Attachment #231625 - Flags: review?(gavin.sharp)
Comment on attachment 231642 [details] [diff] [review]
Don't set user prefs which match the defaults, rev. 2

>Index: browser/components/search/nsSearchService.js

>+    if (this._currentEngine.name == this.defaultEngine.name) {

Remove the |.name|s and just compare the objects directly (these are just references to the JS objects).

>+    }
>+    else {

This file uses huggy else brackets :) |} else {|
Attachment #231642 - Flags: review?(gavin.sharp) → review+
Comment on attachment 231607 [details] [diff] [review]
Make extension searchplugins override app-installed, rev. 1

r=sspitzer, this looks reasonable to me.

note, I think that AppendObject() can fail (but if this code fails, and we are out of memory, we have bigger problems.)

note, according to lxr, many callers of AppendObject() don't seem to check the return value either.

what about making AppendFileKey() return a nsresult?

return array.AppendObject(file) ? NS_OK : NS_ERROR_OUT_OF_MEMORY;

up to you, ben.
Attachment #231607 - Flags: review?(sspitzer) → review+
Yeah, I thought about failing out, but I figured it was better to return a partially complete list than fail altogether.
Both patches fixed on trunk.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Attachment #231607 - Flags: approval1.8.1?
Attachment #231642 - Flags: approval1.8.1?
Comment on attachment 231607 [details] [diff] [review]
Make extension searchplugins override app-installed, rev. 1

a=mconnor on behalf of drivers for checkin to 1.8 branch for 1.8.1
Attachment #231607 - Flags: approval1.8.1? → approval1.8.1+
Comment on attachment 231642 [details] [diff] [review]
Don't set user prefs which match the defaults, rev. 2

a=mconnor on behalf of drivers for checkin to 1.8 branch for 1.8.1
Attachment #231642 - Flags: approval1.8.1? → approval1.8.1+
Both patches landed on MOZILLA_1_8_BRANCH
Keywords: fixed1.8.1
Whiteboard: [at serious risk]
You need to log in before you can comment on or make changes to this bug.