Extension Manager is unable to install new extensions

RESOLVED FIXED in mozilla1.8.1alpha1

Status

()

Toolkit
Add-ons Manager
RESOLVED FIXED
12 years ago
9 years ago

People

(Reporter: rstrong, Assigned: rstrong)

Tracking

({fixed1.8.0.4, fixed1.8.1})

1.8 Branch
mozilla1.8.1alpha1
fixed1.8.0.4, fixed1.8.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

In some cases the EM will leave behind files in the staged-xpis directory. Then when another install of the same addon happens it adds a new copy of the file is placed in the staging directory and even if this install succeeds the staging directory is not removed due to it already containing a file.

I have also seen .Desktop files and directories in the staging directory which also breaks new installs since the code always uses the first item it finds in the  staging directory.

Patch coming up
Created attachment 214650 [details] [diff] [review]
patch
Attachment #214650 - Flags: review?(darin)
Comment on attachment 214650 [details] [diff] [review]
patch

>-function fileIsItemPackage(fileURL) {
>-  var extension = fileURL.fileExtension.toLowerCase();
>+function fileIsItemPackage(file) {
>+  var fileURL = getURIFromFile(file);
>+  if (fileURL instanceof nsIURL)
>+    var extension = fileURL.fileExtension.toLowerCase();
>   return extension == "xpi" || extension == "jar";
> }
Changed to take a file instead of an url to simplify the call sites

>  * Return the leaf name used by the extension system for staging an item.
>  * @param   id
>  *          The GUID of the item
>  * @param   type
>@@ -812,27 +814,24 @@
>  *          A ZIP archive to read from
>  * @param   fileName 
>  *          The name of the file to read from the zip. 
>  * @param   suppressErrors
>  *          Whether or not to report errors. 
>  * @return  The file created in the temp directory.
>  */
> function extractRDFFileToTempDir(zipFile, fileName, suppressErrors) {
>-  var file = null;
>+  var file = getFile(KEY_TEMPDIR, [getRandomFileName(fileName)]);
>   try {
>     var zipReader = getZipReaderForFile(zipFile);
>     zipReader.getEntry(fileName);
>-    
>-    file = getFile(KEY_TEMPDIR, [getRandomFileName(fileName)]);
>     zipReader.extract(fileName, file);
>     zipReader.close();
>   }
>   catch (e) {
>-    // always close the zip reader even if we throw
>     if (!suppressErrors) {
>       showMessage("missingFileTitle", [], "missingFileMessage", 
>                   [BundleManager.appName, fileName]);
>       throw e;
>     }
>   }
>   return file;
> }
This would sometimes return null which made the call sites have to check if it was null and whether the file existed

>+  /**
>+   * Returns the most recently staged package (e.g. the last XPI or JAR in a
>+   * directory) for an item and removes items that do not qualify.
>+   * @param   id
>+   *          The ID of the staged package
>+   * @returns an nsIFile if the package exists otherwise null.
>+   */
>   getStageFile: function(id) {
>+    var stageFile = null;
>     var stageDir = this.location;
>     stageDir.append(DIR_STAGE);
>     stageDir.append(id);
>-    if (!stageDir.exists() || !stageDir.isDirectory())
>-      return null;
>     try {
>       var entries = stageDir.directoryEntries.QueryInterface(nsIDirectoryEnumerator);
>-      var stageFile = entries.nextFile;
>-      stageFile instanceof nsILocalFile;
>-      entries.close();
>+      while (entries.hasMoreElements()) {
>+        var file = entries.nextFile;
>+        if (!(file instanceof nsILocalFile))
>+          continue;
>+        if (file.isDirectory())
>+          removeDirRecursive(file);
>+        else if (fileIsItemPackage(file)) {
>+          if (stageFile)
>+            stageFile.remove(false);
>+          stageFile = file;
>+        }
>+        else
>+          file.remove(false);
>+      }
>     }
>     catch (e) {
>     }
>+    if (entries instanceof nsIDirectoryEnumerator)
>+      entries.close();
>     return stageFile;
>   },
This handles the cases where there are files or directories in the staging directory and also removes these items so the staging directory can be removed.

The remainder of the patch are changes to the call sites
Attachment #214650 - Attachment is obsolete: true
Attachment #214650 - Flags: review?(darin)
Created attachment 214691 [details] [diff] [review]
added exists and isDirectory back to getStageFile

I added the check for exists and isDirectory back to getStageFile... otherwise this is the same.
Attachment #214691 - Flags: review?(darin)

Comment 4

12 years ago
Comment on attachment 214691 [details] [diff] [review]
added exists and isDirectory back to getStageFile

ok, would have been nice to see a patch without whitespace changes for this one.  r=darin
Attachment #214691 - Flags: review?(darin) → review+
Thanks for the review. Checked in on trunk
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 2 alpha1
Comment on attachment 214691 [details] [diff] [review]
added exists and isDirectory back to getStageFile

a=ben@mozilla.org for the 1.8.1 branch
Attachment #214691 - Flags: approval-branch-1.8.1+
Fixed on MOZILLA_1_8_BRANCH
Keywords: fixed1.8.1
Attachment #214691 - Flags: approval1.8.0.3?

Comment 8

12 years ago
We've been seeing this bug a lot since we released the latest version of the Google Toolbar for Firefox.  It affects many of our users.  Any chance it'll go into 1.5.0.3?

Comment 9

12 years ago
Annie says that they haven't actually confirmed this bug fix.  She was just reporting that the bug is a top user complaint.  They haven't been able to reproduce the bug in their test lab.
This will be difficult to reproduce since it is contingent on a throw bug - which is probably already fixed - occurring that leaves a copy of the xpi in the staged-xpis dir. After the throw bug occurs then you could experience this bug in that the older version of the extension would then be used for installs and upgrades even though the new version has been downloaded and staged. Performing an uninstall first will usually solve this (Annie mentioned this to me via email that this has worked for solving this for the people that have reported it). Also, if multiple extensions were installed at the time the EM throws it could put all of the extensions in this state.
I forgot to mention that I have also seen .Desktop files in extensions staged dirs sent to me for Mac OS X which would also break extension installs and this patch fixes... there are also a couple of other scenarios it fixes as well.
Comment on attachment 214691 [details] [diff] [review]
added exists and isDirectory back to getStageFile

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #214691 - Flags: approval1.8.0.3? → approval1.8.0.3+
Sent testcase to Jay
Fixed on MOZILLA_1_8_0_BRANCH
Keywords: fixed1.8.0.4
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.