293062 - Mozilla should read HKEY_CURRENT_USER\Software\MozillaPlugin

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Christian Worm Mortensen]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; da-DK; rv:1.7.7) Gecko/20050414 Firefox/1.0.3

Mozilla looks at HKEY_LOCAL_MACHINE\Software\MozillaPlugin for plugins. 
However, it should also look at HKEY_CURRENT_USER\Software\MozillaPlugin. This 
will allow users who do not have administrator privileges to install their own 
plugins. Also, this is suggested on 
http://www.mozilla.org/projects/plugins/first-install-problem.html from which 
I cite here:

"If the key cannot be created under HKEY_LOCAL_MACHINE, create it as 
HKEY_CURRENT_USER\Software\MozillaPlugins\ under HKEY_CURRENT_USER. On Windows 
XP and Windows 2000, in order to write to the registry, sometimes the software 
may need Administrator privileges. Thus, some installers may need to write to 
the HKEY_CURRENT_USER key; this doesn't require Administrator privileges. 
Traditionally, HKEY_CURRENT_USER is a copy of everything in 
HKEY_LOCAL_MACHINE."

It is the file modules/plugin/base/src/nsPluginDirServiceProvider.cpp that 
should be updated.

Finally, if a plugin is registered under both HKEY_LOCAL_MACHINE and 
HKEY_LOCAL_USER the one registered under HKEY_LOCAL_USER should take 
precendence.

Reproducible: Always




I only looked at firefox (bad me).

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

*** This bug has been marked as a duplicate of 126659 ***

Comment 2

[Christian Worm Mortensen]

This is a different issue: Bug 126659 is that changing the key 
HKEY_LOCAL_MACHINE\SOFTWARE\Mozilla\Mozilla 0.9.8+\Extensions\Plugins to some 
directory does not make Mozilla look in that directory. And as mentioned in 
the comments to the bug it is not clear that bug 126659 is even a bug.

The presnt bug is that mozilla only looks in 
HKEY_LOCAL_MACHINE\Software\MozillaPlugin for plugins while it should also 
look in HKEY_CURRENT_USER\Software\MozillaPlugin (according to 
http://www.mozilla.org/projects/plugins/first-install-problem.html).

Comment 3

[Gervase Markham [:gerv]]

This is an automated message, with ID "auto-resolve01".

This bug has had no comments for a long time. Statistically, we have found that
bug reports that have not been confirmed by a second user after three months are
highly unlikely to be the source of a fix to the code.

While your input is very important to us, our resources are limited and so we
are asking for your help in focussing our efforts. If you can still reproduce
this problem in the latest version of the product (see below for how to obtain a
copy) or, for feature requests, if it's not present in the latest version and
you still believe we should implement it, please visit the URL of this bug
(given at the top of this mail) and add a comment to that effect, giving more
reproduction information if you have it.

If it is not a problem any longer, you need take no action. If this bug is not
changed in any way in the next two weeks, it will be automatically resolved.
Thank you for your help in this matter.

The latest beta releases can be obtained from:
Firefox:     http://www.mozilla.org/projects/firefox/
Thunderbird: http://www.mozilla.org/products/thunderbird/releases/1.5beta1.html
Seamonkey:   http://www.mozilla.org/projects/seamonkey/

Comment 4

[Russell Rogers]

This BUG still seems to exist in Firefox 1.5.
Are there any plans to fix it?

Comment 5

[alex lerman]

This bug continues to exists in Firefox 2.0.

The "specification" here: http://developer.mozilla.org/en/docs/Plugins:_The_first_install_problem
implies that the HKEY_CURRENT_USER hive will be checked in addition to the HKEY_LOCAL_MACHINE. 

We can see in nsPluginDirServiceProvider.cpp::GetPLIDDirectories that only the HKEY_LOCAL_MACHINE is checked. 

I agree with the original poster that the expected operation is that the HKEY_CURRENT_USER hive should take precedence over the HKEY_LOCAL_MACHINE if a key is found in both.

Comment 6

[Ere Maijala (slow)]

Attachment: Patch v1

Here's a patch with slight additional cleanup.

Comment 7

[[e-mail]]

Comment on attachment 258931 [details] [diff] [review]
Patch v1

>-#include "nsCOMArray.h"
I was wondering about this, but then I twigged later ;-)

>+  nsresult rv = GetPLIDDirectoriesWithHKEY(HKEY_LOCAL_MACHINE, &dirs);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+  // This can fail for HKEY_CURRENT_USER if the key doesn't exist
>+  GetPLIDDirectoriesWithHKEY(HKEY_CURRENT_USER, &dirs);
Two points:
1. I think (but have no way to tell) you want to check user then machine
2. I think failure is unremarkable for either root, please check both roots

>+nsresult
>+nsPluginDirServiceProvider::GetPLIDDirectoriesWithHKEY(HKEY aKey, nsCOMArray<nsILocalFile> *aDirs)
Two points:
1. The declaration got lost from the .h file (accidentally trimmed?)
2. I'm not convinced about *aDirs, I think perhaps &aDirs is better
   (think about (*aDirs)[i])

>+#include "nsCOMArray.h"
>+
>+#if defined (XP_WIN)
>+#include <windows.h>
>+#endif
The nsCOMArray.h could be XP_WIN too.

Comment 8

[Ere Maijala (slow)]

Comment on attachment 258931 [details] [diff] [review]
Patch v1

(In reply to comment #7)
> Two points:
> 1. I think (but have no way to tell) you want to check user then machine

Why would the order be important? The result is just a list of directories.

Thanks for the comments, I'll fix the patch.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The order matters because it will affect the order in which plugins get searched, right? E.g. if there are multiple plugins that want to handle the same content type.

Comment 10

[Ere Maijala (slow)]

Attachment: Patch v1.1

Ok, here is a less tired patch with fixed order and other improvements.

Comment 11

[[e-mail]]

Comment on attachment 259524 [details] [diff] [review]
Patch v1.1

>       if (ERROR_SUCCESS == ::RegQueryValueEx(keyloc, "Path", NULL, &type,
>-                                             (LPBYTE)&path, &pathlen)) {
>+                                            (LPBYTE)&path, &pathlen)) {
>         nsCOMPtr<nsILocalFile> localFile;
>         if (NS_SUCCEEDED(NS_NewNativeLocalFile(nsDependentCString(path),
>-                                               PR_TRUE,
>-                                               getter_AddRefs(localFile))) &&
>+                                              PR_TRUE,
>+                                              getter_AddRefs(localFile))) &&
>             localFile) {
I forgot to point this out before but these indentation changes don't belong.

>+        ::RegCloseKey(keyloc);
>       }
>-      ::RegCloseKey(keyloc);
This is wrong because now a key without a Path value doesn't get closed.

r=me if you revert the above changes.

Comment 12

[Ere Maijala (slow)]

Checked in to trunk with the goofs pointed out by Neil fixed.