Closed
Bug 486195
Opened 15 years ago
Closed 15 years ago
getItemLocation on WinRegInstallLocation should return a clone of the item location
Categories
(Toolkit :: Add-ons Manager, defect)
Toolkit
Add-ons Manager
Tracking
()
RESOLVED
FIXED
mozilla1.9.2a1
People
(Reporter: mossop, Assigned: mossop)
Details
Attachments
(1 file, 3 obsolete files)
6.17 KB,
patch
|
robert.strong.bugs
:
review+
|
Details | Diff | Splinter Review |
Otherwise callers can modify the location where we think the add-on is held.
Assignee | ||
Comment 1•15 years ago
|
||
Simple fix, clone the file from the id map. We can also drop cloning in getItemFile since they both work on getItemLocation which is already a unique reference.
Attachment #370286 -
Flags: review?(robert.bugzilla)
Assignee | ||
Updated•15 years ago
|
Status: NEW → ASSIGNED
Updated•15 years ago
|
Attachment #370286 -
Flags: review?(robert.bugzilla) → review+
Assignee | ||
Comment 2•15 years ago
|
||
Pushed: http://hg.mozilla.org/mozilla-central/rev/b9659463f938
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Assignee | ||
Comment 3•15 years ago
|
||
This is a simple unit test to verify that we can't mutate the directory for a registry installed add-on. It's also the first test to actually verify registry installs which is probably helpful. It will leave the Software\Mozilla key in the registry in case the user has real add-ons installed in that way.
Attachment #373740 -
Flags: review?(robert.bugzilla)
Comment 4•15 years ago
|
||
Comment on attachment 373740 [details] [diff] [review] unit test rev 1 Removing review... Dave is going to use a mock reg implementation such as the one used in bug 484579
Attachment #373740 -
Attachment is obsolete: true
Attachment #373740 -
Flags: review?(robert.bugzilla)
Assignee | ||
Comment 5•15 years ago
|
||
Better testcase using a basic pretend registry
Attachment #374164 -
Flags: review?(robert.bugzilla)
Updated•15 years ago
|
Attachment #374164 -
Flags: review?(robert.bugzilla) → review+
Comment 6•15 years ago
|
||
Comment on attachment 374164 [details] [diff] [review] unit test rev 2 >diff --git a/toolkit/mozapps/extensions/test/unit/test_bug486195.js b/toolkit/mozapps/extensions/test/unit/test_bug486195.js >new file mode 100644 >--- /dev/null >+++ b/toolkit/mozapps/extensions/test/unit/test_bug486195.js >@@ -0,0 +1,130 @@ >... >+ * The Initial Developer of the Original Code is >+ * Dave Townsend <dtownsend@oxymoronical.com>. >+ * >+ * Portions created by the Initial Developer are Copyright (C) 2007 nit: year is wrong and I believe the name indentation is incorrect >+function run_test() >+{ >+ if (!("nsIWindowsRegKey" in Components.interfaces)) >+ return; nit: can't you use Ci here? r=me with at least the year fixed.
Assignee | ||
Comment 7•15 years ago
|
||
I've fixed the nits in the test however I've discovered that this fix causes a regression and registry installed items no longer uninstall correctly when the registry key is removed. Part of the uninstall process sets needs-uninstall in the startup cache which tried to get the item location. We need to return null if the item doesn't exist to keep the previous behaviour. Can't see a way to test that specific bit since the winreg install location caches the list and we can't restart the EM completely to remove that.
Attachment #370286 -
Attachment is obsolete: true
Attachment #374164 -
Attachment is obsolete: true
Attachment #375218 -
Flags: review?(robert.bugzilla)
Updated•15 years ago
|
Attachment #375218 -
Flags: review?(robert.bugzilla) → review+
Assignee | ||
Comment 8•15 years ago
|
||
Landed as http://hg.mozilla.org/mozilla-central/rev/edf759b1ba4a
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•