491125 - Searchplugin disappears permanently after uninstalling extension with the same named plugin bundled

Status: VERIFIED FIXED

(Firefox :: Search, defect, P1)

Description

[Max]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; ru; rv:1.9.0.10) Gecko/2009042700 SUSE/3.0.10-2.1 Firefox/3.0.10
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; ru; rv:1.9.1b4) Gecko/20090423 Firefox/3.5b4

This only works for Firefox 3.5b4 (and probably previous betas too). If you install an extension, which provides a search plugin that is already present by default, and then disable or uninstall that extension, the search plugin disappears from the search bar.

Reproducible: Always

Steps to Reproduce:
1.Install a testing extension that provides a search plugin already present by default (<ShortName> must be the same)
2.Disable or uninstall this extension. Then restart browser of course.

Actual Results:  
Preinstalled search plugin is gone from the quick search bar.

Expected Results:  
Browser should have restored the original search plugin, located in the searchplugins subdirectory of the installation path.

Works on linux and windows builds (and, I believe, on mac too).

Comment 1

[Max]

Attachment: A testing extension

A testing extension which provides a Yandex quicksearch plugin replacing the preinstalled one.

Comment 2

[Max]

Attached extension should be tested on builds with Yandex search plugin preinstalled (russian for example).

Comment 3

[Danil Ivanov]

(In reply to comment #0)
> .....
> Works on linux and windows builds (and, I believe, on mac too).

Mac OS: confirmed

Comment 4

[Alexander L. Slovesnik]

Confirming, based on comments, asking for blocking Firefox 3.5. This bug affects at least users of Yandex.Bar extension.

Comment 5

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

Haven't had a chance to look into this yet, but this might be a regression from bug 394979. I suspect we're only including the addon engine in the JSON cache when it's installed, and then not invalidating the app cache when it's uninstalled. Perhaps we should invalidate all caches when any of them change...

Comment 6

[Max]

2 Gavin: exactly, this relates bug 394979. Setting the preference "browser.search.cache.enabled" to false fixes this issue.

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Ryan: you can haz blocker!

Comment 8

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Attachment: WIP

Comment 9

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Attachment: Patch

Comment 10

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Attachment: Branch patch

Comment 11

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Attachment: Patch v2

Comment 12

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

Comment on attachment 378790 [details] [diff] [review]
Patch v2

-modifiedDir wants || instead of &&
-remove "locale" temporary, move buildID to above rebuildCache definition
-if you're going to keep cacheEnabled, you might as well check it before calling _buildCache to avoid a pref check
-"let dir" instead of "let file" makes the diff smaller :)
-why |for each| for _loadEnginesFromCache() but forEach() for _loadEnginesFromDir?

Comment 13

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Attachment: Patch v2.1

(In reply to comment #12)
> (From update of attachment 378790 [details] [diff] [review])
> -modifiedDir wants || instead of &&
done

> -remove "locale" temporary, move buildID to above rebuildCache definition
done

> -if you're going to keep cacheEnabled, you might as well check it before
> calling _buildCache to avoid a pref check
done

> -"let dir" instead of "let file" makes the diff smaller :)
done

> -why |for each| for _loadEnginesFromCache() but forEach() for
> _loadEnginesFromDir?
cache.directories is an object, loadDirs is an array.

Comment 14

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Attachment: Patch v2.2

Comment 15

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

Comment on attachment 378805 [details] [diff] [review]
Patch v2.2

s/aCacheDir/aCachePath/ ?

Could use some more comments to explain what's going on, but we can do that in a followup.

Comment 16

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

Attachment: IRC transcript of patch discussion

Comment 17

[Ryan Flint [:rflint] (ping via IRC for reviews)]

http://hg.mozilla.org/mozilla-central/rev/c832d8634f23
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f9fdf276d414

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified on trunk and 1.9.1 with builds on Windows:

Mozilla/5.0 (Windows; U; Windows NT 5.1; ru; rv:1.9.2a1pre) Gecko/20090525 Minefield/3.6a1pre

Mozilla/5.0 (Windows; U; Windows NT 5.1; ru; rv:1.9.1pre) Gecko/20090525 Shiretoko/3.5pre