Open Bug 1547662 Opened 1 year ago Updated 2 months ago

Deleted built-in Search Engine add-ons are not removed from Addon Manager's local profile cache

Categories

(Firefox :: Search, defect, P3)

defect
Points:
3

Tracking

()

People

(Reporter: standard8, Unassigned)

References

(Blocks 1 open bug, Regression)

Details

I just noticed this on one of my test profiles, since we removed some of the built-in search engines in bug 1545517.

STR:

  1. Download the fr locale build from 15 April
  2. Run the build with a fresh profile. Check the search engines.
  3. Run the profile in the latest nightly build
  4. Observe the browser console.

Actual Results

Error while loading 'jar:file:///Volumes/Firefox%20Nightly/Firefox%20Nightly.app/Contents/Resources/browser/omni.ja!/chrome/browser/search-extensions/cnrtl-tlfi-fr/manifest.json' (NS_ERROR_FILE_NOT_FOUND)
addons.xpi	WARN	Exception running bootstrap method startup on cnrtl-tlfi-fr@search.mozilla.org: Error: Error while loading 'jar:file:///Volumes/Firefox%20Nightly/Firefox%20Nightly.app/Contents/Resources/browser/omni.ja!/chrome/browser/search-extensions/cnrtl-tlfi-fr/manifest.json' (NS_ERROR_FILE_NOT_FOUND)(resource://gre/modules/Extension.jsm:472:18) JS Stack trace: readJSON/</<@Extension.jsm:472:18
onStopRequest@NetUtil.jsm:123:17

Expected Results

No error messages / doesn't attempt to load the removed add-ons.

It looks like these are cached in the user's extensions.json.

The error messages are repeated everytime we reload the browser, which implies that we are not handling the fact that they've been removed.

I suspect this is an add-on manager issue, but filing in search as I don't know the full flow/expectations.

Also, about:support still shows the engine(s) as being installed.

Priority: -- → P3
Duplicate of this bug: 1553414

Shane, should this really be an add-on manager responsibility to clean up here?

Flags: needinfo?(mixedpuppy)

(In reply to Mark Banner (:standard8) from comment #3)

Shane, should this really be an add-on manager responsibility to clean up here?

yes, AOM should recover from any removed extension, builtin or not. Lets see if aswan has input here.

Flags: needinfo?(mixedpuppy) → needinfo?(aswan)

(In reply to Shane Caraveo (:mixedpuppy) from comment #4)

(In reply to Mark Banner (:standard8) from comment #3)

Shane, should this really be an add-on manager responsibility to clean up here?

yes, AOM should recover from any removed extension, builtin or not. Lets see if aswan has input here.

That was not how I intended this to work. I pictured the search service being in charge of deciding what should be installed and the addon manager just handling the mechanics of loading and unloading the extensions for it. If the addon manager is also able to affect what is installed then the search service has to get notified or detect changes that it didn't initiate, and that seems prone to bugs.

To put it another way, is there a good reason not to handle this from the search service?

Flags: needinfo?(aswan)

(In reply to Andrew Swan [:aswan] from comment #5)

(In reply to Shane Caraveo (:mixedpuppy) from comment #4)

(In reply to Mark Banner (:standard8) from comment #3)

Shane, should this really be an add-on manager responsibility to clean up here?

yes, AOM should recover from any removed extension, builtin or not. Lets see if aswan has input here.

That was not how I intended this to work. I pictured the search service being in charge of deciding what should be installed and the addon manager just handling the mechanics of loading and unloading the extensions for it. If the addon manager is also able to affect what is installed then the search service has to get notified or detect changes that it didn't initiate, and that seems prone to bugs.

To put it another way, is there a good reason not to handle this from the search service?

This is about an extension being removed from the system. What happens when I install an extension, shutdown, delete it from disk, then startup? This is essentially what is happening (potentially) on an upgrade. It seems that it is left in the list of extensions to load on startup.

(In reply to Shane Caraveo (:mixedpuppy) from comment #6)

This is about an extension being removed from the system. What happens when I install an extension, shutdown, delete it from disk, then startup? This is essentially what is happening (potentially) on an upgrade. It seems that it is left in the list of extensions to load on startup.

This seems very different since the addon manager is ultimately in control over what's in the profile, there isn't some other component that's going to fall out of sync. Builtin addons, on the other hand, are controlled by some other component and if they're changing out from under it, that other component has to keep itself in sync. That's possible of course, but it sounds undesirable.

Anyway, processFileChanges() doesn't currently know how to do anything with builtin addons (which means onUpdate handles won't get called if the extension is updated for instance). If/when that gets fixed, it wouldn't be outrageous to make it delete any extensions that are no longer present, but I really think it would be better to leave this in the hands of the search service.

Severity: normal → S3
Points: --- → 3
You need to log in before you can comment on or make changes to this bug.