Last Comment Bug 293062 - Mozilla should read HKEY_CURRENT_USER\Software\MozillaPlugin
: Mozilla should read HKEY_CURRENT_USER\Software\MozillaPlugin
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: x86 Windows XP
: -- minor with 1 vote (vote)
: ---
Assigned To: Ere Maijala (slow)
:
:
Mentors:
: 397318 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-05-05 13:37 PDT by Christian Worm Mortensen
Modified: 2007-10-04 18:59 PDT (History)
6 users (show)
dveditz: wanted1.8.1.x+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (7.00 KB, patch)
2007-03-17 18:01 PDT, Ere Maijala (slow)
neil: review-
Details | Diff | Splinter Review
Patch v1.1 (5.11 KB, patch)
2007-03-24 10:44 PDT, Ere Maijala (slow)
neil: review+
roc: superreview+
Details | Diff | Splinter Review

Description Christian Worm Mortensen 2005-05-05 13:37:24 PDT
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] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) 2005-05-17 05:00:46 PDT

*** This bug has been marked as a duplicate of 126659 ***
Comment 2 Christian Worm Mortensen 2005-05-17 10:30:21 PDT
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] 2005-09-27 02:04:27 PDT
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 2006-07-19 01:44:08 PDT
This BUG still seems to exist in Firefox 1.5.
Are there any plans to fix it?
Comment 5 alex lerman 2007-03-12 14:24:57 PDT
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) 2007-03-17 18:01:27 PDT
Created attachment 258931 [details] [diff] [review]
Patch v1

Here's a patch with slight additional cleanup.
Comment 7 neil@parkwaycc.co.uk 2007-03-19 10:51:54 PDT
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) 2007-03-19 12:28:56 PDT
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) 2007-03-19 19:26:39 PDT
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) 2007-03-24 10:44:39 PDT
Created attachment 259524 [details] [diff] [review]
Patch v1.1

Ok, here is a less tired patch with fixed order and other improvements.
Comment 11 neil@parkwaycc.co.uk 2007-03-25 04:47:02 PDT
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) 2007-04-03 08:57:15 PDT
Checked in to trunk with the goofs pointed out by Neil fixed.
Comment 13 Benjamin C. Wiley Sittler 2007-10-04 16:24:51 PDT
*** Bug 397318 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.