419582 - Unblocklisted plugin should be set to enable on startup

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect, P2)

Description

[Tony Chung [:tchung]]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b4pre) Gecko/2008022504 Minefield/3.0b4pre
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b4pre) Gecko/2008022504 Minefield/3.0b4pre

updating a unblocklisted plugin version from a blocklisted plugin should be set to "enable" on restart.  It currently is still enabled, and needs to be manually enabled through addons manager

** See screenshot

Reproducible: Always

Steps to Reproduce:
1. Load clean nightly trunk build.  
2. set plugin blocklist settings called out at:
https://bugzilla.mozilla.org/show_bug.cgi?id=391731#c3
3. Install blocklisted plugin  (eg. Flash 7
4. Restart browser, and go to URL
5. Verify flash7 plugin is blocked, and flash website fails to load (eg. youtube)
6. Now install over a supported plugin version that is not blocklisted (eg. Flash 8)
7. Restart browser, and verify the supported unblocklisted plugin, is set to "Disable".   User needs to go to addons Manager > Plugins Tab and Enable by hand.  This needs to be "enabled" upon restart.

Actual Results:  
Unblocklisted plugin should be Enabled upon restart automatically 

Expected Results:  
Unblocklisted plugin needs to be manually Enabled after restart.

Comment 1

[Tony Chung [:tchung]]

Attachment: Unblocklisted Plugin Disabled screenshot

Comment 2

[Dave Townsend [:mossop]]

When a plugin is marked as blocklisted it is also getting disabled, but when unblocklisted it isn't getting re-enabled. This could be as simple as not disabling at all, but I need to check through to see if we are correctly checking for blocklisted in the right places.

http://mxr.mozilla.org/seamonkey/source/modules/plugin/base/src/nsPluginHostImpl.cpp#1038

Comment 3

[Dave Townsend [:mossop]]

This is I think not totally critical, but should be a fairly easy fix that I can tackle.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Doesn't seem like a blocker. Why would we ever unblocklist a plugin? The only thing I could think of is if we accidentally blocklist something, but we'd never do that since we're perfect :)

Comment 5

[Dave Townsend [:mossop]]

(In reply to comment #4)
> Doesn't seem like a blocker. Why would we ever unblocklist a plugin? The only
> thing I could think of is if we accidentally blocklist something, but we'd
> never do that since we're perfect :)

I'm re-requesting blocking because I think you've misunderstood the problem. This is not about unblocklisting an already blocklisted plugin, it is about installing a newer safe version of a plugin on top of your old unsafe plugin. So imagine we blocklisted just Flash 8, user finds Flash 8 is blocked, installs Flash 9 and with this bug would then still need to go to tools - addons to re-enable the plugin.

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I see. Still not the end of the world, but seems like it would be very nice to have fixed before release.

Comment 7

[Dave Townsend [:mossop]]

Going to need this to fix bug 419552 without too much churn

Comment 8

[Dave Townsend [:mossop]]

Attachment: patch rev 1

The problem here is that we use 2 flags to store a plugin's state. 1 holds whether the plugin is blocklisted. The other holds whether the plugin is not blocklisted and is not disabled by the user.

The majority of this patch changes the meaning of the flag NS_PLUGIN_FLAG_ENABLED to mean "the user has not disabled this plugin". Added to nsPluginTag is a simple method IsEnabled() which tests that and the blocklist flag, the result of which is the same as the old flag used to mean. So mostly it replaces checks for the flag to checks for IsEnabled.

A couple of occasions I had to stop registering a plugin's mimetypes with the category manager because it assumed that a plugin from the cache was always enabled which is not the case any longer.

Added in is the work for bug 419552. Much of that is fixed by the above however there are some remaining issues because in a lot of places we use the enable_java pref to determine if java is enabled, and that ignores the blocklist.

So I've dropped the use of the pref in nsIJVMManager preferring instead to just query the plugin host for whether a java plugin is installed and enabled. It watches the jvm mimetype in the category manager to know when the plugin is enabled and disabled. This ends up also fixing bug 401110 as well as an unfiled bug on live connect failing when no jvm is installed.

Comment 9

[Johnny Stenback (:jst)]

Comment on attachment 309533 [details] [diff] [review]
patch rev 1

Looks good, assuming the UI pref for enabling/disabling Java still works, which it looks like it would.

Comment 10

[Dave Townsend [:mossop]]

Attachment: patch checked in

Double checked that and it does, this is the same patch apart from a little bitrotting.

Comment 11

[Dave Townsend [:mossop]]

Checking in dom/src/base/nsGlobalWindow.cpp;
/cvsroot/mozilla/dom/src/base/nsGlobalWindow.cpp,v  <--  nsGlobalWindow.cpp
new revision: 1.1003; previous revision: 1.1002
done
Checking in modules/oji/src/nsJVMManager.cpp;
/cvsroot/mozilla/modules/oji/src/nsJVMManager.cpp,v  <--  nsJVMManager.cpp
new revision: 1.83; previous revision: 1.82
done
Checking in toolkit/mozapps/extensions/content/extensions.js;
/cvsroot/mozilla/toolkit/mozapps/extensions/content/extensions.js,v  <--  extensions.js
new revision: 1.175; previous revision: 1.174
done
Checking in modules/plugin/base/src/nsPluginHostImpl.cpp;
/cvsroot/mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp,v  <--  nsPluginHostImpl.cpp
new revision: 1.603; previous revision: 1.602
done
Checking in modules/plugin/base/src/nsPluginHostImpl.h;
/cvsroot/mozilla/modules/plugin/base/src/nsPluginHostImpl.h,v  <--  nsPluginHostImpl.h
new revision: 1.113; previous revision: 1.112
done

Comment 12

[Tony Chung [:tchung]]

Verified fix on Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008042106 Minefield/3.0pre.   reinstalling an unblocked plugin is now enabled on startup.