Unable to uninstall or upgrade extensions due to existing extension directories having the read only attribute set

RESOLVED FIXED

Status

()

RESOLVED FIXED
13 years ago
10 years ago

People

(Reporter: rstrong, Assigned: rstrong)

Tracking

(Blocks: 1 bug, {fixed1.8.0.4, fixed1.8.1})

1.8.0 Branch
x86
Windows XP
fixed1.8.0.4, fixed1.8.1
Points:
---
Bug Flags:
blocking1.8.0.4 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Split off from bug 316808

nsIFile remove respects read only attributes for files that it manages. Since under some circumstances on Win32 this can occur (see bug 316808 comment #21 - restoring from a cd backup comes to mind) and leave the extensions in an unusable state we should reset this when we have the permissions to do so.

To reproduce you have to use attrib +R on an extension's directories and then the nsIFile.remove will not remove the files even though you can do so with explorer. Setting the read only attribute on a directory using the ui (e.g. slecting properties of the directory and checking read only) will not cause this problem because the ui will only set the read only attribute on the files within the directory.

This did not affect 1.0 as noticeabley since it would successfully remove the extension's metadata (e.g. the extension was removed from the EM's ui) but it wouldn't remove the extension's directory when uninstalling. Since we now have the ability to install an extension in 1.5 by placing an extension's directory in the extensions directory this issue is compounded by these partially uninstalled extensions are being re-installed and showing up in the EM ui.

Also of note is bug 223771 which can be reproduced by setting the read only attribute using attrib +R.
bah - "nsIFile remove respects read only attributes for files that it manages."
should read
nsIFile remove respects read only attributes for files and the extension manager respects that for files that it manages.
Created attachment 206730 [details] [diff] [review]
fallback to recursion / setting permissions
Created attachment 206752 [details] [diff] [review]
fallback to recursive setting perms / delete and prevent duplicate pending ops

This should be enough for this bug but since Benjamin is on vacation until the 3rd I'm going to try to find some time to spend with this code before requesting review. Comments welcome
Attachment #206730 - Attachment is obsolete: true
Attachment #206752 - Attachment is obsolete: true

Comment 4

13 years ago
For those people which like to fix this bug temporarily on their PC, the correct cmd to remove the readonly attribute from the profile folder and all subfolders is

c:\Documents And Settings\...\Profiles\XXXXXXX.default\> attrib -R /S /D

This includes all files and folders in the current directory recursively as well as the folders themselves.
Created attachment 207453 [details] [diff] [review]
patch

Adds a helper function named removeDirRecursive that will attempt to recursively set the permissions on each directory entry and then delete it if remove(true) fails. This patch is focused entirely on this one issue except for one minor change from using Components.interfaces.nsIFile.DIRECTORY_TYPE to nsILocalFile.DIRECTORY_TYPE for consistency with the rest of the file. The remaining issues I'll address in the other bugs with the same symptoms but different causes.
Attachment #207453 - Flags: review?(benjamin)
Note: besides bug 223771 which can be reproduced by setting the read only attribute on bookmarks.html I just checked that trying to delete a profile that has a read only attribute on its directory for the most part succeeds though it leaves the directory behind. Using explorer to delete the directory succeeds and does not warn about it being read only on XP. On a Win95 system using a copy of a profile restored from a CD none of the files in the profile were deleted but when deleting with explorer a warning is displayed though it also succeeds after confirming.

I am quite sure that XP no longer has the issue where directories / files copied from a CD have the read only attribute set but it wouldn't surprise me if there were still backup restore programs around that set this attribute while Win2k and below still have this issue. I haven't checked the behavior of a restore point yet.

Perhaps we should work out an overall solution since this affects multiple areas?

Updated

13 years ago
Attachment #207453 - Flags: review?(benjamin) → review+
Fixed on trunk.

Note: there are a couple of more bugs that have the same general symptom of unable to install / upgrade extensions with the general sign in the ui of the extension having the must restart to install message. There are actually different root causes for these errors and this only addresses one of them. After I have addressed the others and this has baked on the trunk for a bit I'll request approval for 2.0 for this patch.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Attachment #207453 - Flags: approval1.8.1?
Attachment #207453 - Flags: approval1.8.1?
Self approving for 1.8.1 per
http://developer.mozilla.org/devnews/index.php/2006/01/30/tree-rules-for-the-18-branch/

Fixed on MOZILLA_1_8_BRANCH
Keywords: fixed1.8.1
*** Bug 325149 has been marked as a duplicate of this bug. ***
Attachment #207453 - Flags: branch-1.8.1+
*** Bug 305620 has been marked as a duplicate of this bug. ***
Comment on attachment 207453 [details] [diff] [review]
patch

Requesting 1.8.0.2 - this adds a fallback method for removing files and directories where we set the appropriate permissions before we try to remove the item. I have exchanged emails with at least 3 people that have experienced this bug and have seen a couple of reports in the forums as well.
Attachment #207453 - Flags: approval1.8.0.2?

Updated

13 years ago
Flags: blocking1.8.0.3?
Comment on attachment 207453 [details] [diff] [review]
patch

as per discussion in triage meeting 23-feb-2006
Attachment #207453 - Flags: approval1.8.0.3?
Attachment #207453 - Flags: approval1.8.0.2?
Attachment #207453 - Flags: approval1.8.0.2-
Flags: blocking1.8.0.3? → blocking1.8.0.3+
Comment on attachment 207453 [details] [diff] [review]
patch

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #207453 - Flags: approval1.8.0.3? → approval1.8.0.3+
Checked in to MOZILLA_1_8_0_BRANCH

Checking in mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in;
/cvsroot/mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in,v  <--  nsExtensionManager.js.in
new revision: 1.144.2.16.2.7; previous revision: 1.144.2.16.2.6
Keywords: fixed1.8.0.3
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.