bug 299040 set the updateURL for reporter to about:blank which generates an error when checking for updates on reporter. Instead of using about:blank I suggest using a more descriptive value (e.g. none). Patch coming up.
Don't check for updates when updateURL="none" Besides reporter, I also set updateURL="none" for all themes using toolkit. Other extensions that should not be checked for updates can be decided in bug 299040 and handled in this manner.
BTW: if you prefer this could also be handled by setting a pref for each extension shipped instead of setting an updateURL value to be checked: pref("extensions.%UUID%.update.enabled",false); where %UUID% is the extension's ID.
I am ok with either method but I think I prefer using the pref method since it centralizes management of this based on the app. If the extension is installed separately from the app the extension can specify the pref in its default prefs to enable updates if it is desired.
I think using the existing method of using the pref is cleaner. I also disable "Update" in the EM's context menu when the pref to enable updates for an extension is set to false. This is only for Firefox and I'll submit separate patches to disable update for Thunderbird's and Sunbird's default themes if this is the way to go for this. BTW: I also fixed one error that is thrown when bringing up the EM's context menu when nothing is selected.
This is probably a stupid question but how about the appManaged arc, could that be used to indicate that no attempts to update should be made?
Not a stupid question at all... I hadn't noticed that Ben recently added that and it looks like he may have done so for siimilar reasons as needed here. Instead of adding an updateURL of about:blank as bug 299040 did we should just leave it empty, use the appManaged property, and add the appManaged property to the default themes. I still think it would be a good thing to control the command state of cmd_update in a similar fashion as the patch does except it should also include checking the appManaged property. Good catch and thanks
Comment on attachment 188499 [details] [diff] [review] better patch Benjamin - I am going to resubmit using the appManaged property Ben recently added. Are you ok with reading the pref as this patch does as well as the appManaged property for controlling the command state for update so the correct widget state can be shown to the user?
You want to check for both updateURL="none" and appManaged="true" ? That's ok, I supposed.
Actually, I am trying to avoid special casing updateURL="anything" for disabling updates. I would prefer not adding default pref values to disable updates for appManaged items since that would apply to all installs of the item. We only have an nsIUpdateItem at the point in the code where we need to check for appManaged="true". As I see it this needs a cleaner fix than what I have come up with so far.
This fixes several problems with extension updates. The one it doesn't is not checking for updates to items that we don't have write access to (currently this is checked in the EM ui for individual items but not elsewhere) and I would prefer to deal with that in bug 300265. There are multiple criteria involved in deciding whether or not an extension - or theme for that matter - can be updated. If any of the following is true updates will not be performed on the item. opType == OP_NEEDS_INSTALL opType == OP_NEEDS_UNINSTALL opType == OP_NEEDS_UPGRADE appManaged == "true" extension's update.enabled pref value == false I need to go through the code at least one more time but reviews are welcome at this time. I want to make sure this is a good direction to go in under these circumstances.
This should cover all of the conditions when an update check shouldn't happen for an item and properly set the command state for update of individual items in the managers.
Whiteboard: [checkin needed][a+]
Summary: Error when checking for reporter updates - fallout from bug 299040 → Only check for updates for items that are uupdateable (error checking for reporter updates - bug 299040 fallout)
Summary: Only check for updates for items that are uupdateable (error checking for reporter updates - bug 299040 fallout) → Only check for updates for items that are updateable (error checking for reporter updates - bug 299040 fallout)
Checking in content/extensions.js; /cvsroot/mozilla/toolkit/mozapps/extensions/content/extensions.js,v <-- extensions.js new revision: 1.64; previous revision: 1.63 done Checking in content/extensions.xul; /cvsroot/mozilla/toolkit/mozapps/extensions/content/extensions.xul,v <-- extensions.xul new revision: 1.32; previous revision: 1.31 done Checking in content/update.js; /cvsroot/mozilla/toolkit/mozapps/extensions/content/update.js,v <-- update.js new revision: 1.13; previous revision: 1.12 done Checking in public/nsIExtensionManager.idl; /cvsroot/mozilla/toolkit/mozapps/extensions/public/nsIExtensionManager.idl,v <-- nsIExtensionManager.idl new revision: 1.36; previous revision: 1.35 done Checking in src/nsExtensionManager.js.in; /cvsroot/mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in,v <-- nsExtensionManager.js.in new revision: 1.124; previous revision: 1.123 done
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox1.1
*** Bug 300342 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.