Firefox upgrade incorrectly reset all disabled plugin to enabled state

RESOLVED FIXED in mozilla1.9.1b2

Status

()

defect
P1
normal
RESOLVED FIXED
11 years ago
6 years ago

People

(Reporter: matp75zilla, Assigned: mossop)

Tracking

Trunk
mozilla1.9.1b2
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 -
wanted1.9.1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 2 obsolete attachments)

Reporter

Description

11 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20080913031911 Minefield/3.1b1pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20080913031911 Minefield/3.1b1pre

When upgrading from Firefox 3.0.x to Firefox 3.x (nightly build of Firefox 3.1), all my disabled plugin get reenabled.
Even after going back to Firefox 3.0.x, there are still disabled.

Using Firefox 3.0.1
Minefield (Fx3.1) (20080913) is installed in another directory.
To reproduce : 
In Firefox 3.0.1, disable some plugins in the plugin manager
You can restart Firefox 3.0 and it will be persistent.

Exit Firefox and launch Firefox 3.1 with the same profile.
Go to add on manager
All plugins have been reenabled (and one (Activtouch General Plugin container)  is missing, see screenshots following)
Expected result : the disabled plugins should stay disabled.

Exit Firefox 3.1
Launch Firefox 3.0
Go to add-on manager 
All plugins have been reenabled.
Expected result : the disabled plugins should have stayed disabled.

Following are 3 screenshots : 
- addon manager : Fx 3.0 with disabled plugins
- addon manager Fx 3.1 
- addon manager Fx 3.0 after having launched Fx 3.1



Reproducible: Always
Reporter

Updated

11 years ago
Component: Extension Compatibility → Plug-ins
Product: Firefox → Core
Version: unspecified → Trunk
Reporter

Comment 3

11 years ago
Assignee

Comment 4

11 years ago
Your third screenshot, from Firefox 3.0 shows the plugins disabled again, but your description suggests that that is not the case, which is correct?
Reporter

Comment 5

11 years ago
Sorry, I had selected the wrong screenshot.
Attachment #338511 - Attachment is obsolete: true
Reporter

Comment 6

11 years ago
Additional precisions : this has been occuring from some time each time I try a nigthly build.
At first I was not sure of the event which reenabled the plugins because I'm not always having a look at the plugin manager window (could have been the plugins themselves)
Assignee

Comment 7

11 years ago
Are you running the two different versions with different profiles at all?
Reporter

Comment 8

11 years ago
No, this is done with the same profile. (ie I guess at some time people will be able to upgrade without recreating a profile)

I did some more tests :

Fx 3.1 : 
- Launch with -P
- create a new profile
- go to plugin manager
- disable the same plugins
- exit firefox 3.1
- relaunch nigthly 3.1
- go to plugin manager
expected result : 
- plugin disabled
observed behaviour : 
- plugins disabled 


Fx 3.0 : 
- launch Fx 3.0.1 with -P
- create a new profile
- go to plugin manager
- disable the same plugins
- exit firefox 3.0
- launch nigthly 3.1
- go to plugin manager
expected result : 
- plugin disabled
observed behaviour : 
- plugins reenabled.
(ie this is the same as with my more complex profile)

with the same new profile : 
- continue and disable plugin (ie running Fx 3.1)
- restart Firefox (I exit and relaunch the app with the same profile)
- go to plugin manager
expected result :
- plugins disabled
observed behaviour
- plugins disabled

So the problem occurs only when upgrading.
I don't known if it's a global behaviour or if there's something special with my plugin list.
Assignee

Comment 9

11 years ago
So the way things are with the plugin registry, any time we change its format to include new data (as happened in bug 427743) we will throw away the cached information and rebuild from scratch. This means throwing away the disabled flag (technically the blocklisted flag too but that gets reset correctly anyway).

It might be possible to add the file version from bug 427743 in a manner that is compatible with both Firefox 3 and 3.1, otherwise we'd have to figure out some way to parse out the required information from old forms of the cache and rebuild the rest.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.9.1?
OS: Windows XP → All
Hardware: PC → All
Mossop, giving this to you to figure out whether we need to add support for parsing old format plugin info files or just mark this one wontfix.
Assignee: nobody → dtownsend
Flags: blocking1.9.1? → blocking1.9.1+
Not blocking on this after all, but Mossop, please do investigate this!
Flags: wanted1.9.1+
Flags: blocking1.9.1-
Flags: blocking1.9.1+
Priority: -- → P1
Posted patch patch rev 1 (obsolete) — Splinter Review
It seems that the pluginreg format is not great for forwards/backwards compatibility really. In this case we can make it work fairly easily for the upgrade case.

This defines a minimum version of the registry we can read and will attempt to parse anything between that and the current version. In the event that the registry is not the current version it makes the modified time of a plugin as -1. This will mean that in the directory scan that follows ReadPluginInfo the plugin tag will be recreated from the real plugin library, carrying across the disabled flags, so correctly getting the missing version.

Included is a simple unit test, it writes out a 0.9 version registry for the test plugin then brings up the plugin host. This will perform the load and scan and we can test that the plugin is correctly disabled and has the version added.

If we wanted to fix the downgrade case then we'd need a branch patch, which wouldn't be too difficult, but I'm not sure how necessary that is.

If changes to the plugin registry are going to be more frequent we might have to think about using a file format that is better at handling missing/extra data fields. INI, JSON, XML are all capable of this more than what we have.
Attachment #340953 - Flags: review?(jst)
Assignee

Updated

11 years ago
Status: NEW → ASSIGNED
Comment on attachment 340953 [details] [diff] [review]
patch rev 1

Looks good to me, Josh should look this over as well.
Attachment #340953 - Flags: superreview+
Attachment #340953 - Flags: review?(jst)
Attachment #340953 - Flags: review?(joshmoz)

Updated

11 years ago
Attachment #340953 - Flags: review?(joshmoz) → review+
Landed: http://hg.mozilla.org/mozilla-central/rev/62274f98ccf8
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
Backed out due to test failures
Status: RESOLVED → REOPENED
Flags: in-testsuite+
Resolution: FIXED → ---
(In reply to comment #15)
> Backed out due to test failures

Just to note for reference that this patch also seemed to cause mailnews to reload plugins every time it changed message (at least on Linux & Mac, Windows may also be a problem). I haven't looked into the details as to why this happened, let me know if there is anything specific you need me to look at.
(In reply to comment #16)
> (In reply to comment #15)
> > Backed out due to test failures
> 
> Just to note for reference that this patch also seemed to cause mailnews to
> reload plugins every time it changed message (at least on Linux & Mac, Windows
> may also be a problem). I haven't looked into the details as to why this
> happened, let me know if there is anything specific you need me to look at.

Yeah that is the reason for the majority of the test failures.
Posted patch patch rev 2Splinter Review
Some sloppy coding on my part caused the problem.

Firstly the unit test only tried to find the OSX plugin library, something I forgot to correct after. I've just added the search for nptest.dll and libnptest.so to get_test_plugin in the unit test.

Secondly the ReadPluginInfo method was failing. I needed to cache the fact that the registry supports the version field since values[1] gets overwritten at a later point and caused us to read essentially corrupt data.

Only a couple of very minor changes here anyway, basically the same stuff but working better
Attachment #340953 - Attachment is obsolete: true
Attachment #342568 - Flags: superreview?(jst)
Attachment #342568 - Flags: review?(joshmoz)
Posted patch patch diffSplinter Review
Maybe helpful, this is a diff between what the old patch gave and what the new patch gives.

Updated

11 years ago
Attachment #342568 - Flags: review?(joshmoz) → review+
Assignee

Updated

11 years ago
Status: REOPENED → ASSIGNED
Attachment #342568 - Flags: superreview?(jst) → superreview+
Pushed: http://hg.mozilla.org/mozilla-central/rev/d0132a133f2d
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago11 years ago
Resolution: --- → FIXED
Assignee

Updated

11 years ago
Flags: in-testsuite+
Backed out due to test failures
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Relanded: http://hg.mozilla.org/mozilla-central/rev/f43b09ee4e1b

Not sure how I missed this but the plugin registry uses different field delimiters on windows so the file the testcase was writing out was wrong. As I've just tweaked the test I didn't see a need for a re-review.
Status: REOPENED → RESOLVED
Last Resolved: 11 years ago11 years ago
Resolution: --- → FIXED
Duplicate of this bug: 497966
You need to log in before you can comment on or make changes to this bug.