Closed Bug 284515 Opened 16 years ago Closed 16 years ago

Viewing the options of an extension that has just been enabled makes extension manager unusable.

Categories

(Toolkit :: Startup and Profile System, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.8beta2

People

(Reporter: mossop, Assigned: robert.strong.bugs)

References

Details

(Keywords: regression)

Attachments

(2 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050302 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050302 Firefox/1.0+

If you enable an extension then try to view it's options before restarting,
nothing displays and the extension manager becomes unusable.

Reproducible: Always

Steps to Reproduce:
1. Enable a disabled extension.
2. Right click and select options.

Actual Results:  
Nothing displays and you hear a beep when you click anywhere on the extension
manager.


Expected Results:  
The options entry on the menu should not be available until after firefox has
restarted.
I agree that would make it unusable if it was possible click Options. For me
however, Options is disabled until I restart.

WFM Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050306
Firefox/1.0+
Version: unspecified → Trunk
Confirming with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2)
Gecko/20050310 Firefox/1.0+ 

I believe this is due to the checkin for bug 278534 in that the enable disable
functionality in nsExtensionManager.js.in was changed / removed. Also, bug
285544 though different is similarly affected by the checkin.
Status: UNCONFIRMED → NEW
Ever confirmed: true
This just happened to me in Thunderbird 3/9/05 also.
Robert, would you be willing to take a look at this? I certainly didn't mean to
regress this functionality. If you can't, reassign it back to me.
Assignee: bugs → moz_bugzilla
Component: Extension/Theme Manager → XRE Startup
Keywords: regression
Product: Firefox → Toolkit
QA Contact: bugs → benjamin
Target Milestone: --- → mozilla1.8beta2
Version: Trunk → unspecified
Not a problem Benjamin, I'll see if I can figure it out.
It seems that the changes that will be introduced by bug 286034 are going to
change the flow and probably process for this to the point that any patch for
this would require a rewrite after bug 286034 is checked in if Ben doesn't take
this bug into account with that patch.
Attached patch patch (obsolete) — Splinter Review
This is a simple one liner. I've tested it with trunk and with the latest patch
from bug 286034.
Attachment #178657 - Flags: first-review?(benjamin)
Though I think the behavior with this patch applied is acceptable I believe it
is different that the previous behavior. With this patch if toBeEnabled ==
"true" the  options for the extension are not available and if an extension is
set to be disabled, then re-enabled the options for the extension will not be
available until after restart. Previously, I believe that if an extension was
enabled, then set to be disabled, then re-enabled the options would be available
without a restart. I don't believe it would be worth the additional code to
provide the previous functionality considering that the scenario is rather rare
and can be easily resolved by restarting.
Flags: blocking-aviary1.1?
Comment on attachment 178657 [details] [diff] [review]
patch

Clearing review and obsoleting. The patch for bug 286034 appears to have
removed the code that set toBeEnabled along with the other toBe's though they
still exist in extensions.xul (probably a cleanup oversight). Another approach
will have to be taken.
Attachment #178657 - Attachment is obsolete: true
Attachment #178657 - Flags: first-review?(benjamin)
Flags: blocking-aviary1.1? → blocking-aviary1.1+
Depends on: 292506
Attached patch patch (obsolete) — Splinter Review
Attachment #183814 - Flags: first-review?(benjamin)
In addition to the options issue this patch also does the following:
* displays the generic about when an item is disabled.
* updates the minVersion and maxVersion of an install.rdf during
_applyVersionUpdates. The case during install still needs to be addressed.
* adds ability to cancel an uninstall by selecting enable on an extension that
is to be uninstalled.
* when enabling then disabling an extension or vice versa an operation will
always be performed even if the extension is already in that state.
* extensions only show as disabled in the EM when they are disabled.
* a couple of JavaScript strict warnings in extensions.js
Comment on attachment 183814 [details] [diff] [review]
patch

I don't understand this block:
+      // apply changes to the install manifest if it is already installed and
+      // we can write to the location
+      var installLocation = this._em.getInstallLocation(aLocalItem.id);
+      var installManifestFile = installLocation.getItemFile(aLocalItem.id,
+							
FILE_INSTALL_MANIFEST);
+      if (installManifestFile.exists() && installLocation.canAccess) {
+	 this._emDS.setTargetApplicationInfo(aLocalItem.id,
+					     aRemoteItem.minAppVersion,
+					     aRemoteItem.maxAppVersion,
+					    
getInstallManifest(installManifestFile));
+      }

Could you explain this?
When an extension has its minVersion and maxVersion changed during an update
check only the extensions.rdf is updated. If for whatever reason the
extensions.rdf no longer exists then the install.rdf is read to populate it and
last I checked it will also uninstall the extension without any user
interaction. This will update the minVersion and maxVersion in the extension's
install.rdf with the updated info to prevent this in one of the cases.

This only handles the situation when the extension is already installed and not
during the install which I have been working on as well. I can remove this if
you'd prefer and add it to the other patch I am working on.
Attached patch patchSplinter Review
I removed it from the patch and will try to come up with a patch for the other
cases in another bug.
Attachment #183814 - Attachment is obsolete: true
Attachment #183877 - Flags: first-review?(benjamin)
Attachment #183814 - Flags: first-review?(benjamin)
No longer depends on: 292506
Comment on attachment 183877 [details] [diff] [review]
patch

This is a little late in the game for Firefox 1.1a1: let's hold off until after
that release and land this for 1.1a2.
Attachment #183877 - Flags: first-review?(benjamin)
Attachment #183877 - Flags: first-review+
Attachment #183877 - Flags: approval1.8b3?
Status: NEW → ASSIGNED
Blocks: 279634
Attached patch Updated patchSplinter Review
This adds a fix one ui glitch when cancelling an uninstall in that the buttons
along the bottom were not updated to reflect that it could then be uninstalled
or updated. It also adds EM_ITEM_CANCEL to nsIExtensionManager.idl which I
overlooked since they aren't used anywhere as of yet.
Attachment #184984 - Flags: first-review?(benjamin)
Comment on attachment 183877 [details] [diff] [review]
patch

a=shaver
Attachment #183877 - Flags: approval1.8b3? → approval1.8b3+
Attachment #184984 - Flags: first-review?(benjamin)
Attachment #184984 - Flags: first-review+
Attachment #184984 - Flags: approval-aviary1.1a2+
Comment on attachment 184984 [details] [diff] [review]
Updated patch

Checking in toolkit/mozapps/extensions/content/extensions.js;
/cvsroot/mozilla/toolkit/mozapps/extensions/content/extensions.js,v  <-- 
extensions.js
new revision: 1.55; previous revision: 1.54
done
Checking in toolkit/mozapps/extensions/content/extensions.xul;
/cvsroot/mozilla/toolkit/mozapps/extensions/content/extensions.xul,v  <-- 
extensions.xul
new revision: 1.27; previous revision: 1.26
done
Checking in toolkit/mozapps/extensions/public/nsIExtensionManager.idl;
/cvsroot/mozilla/toolkit/mozapps/extensions/public/nsIExtensionManager.idl,v 
<--  nsIExtensionManager.idl
new revision: 1.32; previous revision: 1.31
done
Checking in toolkit/mozapps/extensions/src/nsExtensionManager.js.in;
/cvsroot/mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in,v  <--
 nsExtensionManager.js.in
new revision: 1.110; previous revision: 1.109
done
fixed on trunk
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
*** Bug 296773 has been marked as a duplicate of this bug. ***
Verified with Deer Park builds:
Windows 2005-07-01-06-trunk
Mac 2005-07-01-07-trunk
Status: RESOLVED → VERIFIED
Flags: in-testsuite?
Component: XRE Startup → Startup and Profile System
QA Contact: benjamin → startup
You need to log in before you can comment on or make changes to this bug.