Closed Bug 298497 Opened 19 years ago Closed 18 years ago

Extension Dependencies

Categories

(Toolkit :: Add-ons Manager, defect)

Product:
Toolkit
Component:
Add-ons Manager
Version:
1.8.0 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.8.1alpha1

People

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

References

(
URL
)

Details

(Keywords: dev-doc-complete, fixed1.8.1)

Attachments

(4 files, 2 obsolete files)

patch
61.91 KB, patch
benjamin
: approval1.8b4-
Details | Diff | Splinter Review
screenshot of various parts of the ui
29.58 KB, image/png
Details
use nsIExtensionManager2
62.07 KB, patch
benjamin
: review+
Details | Diff | Splinter Review
checked in patch (fixed an incorrect comment)
62.07 KB, patch
Details | Diff | Splinter Review 
Extensions should be able to depend on other extensions. For 1.8, this can
simply be the EM refusing to enable an extension until another extension is
installed+enabled. In 1.9 timeframe it would be good to ask UMO to help resolve
extension dependencies for semi-automatic download and install.

For bonus points, when an extension specifies a dependency it should be able to
provide a download link + extension title, not just the extension ID.
Whoops ;-).

See also bug 298498.
Assignee: benjamin → rob_strong
QA Contact: nobody → benjamin
Rough draft at
http://wiki.mozilla.org/User_talk:Robert_Strong

I'll add it after a once over.
Hm, does this supersede bug 293017?  And shouldn't this be in the Extension
Manager component?
*** Bug 293017 has been marked as a duplicate of this bug. ***
Depends on: 264750
Whiteboard: [affects l10n][ETA 8/6]
Target Milestone: --- → mozilla1.8beta4
Attached patch patch (obsolete) — — Splinter Review 
This covers the basics. It refuses to enable an extension that doesn't have its
dependencies satisfied (e.g. dependency not installed, not enabled, not
compatible per the em:requires minVersion and maxVersion). If an extension is
to be disabled or uninstalled it displays a list of the extensions that depend
on it - if any - and informs the user that these extensions will also be
disabled... in the case of disabling it only displays a list of dependent
extensions that are enabled.

There is still a lot of functionality to be added but I believe this should be
good enough for 1.8. Also, there is additional work that needs to be done in
regards to setting the correct pending operation for proper display in the ui
when an operation is canceled.
Attachment #191959 - Flags: first-review?(benjamin)
Comment on attachment 191959 [details] [diff] [review]
patch

-  <tree rows="10" hidecolumnpicker="true">
+  <tree id="addonsTree" rows="10" hidecolumnpicker="true" hidden="true">

Why?

+requiresFormat=%S %S

Why? Can't you just format the string in JS (var + " " + var2)?

+needsDependenciesDisabled=Disabled - requires additional items to operate.

How does the user figure out what "additional items" are? Does the extension
"about" page list the dependencies?

disableWarningDependMessage=If you disable %S, the following items that require
%S will also be disabled.

What does this UI look like? It seems that this should probably end with a
colon, not a period.
> What does this UI look like? It seems that this should probably end with a
> colon, not a period.
Will do.

The reason the tree is hidden is to allow using the same dialog for uninstalls
whether the extension has dependent items or not. If it does then the tree lists
these items.
Uninstalling an item with dependencies:
+-----------------------------------------------------------+
| Uninstall ext_name                                V  #  X |
+-----------------------------------------------------------+
| If you uninstall ext_name, the functionality it offers    |
| will no longer be available and the following items that  |
| require ext_name will be disabled:                        |
| +-------------------------------------------------------+ |
| | dependent ext_name_1 version                          | |
| | dependent ext_name_2 version                          | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| +-------------------------------------------------------+ |
| Do you want to uninstall ext_name?                        |
|                                                           |
|                                         Uninstall  Cancel |
+-----------------------------------------------------------+

Uninstalling an item without dependencies:
+-----------------------------------------------------------+
| Uninstall ext_name                                V  #  X |
+-----------------------------------------------------------+
| If you uninstall ext_name, the functionality it offers    |
| will no longer be available.                              |
|                                                           |
| Do you want to uninstall ext_name?                        |
|                                                           |
|                                         Uninstall  Cancel |
+-----------------------------------------------------------+ 

Disabling an item with dependencies that are enabled:
+-----------------------------------------------------------+
| Disable ext_name                                  V  #  X |
+-----------------------------------------------------------+
| If you disable ext_name, the following items that require |
| require ext_name will also be disabled:                   |
| +-------------------------------------------------------+ |
| | dependent ext_name_1 version                          | |
| | dependent ext_name_2 version                          | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| +-------------------------------------------------------+ |
| Do you want to disable ext_name?                          |
|                                                           |
|                                           Disable  Cancel |
+-----------------------------------------=-----------------+

Disabling an item without dependencies does not display a dialog.

> Why? Can't you just format the string in JS (var + " " + var2)?
I did it this way for consistency with the other use of list.xul and either way
is fine by me.

> How does the user figure out what "additional items" are? Does the extension
> "about" page list the dependencies?
They don't currently.

I am leaning towards combining the install of dependencies with the listing of
dependencies in order to not add complexity. The user could be given the option
to find the missing items, the ext's updateURL would be queried for them, and if
updateLinks are found he user would be given the option to install them.

This should not be a problem for a multiple item extension package and it is
possible to point the updateLink in the update datasource to a multiple item
extension package thereby updating the extension and its dependencies at the
same time. I thought about adding the items to the about dialog and decided not
to since that would be difficult to discover but I'm not convinced that it
shouldn't be added to it.
Just remembered why I didn't use the about dialog... this can be defined by the
extension so it can't be used.
Proposed ui for missing dependencies... I thought this wouldn't be necessary for
1.8 but if it is I can try to knock it out.

Extension item in the EM (similar to install update)
+-----------------------------------------------------+
| #### Extension Name                        +------+ |
| ## Disabled - requires additional items    | Find | |
+--------------------------------------------+------+-+

+-----------------------------------------------------+
| #### Extension Name                                 |
| #### @ Looking for missing items                    |
+-----------------------------------------------------+

The extension's updateURL would be queried using the id's of the missing items.

If all items are found the xpinstall dialog would be displayed with the items
listed.
+-----------------------------------------------------------+
| Install Required Items                            V  #  X |
+-----------------------------------------------------------+
| All of the items required by ext_name were found.         |
|                                                           |
| Do you want to install these items?                       |
| +-------------------------------------------------------+ |
| | dependent ext_filename_1 version                      | |
| | dependent ext_filename_2 version                      | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| +-------------------------------------------------------+ ||                 
                                         |
|                                           Install  Cancel |
+-----------------------------------------=-----------------+

If some or all items are not found display the following:
+-----------------------------------------------------------+
| Missing Items                                     V  #  X |
+-----------------------------------------------------------+
| The following items that are required by ext_name could   |
| not be found:                                             |
| +-------------------------------------------------------+ |
| | dependent ext_id_1 (minVersion through maxVersion)    | |
| | dependent ext_id_2 (minVersion through maxVersion)    | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| |                                                       | |
| +-------------------------------------------------------+ |
| Please contact the author about this problem.             |
|                                                           |
|                                                      OK   |
+-----------------------------------------=-----------------+
The UI is not necessary, please don't pollute your tree with it ;-)

I would like a patch with the "%S %S" property replaced with JS string
manipulations. Otherwise this looks good.
Attachment #191959 - Attachment is obsolete: true
Attachment #191959 - Flags: first-review?(benjamin)
Attached patch patch — — Splinter Review 
Fixed the JS string manipulations and straightened out the operation type so
the ui will always be in a consistent state when an enable or disable operation
is canceled, etc. - I also removed the redundant repeating of the extension
name in the new strings so that instead of:
If you uninstall ext_name, the functionality it offers will no longer be
available and the following items that require ext_name will be disabled:

it now reads as:
If you uninstall ext_name, the functionality it offers will no longer be
available and the following items that require this extension will be disabled:
Attachment #192058 - Flags: first-review?(benjamin)
Attachment #192058 - Flags: first-review?(benjamin) → first-review+
Attachment #192058 - Flags: approval1.8b4?
Comment on attachment 192058 [details] [diff] [review]
patch

Per the product team, this is too big+scary for 1.5 :-( We can get this landed
on trunk after the branch.
Attachment #192058 - Flags: approval1.8b4? → approval1.8b4-
All I can say is damn... with the number of regressions I have been seeing
elsewhere I spent a ton of time making this patch as safe and regression free as
possible in hope that it would make it into 1.8b4.

Anyways, the wiki has been updated to reflect the state of the patch so an idea
of the remaining functionality that needs to be completed can be flushed out.
I'd prefer to not make any of those items part of this bug since they require
new ui which would hold this up.
Whiteboard: [affects l10n][ETA 8/6]
beltzner - below are the strings we discussed. I will attach screenshots since to see the ui changes requires the use of an extension that is a dependency of another extension, setting the values on the extensions' metadata, and specific actions taken on these same extensions.

Index: mozilla/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.dtd
===================================================================
RCS file: /cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.dtd,v
retrieving revision 1.8
diff -u -8 -r1.8 extensions.dtd
--- mozilla/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.dtd	2 Aug 2005 11:57:28 -0000	1.8
+++ mozilla/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.dtd	3 Feb 2006 01:53:32 -0000
@@ -25,30 +25,30 @@
 <!ENTITY cmd.options.tooltip              "Set Options for the selected Extension">
 <!ENTITY cmd.optionsUnix.label            "Preferences">
 <!ENTITY cmd.optionsUnix.accesskey        "r">
 <!ENTITY cmd.optionsUnix.tooltip          "Edit Preferences for the selected Extension">
 <!ENTITY cmd.homepage.label               "Visit Home Page">
 <!ENTITY cmd.homepage.accesskey           "H">
 <!ENTITY cmd.about.label                  "About this Extension">
 <!ENTITY cmd.about.accesskey              "A">
+<!ENTITY cmd.cancelUninstall.label        "Cancel Uninstall">
+<!ENTITY cmd.cancelUninstall.accesskey    "C">

Index: mozilla/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties
===================================================================
RCS file: /cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties,v
retrieving revision 1.18.2.1
diff -u -8 -r1.18.2.1 extensions.properties
--- mozilla/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties	13 Sep 2005 19:04:11 -0000	1.18.2.1
+++ mozilla/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties	3 Feb 2006 01:53:32 -0000
@@ -15,25 +15,33 @@
 restartBeforeDisableTitle=Disable Extension
 restartBeforeEnableMessage=%S will be enabled when %S is restarted.
 restartBeforeDisableMessage=%S will be disabled when %S is restarted.
 restartBeforeUninstallTitle=Uninstall
 restartBeforeUninstallMessage=%S will be uninstalled when %S is restarted.
 restartBeforeInstallMessage=%S will be installed when %S is restarted.
 restartBeforeUpgradeMessage=%S will be upgraded when %S is restarted.
 incompatibleUpdateMessage=%S is checking for a compatibility update to %S.
-queryUninstallExtensionMessage=If you uninstall %S, the functionality it offers will no longer be available. Do you want to uninstall %S?
-queryUninstallThemeMessage=Do you want to uninstall %S?
-queryUninstallTitle=Uninstall %S
 installSuccess=Install completed successfully
 installWaiting=Waiting...
 installInstalling=Installing...
 droppedInWarning=The following items were found in your Extensions folder. Do you want to install them?
 disabledBySafeMode=%S is disabled by safe mode.
 
+uninstallButton=Uninstall
+disableButton=Disable
+cancelButton=Cancel
+uninstallTitle=Uninstall %S
+uninstallWarningDependMessage=If you uninstall %S, the functionality it offers will no longer be available and the following items that require this extension will be disabled:
+uninstallWarningMessage=If you uninstall %S, the functionality it offers will no longer be available.
+uninstallQueryMessage=Do you want to uninstall %S?
+disableTitle=Disable %S
+disableWarningDependMessage=If you disable %S, the following items that require this extension will also be disabled:
+disableQueryMessage=Do you want to disable %S?
+
@@ -53,16 +61,18 @@
 type-2=Extension
 incompatibleTitle=Incompatible %S
 incompatibleMsg=%S %S could not be installed because it is not compatible with %S %S. (%S %S will only work with %S versions from %S to %S)
 incompatibleMsgSingleAppVersion=%S %S could not be installed because it is not compatible with %S %S. (%S %S will only work with %S %S)
 incompatibleMessageNoApp=%S %S could not be installed because it is not compatible with %S.
 incompatibleOlder=versions 0.8 or older.
 incompatibleThemeName=this Theme
 incompatibleExtension=Disabled - not compatible with %S %S
+needsDependenciesDisabled=Disabled - requires additional items to operate.
+
Target Milestone: mozilla1.8beta4 → mozilla1.8.1
Component: XRE Startup → Extension/Theme Manager
Flags: first-review+
Product: Toolkit → Firefox
Target Milestone: mozilla1.8.1 → ---
Version: unspecified → 1.5 Branch
also, see previous comment
Attachment #211150 - Flags: ui-review?
Attachment #211150 - Flags: ui-review? → ui-review?(beltzner)
Attached patch updated to trunk (obsolete) — — Splinter Review 
Patch updated to apply to the trunk. The only difference should be an addition to cmd_cancelUninstall in extensions.js where it now updates the commands so the context menu is updated
Attachment #212016 - Flags: review?(benjamin)
There is one change to enableItem: in that this stomps on nsExtensionManager.js.in. I'll fix that when / if the rest passes review again.
Comment on attachment 212016 [details] [diff] [review]
updated to trunk

Hrm... how are you planning on backporting this to 1.8 (no interface changes!)? Perhaps it would be better to have nsIExtensionManager2 on trunk and branch to avoid code forkage until after ff2 ships. You make the call.
Attachment #212016 - Flags: review?(benjamin) → review+
Thanks for reminding me... I forgot.

I'll resubmit with nsIExtensionManager2 at the top of the patch so you can review it easily. I also just found a bug where the commands aren't updated properly when uninstalling an extension with only one extension in the list since it relies on being able to change the selection.
Benjamin, this is the same as the last patch except for the addition of nsIExtensionManager2 and an uninstall fix where the commands weren't updated when we only had one item in the list.
Attachment #212016 - Attachment is obsolete: true
Attachment #212123 - Flags: review?(benjamin)
Attachment #212123 - Flags: review?(benjamin) → review+
Fixed trunk
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Attachment #211150 - Flags: ui-review?(beltzner)
Fixed on MOZILLA_1_8_BRANCH. If there is any fallout please file new bugs an assign them to me.
Keywords: fixed1.8.1
Just wondering what would happen if an extension gets approved in UMO, and lists a depenency on another server, then at some point in the future they do a bait-and-switch with the other file to include a trojan. How would this be prevented?
(In reply to comment #25)
> Just wondering what would happen if an extension gets approved in UMO, and
> lists a depenency on another server, then at some point in the future they do a
> bait-and-switch with the other file to include a trojan. How would this be
> prevented?

Just as AMO has a policy of only approving extensions whose update URL is set back to AMO (or unset), we should add a policy that all dependencies must be available from AMO.
That is the plan for extensions hosted on AMO when that is implemented for dependencies.
Is there any bug filed for apt-get for extensions or "extention installer", which would facilitate discovery and installation of dependencies.

As user, I want to be able to say "I want to have extension X" and apt-get would say "Then I'm going to download/install extension(s): X, Y, Z; Is that Okay?". IOW, dependencies are hell to resolve manually - automation is possible and needed. Prerequisite is as mentioned in comments #25, #26 for all dependencies to reside on AMO/UMO. (Of course, ability to specify other locations would be good. Just like apt-get's sources.list)

Shortly. Does Mozilla plan to add to Ff capability to automatically install dependencies?

P.S. "apt-get" is package management utility of Debian fame, which automates software installation and maintenance. If you want to download/install piece of software - apt-get allows to automatically download/install its dependencies. If during upgrade, new dependency appeared - if user aproves - it would be automatically installed. Also, if during upgrade dependency was removed and dependency was installed automatically - iow, it wasn't selected by user - the dependency would be removed. You got the idea, I hope.
It is planned though there is no bug as of yet specifically for this. I would like to implement bug 285848 first since I may be able to implement this functionality in that bug.
Depends on: 329045
Target Milestone: --- → Firefox 2 alpha1
Is the wiki page in the URL field up-to-date as documentation for this feature?
I did a quick once over and it appears to be. The major blocker for taking this further is IMO having a ui that can be more easily extended to support user feedback along with the ability to display additional details which should be possible after bug 329045 is fixed.
No longer blocks: 285848
Does that mean that now install.rdf constructs like <http://wiki.mozilla.org/Talk:Extension_Dependencies> are now possible ?

E.g.: dependency for either sunbird or lightning, via "explicit" (first two examples) or "implicit" (3rd one) dependency on Thunderbird.

I've done a few tests but I didn't manage to get it to work.
The implementation is detailed on
http://wiki.mozilla.org/Extension_Dependencies

and does not implement what is discussed on the talk page
Is this implemented enough to justify documenting the stuff that's listed as "complete" or should I hold off?  I don't know whether they're in Firefox 2 or coming in Fx3.
It is in Firefox 2.0 and it would be helpful to document it using the existing docs as a guide... as a point of interest the extremely basic functionality of dependencies (e.g. em:requires) was documented as being part of 1.0 though it wasn't implemented.
Have updateURL and other options for em:requires been implemented yet?  Otherwise, this documentation appears to already be complete.
Given that the notes I see indicate that updateURL and the like haven't been implemented yet, and there's been no reply to the contrary, I'm marking this as doc complete.  Please re-mark if this is in error.
Keywords: dev-doc-needed → dev-doc-complete
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: