Closed Bug 635546 Opened 9 years ago Closed 9 years ago

[XP] No restart prompt after installing/updating non-restartless extensions and Java installed on the system

Categories

(Toolkit :: Add-ons Manager, defect, major)

defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla2.0b12
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: aryx, Assigned: ehsan)

References

Details

(Keywords: hang, regression, Whiteboard: [hardblocker])

Attachments

(1 file, 1 obsolete file)

Broken in Mozilla/5.0 (Windows NT 5.1; rv:2.0b12pre) Gecko/20110219 Firefox/4.0b12pre
Works with Mozilla/5.0 (Windows NT 5.1; rv:2.0b12pre) Gecko/20110218 Firefox/4.0b12pre

Installing an add-on (independent if web install or by file drag and drop) shows a doorhanger that the add-on has been installed and can be used, the button shows 'Open add-ons manager', even for normal non-bootstrapped, non-jetpack add-ons. The add-ons manager shows them as installed, but of course they are not active.
I can't reproduce that on Mac. I always get the restart notification for non-restartless add-ons. Given the above regression range we have:

PASS: http://hg.mozilla.org/mozilla-central/rev/1eed87fac9ce
FAIL: http://hg.mozilla.org/mozilla-central/rev/657c2a92ee2b

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1eed87fac9ce&tochange=657c2a92ee2b

I cannot find anything related in this list. Sebastian, can you reproduce that also in a fresh profile?
Yes, I reproduced in a fresh profile, even with trunk downloaded again. Fund now something in the error console (installing the WikiTrust extension for the first time will not show the install confirmation prompt, keeps showing the full download bar; works on second try). https://addons.mozilla.org/en-US/firefox/search/?q=wikitrust&cat=all&x=0&y=0

Error: ERROR addons.manager: Exception calling provider startup: [Exception... "Component returned failure code: 0x80520011 (NS_ERROR_FILE_NAME_TOO_LONG) [nsIFile.directoryEntries]"  nsresult: "0x80520011 (NS_ERROR_FILE_NAME_TOO_LONG)"  location: "JS frame :: resource://gre/modules/XPIProvider.jsm :: recursiveLastModifiedTime :: line 1125"  data: no]
Source File: resource://gre/modules/XPIProvider.jsm
Line: 1125

Error: ERROR addons.xpi: Error creating statement getVisibleAddonForID (SELECT internal_id, id, location, version, type, internalName, updateURL, updateKey, optionsURL, aboutURL, iconURL, icon64URL, defaultLocale, visible, active, userDisabled, appDisabled, pendingUninstall, descriptor, installDate, updateDate, applyBackgroundUpdates, bootstrap, skinnable, size, sourceURI, releaseNotesURI FROM addon WHERE visible=1 AND id=:id)
Source File: resource://gre/modules/XPIProvider.jsm
Line: 3997

Error: ERROR addons.manager: Exception calling provider getAddonByID: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIINIParserFactory.createINIParser]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/XPIProvider.jsm :: XPIDB_getActiveBundles :: line 3804"  data: no]
Source File: resource://gre/modules/XPIProvider.jsm
Line: 3804

In 20110218, the Java Quick Starter 1.0 and Microsoft .NET Framework Assistant 0.0.0 are installed by default in the new profile, in 20110219 they are missing.
Duplicate of this bug: 635627
Given the error you are seeing we fail in nsIFile.directoryEntries with NS_ERROR_FILE_NAME_TOO_LONG. Where is your profile located? Does it exceed the 255 character limit for one of your installed add-ons?
So the .net framework registers itself via the Windows Registry. Ehsan, I wonder if that has been caused by your patch on bug 634819.
Sebastian, can you please temporarily remove the registry key for the .net framework and try again if that fixes the issue?
Also for anyone who can see this issue, do you have a localized version of Windows installed?
Error: ERROR addons.manager: Exception calling provider startup: [Exception... "Component returned failure code: 0x80520011 (NS_ERROR_FILE_NAME_TOO_LONG) [nsIFile.directoryEntries]"  nsresult: "0x80520011 (NS_ERROR_FILE_NAME_TOO_LONG)"  location: "JS frame :: resource://gre/modules/XPIProvider.jsm :: recursiveLastModifiedTime :: line 1125"  data: no]
Source File: resource://gre/modules/XPIProvider.jsm
Line: 1125

as for me, this error no longer occurs after deleting sun java registry key (jqs@sun.com REG_EXPAND_SZ C:\Program Files\Java\jre6\lib\deploy\jqs\ff) from HKEY_LOCAL_MACHINE\SOFTWARE\Mozilla\Firefox\Extensions and not depends of .net key
Thanks! That would make it really a regression from bug 634819. Requesting blocking as it is a fairly intensive regression.
Blocks: 634819
blocking2.0: --- → ?
Blocks: 635615
Confirming that removing the registry key for java fixes the problem.
Duplicate of this bug: 635585
Ok, I can reproduce it now with Java installed. This is Windows XP only issue and doesn't happen on Windows Vista or 7. Seems like it's related to the older registry format which is now broken due to bug 634819.

Steps:
1. Install latest Java version on your system
2. Create a fresh profile
3. Install any non-restartless extension (i.e. adblock plus)
4. Click 'options' if available to cause a freeze of Firefox

Dave, I think step 4 is a different beast we should try to fix for the AOM. We shouldn't give the user the option to open the preferences of non-restartless add-ons as long those haven't been correctly installed.
Keywords: hang
Summary: All add-ons treated as bootstapped/restartless/jetpacks, doorhanger shows 'Open add-ons manager' instead of 'restart now' after add-on install → [XP] No restart prompt after installing non-restartless extensions and Java installed on the system
I think this should be a beta 12 blocker. CC'ing Christian.
No longer blocks: 635615
Duplicate of this bug: 635615
Similar behavior appears for updates, which doesn't let us delete the old extension files under the extensions/trash folder.
Summary: [XP] No restart prompt after installing non-restartless extensions and Java installed on the system → [XP] No restart prompt after installing/updating non-restartless extensions and Java installed on the system
Also notice the absense of the Default theme as well as other 'built-in' addons (Roboform in may case), and inability of the XPI provider to collect any manual additions/deletions to the extensions folder. It seems to just fail to read extension files anymore after the NS_ERROR_FILE_NAME_TOO_LONG on startup, and in particular chrome.manifest files which effectively leads it to think that everything installed is restartless.

Also confirming that the presence of the Java reg key is the trigger for this. My XP in non-localized.
Duplicate of this bug: 635695
Duplicate of this bug: 635655
I agree that failing to prompt to restart after installing an addon is a blocker, but I don't think this needs to block a beta. Disagree, Dave?
blocking2.0: ? → final+
Whiteboard: [hardblocker]
Please note, that this regression not only about failing to prompt to restart after installing an addon, but addons manager is completely broken when java installed and none of addons and themes can be installed or managed even with browser restart if jqs@sun.com key is in registry!
Assignee: nobody → ehsan
I think I've found the cause of the bug.  ExpandEnvironmentStringsW (used in nsWindowsRegKey::ReadStringValue) returns the length of the buffer including the terminating null character, and we fail to account for that character, so the length of the string containing "C:\Program Files\Java\jre6\lib\deploy\jqs\ff" would be returned as 45 (instead of 44).

Later on, when we try to append to this path at <http://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsLocalFileWin.cpp#1126>, we end up appending the string after the terminating null character, which means that when the string is passed to the Windows APIs, the appended part would be hidden, which makes the recursion in recursiveLastModifiedTime infinite.  Later on, we bail out here <http://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsLocalFileWin.cpp#518> when the length of the path ends up exceeding MAX_PATH.
Attached patch Patch (v1) (obsolete) — Splinter Review
Attachment #514095 - Flags: review?(jmathies)
blocking2.0: final+ → ---
OS: Windows XP → All
Whiteboard: [hardblocker] → [hardblocker][has patch][needs review jimm]
Attached patch Patch (v1)Splinter Review
With the correct patch header...
Attachment #514095 - Attachment is obsolete: true
Attachment #514095 - Flags: review?(jmathies)
Attachment #514097 - Flags: review?(jmathies)
blocking2.0: --- → final+
I'd recommend that this should block beta12, as it cripples the add-on manager for all XP users who have Java installed on their system, and we wouldn't be able to get useful feedback with regard to the add-on manager from them if we release beta12 with this bug.
blocking2.0: final+ → ?
Yep, let's hustle.
blocking2.0: ? → betaN+
Comment on attachment 514097 [details] [diff] [review]
Patch (v1)

Benjamin, if you can get to this sooner, I can land it.
Attachment #514097 - Flags: review?(benjamin)
This tryserver build works fine. Thanks Ehsan. Also why does it only happen for the registry key of Java and not for the .net framework extension?
Status: NEW → ASSIGNED
(In reply to comment #28)
> This tryserver build works fine. Thanks Ehsan. Also why does it only happen for
> the registry key of Java and not for the .net framework extension?

The type of the Java registry value is REG_EXPAND_SZ, which triggers the bug.  The .NET registry value is REG_SZ, so it doesn't trigger this bug.
Comment on attachment 514097 [details] [diff] [review]
Patch (v1)

Can we unit-test this?
Attachment #514097 - Flags: review?(benjamin) → review+
Attachment #514097 - Flags: review?(jmathies) → review+
(In reply to comment #30)
> Comment on attachment 514097 [details] [diff] [review]
> Patch (v1)
> 
> Can we unit-test this?

Sure, will write a test for it.
Flags: in-testsuite?
Whiteboard: [hardblocker][has patch][needs review jimm] → [hardblocker]
http://hg.mozilla.org/mozilla-central/rev/d14d4de010c5
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b12
(In reply to comment #31)
> (In reply to comment #30)
> > Comment on attachment 514097 [details] [diff] [review]
> > Patch (v1)
> > 
> > Can we unit-test this?
> 
> Sure, will write a test for it.

Based on a MXR search this code isn't tested at all. We should work up a suite of tests for this at some point. We could use xpc shell test so that we could read & write values using registry apis and confirm the interface is working. Care to file a follow up ehsan and cc me on it?
Verified fixed with Mozilla/5.0 (Windows NT 5.1; rv:2.0b12) Gecko/20100101 Firefox/4.0b12. Installation, upgrading, and removing of add-ons work as expected with Java installed.
Status: RESOLVED → VERIFIED
Flags: in-litmus-
Hardware: x86 → All
(In reply to comment #33)
> (In reply to comment #31)
> > (In reply to comment #30)
> > > Comment on attachment 514097 [details] [diff] [review]
> > > Patch (v1)
> > > 
> > > Can we unit-test this?
> > 
> > Sure, will write a test for it.
> 
> Based on a MXR search this code isn't tested at all. We should work up a suite
> of tests for this at some point. We could use xpc shell test so that we could
> read & write values using registry apis and confirm the interface is working.
> Care to file a follow up ehsan and cc me on it?

Filed bug 636276.
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.