User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040719 Firefox/0.9.1+ Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040719 Firefox/0.9.1+ If a file is removed from a newer version of an extension the EM does not remove them when the extension is uninstalled. It appears that this is due to when an extension is updated files no longer in use are not removed. Also, the Uninstall file is being written with only the files installed during for the update so any record of the previous files is lost as far as uninstall is concerned. Reproducible: Always Steps to Reproduce: 1. add a components folder with a component inside it to any extension. 2. install the extension - exit and restart. 3. install the original extension - exit and restart 4. uninstall the extension - exit and restart 5. find the extensions folder (e.g. look at the install.rdf for the guid) 6. there will be a components folder with the component inside. Actual Results: The component was still located inside the components directory for the uninstalled extension. The good news is that the extensions were not listed in either the compreg.dat or the xpti.dat. Expected Results: Removed the components directory and the root directory for the extension.

Confirming with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20040917 Firefox/0.10 Could you attach a simple testcase for this bug?

a "remove all evidence" is needed. extension preferences in prefs.js are not removed either

"Remove all evidence" should be filed as a separate bug. I'm not sure it *is* needed. This bug describes an actual bug in EM that needs to be fixed.

The testcase consists of two xpi's packaged for the new extension manager only. They both only have one install.rdf and one .txt file that I chose to put in the components directory. I did this to demonstrate that if you just install Testcase part 1 (e.g. testcase1.xpi) and uninstall it the components and the extension's (e.g. guid) directory will not be removed when uninstalling. I believe this would be a separate bug but is important to note. To test for this bug the following steps need to be performed: Install testcase1.xpi and restart. Install testcase2.xpi and restart. Uninstall the item labeled as "Testcase 2 for Bug ID 252418" in the extension manager. View the contents of the directory named {5DB6B72E-5C55-47a0-AB3B-945B4023E398} in your profile's extensions directory. There will be a chrome and a components directory. There will also be a file named testcase1.txt in the components directory. The reason for the testcase1.txt still being there is that the Uninstall file only contains the file information for the last installed xpi. Since an xpi should be a fully functional package in regards to it not requiring files from previous versions of installations it seems that the install process should remove all previous files and sub-directories and then perform the installation.

I accidentally copied the description over from the install.rdf instead of the name. The name in the Extension Manager will read Bug ID 252418 Testcase 1 or 2 depending on if testcase1.xpi or testcase2.xpi is installed at the time.

adding keyword testcase

Testcases updated to maxVersion 2.0... the old testcases maxVersion were 0.10

This patch steals existing code and with slight modifications fixes this bug by copying the Uninstall file to Uninstall.tmp. Then when an extension is installed / updated it checks for the existence of an Uninstall.tmp and if found it compares the contents of the two files. What ever exists in Uninstall.tmp that doesn't exist in Uninstall it then removes using the same process that uninstalling an extension uses. I have also successfully tested this with an update that removes a component. I tried to stay consistent with the current naming scheme and used nsInstallTmpReader but I expect a better name could be used. To test after applying the patch follow the instructions in comment #6 and use the updated testcases. Not sure who to ask for sr or if this patch would need an sr so any info in this regards would be appreciated.

Comment on attachment 176000 [details] [diff] [review] patch Ben G. wrote all this, and I only pretend to understand most of it. Bumping review to him, though I can say that "Uninstall.tmp" is a bad name for this file. Why not Uninstall.old ?

Comment on attachment 176000 [details] [diff] [review] patch I'm started to take a stab at adding support for for themes as well even though this case would be rare. I am obsoleting this patch and should have a new one soon.

Ben - Benjamin passed the review of this patch to you so I am asking for review. This basically takes parts of the existing code to make nsInstallBakReader which copies the Uninstall file if it exists to Uninstall.bak and uses that to compare against the new Uninstall file and then removes any files that existed in the previous version of an extension that don't exist in the new version. Thanks.

Sorry about the spam. Fixed a declaration... everything else still applies.

Comment on attachment 176412 [details] [diff] [review] patch My initial testing of the patch for bug 286034 shows that it will fix this bug and with the changes to nsExtensionManager.js.in this patch would no longer makes sense anyway.

Robert, should we just resolve this bug then?

Asa, I'd prefer to wait until after the checkin for bug 286034 especially with the number of changes and size of that patch. If for what ever reason it doesn't fix this bug I should be able to come up with a patch within a day or two after bug 286034 is checked in and if it does I will mark this resolved shortly after the checkin.

The checkin for bug 286034 has fixed this.