Add helper method to handle modal install dialog to Addons.js

RESOLVED FIXED

Status

RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: ashughes, Assigned: vladmaniac)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

7 years ago
All of the discovery pane tests for the Add-ons Manager are reusing the same code over and over again:

>+/**
>+ * Handle the modal dialog to install an addon
>+ */
>+function handleInstallAddonDialog(controller) {
>+  // Wait for the install button is enabled before clicking on it
>+  var installButton = new elementslib.Lookup(controller.window.document, 
>+                                             '/id("xpinstallConfirm")' +
>+                                             '/anon({"anonid":"buttons"})' + 
>+                                             '/{"dlgtype":"accept"}');
>+  controller.waitFor(function(){
>+    return !installButton.getNode().disabled; 
>+  }, "Install button is enabled: got '" + !installButton.getNode().disabled 
>+  + "', expected 'true'");
>+
>+  controller.click(installButton);
>+}

It would be useful if we could get this in the Addons shared module.
(Reporter)

Updated

7 years ago
Blocks: 657492
(Reporter)

Updated

7 years ago
Blocks: 658365
(Reporter)

Updated

7 years ago
Blocks: 658369
(Reporter)

Updated

7 years ago
Blocks: 664018
(Assignee)

Updated

7 years ago
Assignee: nobody → vlad.maniac
Status: NEW → ASSIGNED
(Reporter)

Comment 1

7 years ago
Created attachment 545523 [details] [diff] [review]
Patch v1.0

Initial patch -- basically moves the handleAddonInstallDialog() function, which is in use in many discovery pane tests already, into the Addons API. I've tested this patch with some of the Discovery Pane tests and it works great.

Vlad, if Henrik r- this patch, feel free to take over and make the revisions as necessary. Thanks.
Attachment #545523 - Flags: review?(hskupin)
Comment on attachment 545523 [details] [diff] [review]
Patch v1.0

>+// XPath for modal dialog install button
>+const MODAL_INSTALL_BUTTON = '/id("xpinstallConfirm")/anon({"anonid":"buttons"})/{"dlgtype":"accept"}';

We don't use lookups anymore in this module. This should be moved to a selector or nodeCollector calls.

>+  /**
>+   * Handles the modal dialog to install an add-on
>+   *
>+   * @param {MozMillController} MozMill Controller
>+   */
>+  handleInstallAddonDialog : function AddonsManager_handleInstallAddonDialog(aController) {

This method cannot be part of the add-ons manager. It has to be global in this module.

>+    // Wait for the install button to be enabled
>+    mozmill.utils.waitFor(function() {

nit: please add a space after function.
Attachment #545523 - Flags: review?(hskupin) → review-
(Assignee)

Updated

7 years ago
Depends on: 671322
(Assignee)

Comment 3

7 years ago
Created attachment 545695 [details] [diff] [review]
patch v1.1

Added patch with requested changes
Attachment #545695 - Flags: review?(hskupin)
Comment on attachment 545695 [details] [diff] [review]
patch v1.1

>+  var nodeCollector = new domUtils.nodeCollector(aController.window.document.documentElement);
>+
>+  nodeCollector.queryNodes("#xpinstallConfirm");
>+  nodeCollector.root = nodeCollector.nodes[0];
>+
>+  var installButton = nodeCollector.queryAnonymousNodes("dlgtype", "accept").elements[0];

Can you please separate the last line into two? Means call nodeCollector.elements[0] in the next line. Also remove the empty line after the nodeCollector declaration. Everything is one single block here.

> // Export of functions
> exports.addToWhiteList = addToWhiteList;
> exports.getInstalledAddons = getInstalledAddons;
> exports.removeFromWhiteList = removeFromWhiteList;
> exports.resetAmoPreviewUrls = resetAmoPreviewUrls;
> exports.useAmoPreviewUrls = useAmoPreviewUrls;
>+exports.handleInstallAddonDialog = handleInstallAddonDialog;

Please keep the alphabetical order of the export statements.
Attachment #545695 - Flags: review?(hskupin) → review-
(Assignee)

Comment 5

7 years ago
Created attachment 545865 [details] [diff] [review]
patch v1.2

Patch with requested changes
Attachment #545695 - Attachment is obsolete: true
Attachment #545865 - Flags: review?(hskupin)
Attachment #545865 - Flags: review?(hskupin) → review+
Vlad, can you please file a new bug so we can get the workaround removed?
You need to log in before you can comment on or make changes to this bug.