Bug 1253248 - Use element.Strategy enum consistently; r?automatedtester draft
authorAndreas Tolfsen <ato@mozilla.com>
Fri, 04 Mar 2016 12:01:09 +0000
changeset 338096 b5ce1d4507d66a339a8cb8c56f649420603477cf
parent 338095 05c087337043dd8e71cc27bdb5b9d55fd00aaa26
child 515716 a95127db276bd3224c3098fedfcdac0004a172ab
push id12420
push userbmo:ato@mozilla.com
push dateTue, 08 Mar 2016 11:26:41 +0000
reviewersautomatedtester
bugs1253248
milestone48.0a1
Bug 1253248 - Use element.Strategy enum consistently; r?automatedtester Removes the exported constants from testing/marionette/element.js and introduces the migrates to using the element.Strategy enum consistently throughout Marionette. The supported strategies array passed into ElementManager's ctor has also received some much needed attention and now actually works. MozReview-Commit-ID: LPuDX0aishM
testing/marionette/driver.js
testing/marionette/element.js
testing/marionette/listener.js
--- a/testing/marionette/driver.js
+++ b/testing/marionette/driver.js
@@ -3009,17 +3009,21 @@ var BrowserObj = function(win, driver) {
   this.window = win;
   this.driver = driver;
   this.knownFrames = [];
   this.startPage = "about:blank";
   // used in B2G to identify the homescreen content page
   this.mainContentId = null;
   // used to set curFrameId upon new session
   this.newSession = true;
-  this.elementManager = new ElementManager([NAME, LINK_TEXT, PARTIAL_LINK_TEXT]);
+  this.elementManager = new ElementManager([
+    element.Strategy.Name,
+    element.Strategy.LinkText,
+    element.Strategy.PartialLinkText,
+  ]);
   this.setBrowser(win);
 
   // A reference to the tab corresponding to the current window handle, if any.
   this.tab = null;
   this.pendingCommands = [];
 
   // we should have one FM per BO so that we can handle modals in each Browser
   this.frameManager = new frame.Manager(driver);
--- a/testing/marionette/element.js
+++ b/testing/marionette/element.js
@@ -1,19 +1,23 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 
+Cu.import("resource://gre/modules/Log.jsm");
+
 Cu.import("chrome://marionette/content/atom.js");
 Cu.import("chrome://marionette/content/error.js");
 
+const logger = Log.repository.getLogger("Marionette");
+
 /**
  * The ElementManager manages DOM element references and
  * interactions with elements.
  *
  * A web element is an abstraction used to identify an element when it
  * is transported across the protocol, between remote- and local ends.
  *
  * Each element has an associated web element reference (a UUID) that
@@ -24,52 +28,51 @@ Cu.import("chrome://marionette/content/e
  * The element manager provides a mapping between web element references
  * and DOM elements for each browsing context.  It also provides
  * functionality for looking up and retrieving elements.
  */
 
 this.EXPORTED_SYMBOLS = [
   "element",
   "ElementManager",
-  "CLASS_NAME",
-  "SELECTOR",
-  "ID",
-  "NAME",
-  "LINK_TEXT",
-  "PARTIAL_LINK_TEXT",
-  "TAG",
-  "XPATH",
-  "ANON",
-  "ANON_ATTRIBUTE"
 ];
 
 const DOCUMENT_POSITION_DISCONNECTED = 1;
 
 const uuidGen = Cc["@mozilla.org/uuid-generator;1"]
     .getService(Ci.nsIUUIDGenerator);
 
-this.CLASS_NAME = "class name";
-this.SELECTOR = "css selector";
-this.ID = "id";
-this.NAME = "name";
-this.LINK_TEXT = "link text";
-this.PARTIAL_LINK_TEXT = "partial link text";
-this.TAG = "tag name";
-this.XPATH = "xpath";
-this.ANON= "anon";
-this.ANON_ATTRIBUTE = "anon attribute";
+this.element = {};
+
+element.LegacyKey = "ELEMENT";
+element.Key = "element-6066-11e4-a52e-4f735466cecf";
 
-this.ElementManager = function ElementManager(notSupported) {
+element.Strategy = {
+  ClassName: "class name",
+  Selector: "css selector",
+  ID: "id",
+  Name: "name",
+  LinkText: "link text",
+  PartialLinkText: "partial link text",
+  TagName: "tag name",
+  XPath: "xpath",
+  Anon: "anon",
+  AnonAttribute: "anon attribute",
+};
+element.Strategies = new Set(Object.values(element.Strategy));
+
+this.ElementManager = function ElementManager(unsupportedStrategies = []) {
   this.seenItems = {};
   this.timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
-  this.elementStrategies = [CLASS_NAME, SELECTOR, ID, NAME, LINK_TEXT, PARTIAL_LINK_TEXT, TAG, XPATH, ANON, ANON_ATTRIBUTE];
-  for (let i = 0; i < notSupported.length; i++) {
-    this.elementStrategies.splice(this.elementStrategies.indexOf(notSupported[i]), 1);
+
+  this.supportedStrategies = new Set(element.Strategies);
+  for (let s of unsupportedStrategies) {
+    this.supportedStrategies.delete(s);
   }
-}
+};
 
 ElementManager.prototype = {
   /**
    * Reset values
    */
   reset: function EM_clear() {
     this.seenItems = {};
   },
@@ -314,17 +317,17 @@ ElementManager.prototype = {
   /**
    * Find a single element or a collection of elements starting at the
    * document root or a given node.
    *
    * If |timeout| is above 0, an implicit search technique is used.
    * This will wait for the duration of |timeout| for the element
    * to appear in the DOM.
    *
-   * See the |element.Strategies| enum for a full list of supported
+   * See the |element.Strategy| enum for a full list of supported
    * search strategies that can be passed to |strategy|.
    *
    * Available flags for |opts|:
    *
    *     |all|
    *       If true, a multi-element search selector is used and a sequence
    *       of elements will be returned.  Otherwise a single element.
    *
@@ -370,26 +373,22 @@ ElementManager.prototype = {
     }
 
     return new Promise((resolve, reject) => {
       let findElements = implicitlyWaitFor(
           () => this.find_(container, strategy, selector, searchFn, opts),
           opts.timeout);
 
       findElements.then(foundEls => {
-        // when looking for a single element and none is found,
-        // an error must be thrown
-        if (foundEls.length == 0 && !opts.all) {
+        // the following code ought to be moved into findElement
+        // and findElements when bug 1254486 is addressed
+        if (!opts.all && (!foundEls || foundEls.length == 0)) {
           let msg;
           switch (strategy) {
-            case ANON:
-              msg = "Unable to locate anonymous children";
-              break;
-
-            case ANON_ATTRIBUTE:
+            case element.Strategy.AnonAttribute:
               msg = "Unable to locate anonymous element: " + JSON.stringify(selector);
               break;
 
             default:
               msg = "Unable to locate element: " + selector;
           }
 
           reject(new NoSuchElementError(msg));
@@ -415,25 +414,26 @@ ElementManager.prototype = {
     let rootNode = container.shadowRoot || container.frame.document;
     let startNode;
     if (opts.startNode) {
       startNode = this.getKnownElement(opts.startNode, container);
     } else {
       startNode = rootNode;
     }
 
-    if (strategy in element.Strategies) {
-      throw new InvalidSelectorError("No such strategy: " + strategy);
+    if (!this.supportedStrategies.has(strategy)) {
+      throw new InvalidSelectorError("Strategy not supported: " + strategy);
     }
 
     let res;
     try {
       res = searchFn(strategy, selector, rootNode, startNode);
     } catch (e) {
-      throw new InvalidSelectorError(`Given ${strategy} expression "${selector}" is invalid`);
+      throw new InvalidSelectorError(
+          `Given ${strategy} expression "${selector}" is invalid`);
     }
 
     if (element.isElementCollection(res)) {
       return res;
     } else if (res !== null) {
       return [res];
     }
     return [];
@@ -480,165 +480,168 @@ ElementManager.prototype = {
       element = values.iterateNext();
     }
     return elements;
   },
 
   /**
    * Finds a single element.
    *
-   * @param {String} using
-   *     Which selector search method to use.
-   * @param {String} value
-   *     Selector query.
-   * @param {nsIDOMElement} rootNode
+   * @param {element.Strategy} using
+   *     Selector strategy to use.
+   * @param {string} value
+   *     Selector expression.
+   * @param {DOMElement} rootNode
    *     Document root.
-   * @param {nsIDOMElement=} startNode
-   *     Optional node from which we start searching.
+   * @param {DOMElement=} startNode
+   *     Optional node from which to start searching.
    *
-   * @return {nsIDOMElement}
-   *     Returns found element.
+   * @return {DOMElement}
+   *     Found elements.
+   *
    * @throws {InvalidSelectorError}
-   *     If the selector query string (value) is invalid, or the selector
-   *     search method (using) is unknown.
+   *     If strategy |using| is not recognised.
+   * @throws {Error}
+   *     If selector expression |value| is malformed.
    */
-  findElement: function EM_findElement(using, value, rootNode, startNode) {
-    let element;
-
+  findElement: function(using, value, rootNode, startNode) {
     switch (using) {
-      case ID:
-        element = startNode.getElementById ?
-                  startNode.getElementById(value) :
-                  this.findByXPath(rootNode, `.//*[@id="${value}"]`, startNode);
-        break;
+      case element.Strategy.ID:
+        if (startNode.getElementById) {
+          return startNode.getElementById(value);
+        }
+        return this.findByXPath(rootNode, `.//*[@id="${value}"]`, startNode);
 
-      case NAME:
-        element = startNode.getElementsByName ?
-                  startNode.getElementsByName(value)[0] :
-                  this.findByXPath(rootNode, `.//*[@name="${value}"]`, startNode);
-        break;
+      case element.Strategy.Name:
+        if (startNode.getElementsByName) {
+          return startNode.getElementsByName(value)[0];
+        }
+        return this.findByXPath(rootNode, `.//*[@name="${value}"]`, startNode);
 
-      case CLASS_NAME:
-        element = startNode.getElementsByClassName(value)[0]; //works for >=FF3
-        break;
+      case element.Strategy.ClassName:
+        // works for >= Firefox 3
+        return  startNode.getElementsByClassName(value)[0];
+
+      case element.Strategy.TagName:
+        // works for all elements
+        return startNode.getElementsByTagName(value)[0];
 
-      case TAG:
-        element = startNode.getElementsByTagName(value)[0]; //works for all elements
-        break;
-
-      case XPATH:
-        element = this.findByXPath(rootNode, value, startNode);
-        break;
+      case element.Strategy.XPath:
+        return  this.findByXPath(rootNode, value, startNode);
 
-      case LINK_TEXT:
-      case PARTIAL_LINK_TEXT:
-        let allLinks = startNode.getElementsByTagName('A');
-        for (let i = 0; i < allLinks.length && !element; i++) {
+      // TODO(ato): Rewrite this, it's hairy:
+      case element.Strategy.LinkText:
+      case element.Strategy.PartialLinkText:
+        let el;
+        let allLinks = startNode.getElementsByTagName("A");
+        for (let i = 0; i < allLinks.length && !el; i++) {
           let text = allLinks[i].text;
-          if (PARTIAL_LINK_TEXT == using) {
+          if (using == element.Strategy.PartialLinkText) {
             if (text.indexOf(value) != -1) {
-              element = allLinks[i];
+              el = allLinks[i];
             }
           } else if (text == value) {
-            element = allLinks[i];
+            el = allLinks[i];
           }
         }
-        break;
-      case SELECTOR:
+        return el;
+
+      case element.Strategy.Selector:
         try {
-          element = startNode.querySelector(value);
+          return startNode.querySelector(value);
         } catch (e) {
           throw new InvalidSelectorError(`${e.message}: "${value}"`);
         }
-        break;
+
+      case element.Strategy.Anon:
+        return rootNode.getAnonymousNodes(startNode);
 
-      case ANON:
-        element = rootNode.getAnonymousNodes(startNode);
-        if (element != null) {
-          element = element[0];
-        }
-        break;
-
-      case ANON_ATTRIBUTE:
+      case element.Strategy.AnonAttribute:
         let attr = Object.keys(value)[0];
-        element = rootNode.getAnonymousElementByAttribute(startNode, attr, value[attr]);
-        break;
+        return rootNode.getAnonymousElementByAttribute(startNode, attr, value[attr]);
 
       default:
         throw new InvalidSelectorError(`No such strategy: ${using}`);
     }
-
-    return element;
-  },
+},
 
   /**
-   * Helper method to find. Finds all element using find's criteria
+   * Find multiple elements.
    *
-   * @param string using
-   *        String identifying which search method to use
-   * @param string value
-   *        Value to look for
-   * @param nsIDOMElement rootNode
-   *        Document root
-   * @param nsIDOMElement startNode
-   *        Node from which we start searching
+   * @param {element.Strategy} using
+   *     Selector strategy to use.
+   * @param {string} value
+   *     Selector expression.
+   * @param {DOMElement} rootNode
+   *     Document root.
+   * @param {DOMElement=} startNode
+   *     Optional node from which to start searching.
    *
-   * @return nsIDOMElement
-   *        Returns found elements or throws Exception if not found
+   * @return {DOMElement}
+   *     Found elements.
+   *
+   * @throws {InvalidSelectorError}
+   *     If strategy |using| is not recognised.
+   * @throws {Error}
+   *     If selector expression |value| is malformed.
    */
-  findElements: function EM_findElements(using, value, rootNode, startNode) {
-    let elements = [];
+  findElements: function(using, value, rootNode, startNode) {
     switch (using) {
-      case ID:
+      case element.Strategy.ID:
         value = `.//*[@id="${value}"]`;
-      case XPATH:
-        elements = this.findByXPathAll(rootNode, value, startNode);
-        break;
-      case NAME:
-        elements = startNode.getElementsByName ?
-                   startNode.getElementsByName(value) :
-                   this.findByXPathAll(rootNode, `.//*[@name="${value}"]`, startNode);
-        break;
-      case CLASS_NAME:
-        elements = startNode.getElementsByClassName(value);
-        break;
-      case TAG:
-        elements = startNode.getElementsByTagName(value);
-        break;
-      case LINK_TEXT:
-      case PARTIAL_LINK_TEXT:
-        let allLinks = startNode.getElementsByTagName('A');
+
+      // fall through
+      case element.Strategy.XPath:
+        return this.findByXPathAll(rootNode, value, startNode);
+
+      case element.Strategy.Name:
+        if (startNode.getElementsByName) {
+          return startNode.getElementsByName(value);
+        }
+        return this.findByXPathAll(rootNode, `.//*[@name="${value}"]`, startNode);
+
+      case element.Strategy.ClassName:
+        return startNode.getElementsByClassName(value);
+
+      case element.Strategy.TagName:
+        return startNode.getElementsByTagName(value);
+
+      case element.Strategy.LinkText:
+      case element.Strategy.PartialLinkText:
+        let els = [];
+        let allLinks = startNode.getElementsByTagName("A");
         for (let i = 0; i < allLinks.length; i++) {
           let text = allLinks[i].text;
-          if (PARTIAL_LINK_TEXT == using) {
+          if (using == element.Strategy.PartialLinkText) {
             if (text.indexOf(value) != -1) {
-              elements.push(allLinks[i]);
+              els.push(allLinks[i]);
             }
           } else if (text == value) {
-            elements.push(allLinks[i]);
+            els.push(allLinks[i]);
           }
         }
-        break;
-      case SELECTOR:
-        elements = Array.slice(startNode.querySelectorAll(value));
-        break;
-      case ANON:
-        elements = rootNode.getAnonymousNodes(startNode) || [];
-        break;
-      case ANON_ATTRIBUTE:
+        return els;
+
+      case element.Strategy.Selector:
+        return Array.slice(startNode.querySelectorAll(value));
+
+      case element.Strategy.Anon:
+        return rootNode.getAnonymousNodes(startNode);
+
+      case element.Strategy.AnonAttribute:
         let attr = Object.keys(value)[0];
         let el = rootNode.getAnonymousElementByAttribute(startNode, attr, value[attr]);
-        if (el != null) {
-          elements = [el];
+        if (el) {
+          return [el];
         }
-        break;
+        return [];
+
       default:
         throw new InvalidSelectorError(`No such strategy: ${using}`);
     }
-    return elements;
   },
 };
 
 /**
  * Runs function off the main thread until its return value is truthy
  * or the provided timeout is reached.  The function is guaranteed to be
  * run at least once, irregardless of the timeout.
  *
@@ -691,35 +694,17 @@ function implicitlyWaitFor(func, timeout
 
     timer.init(observer, interval, Ci.nsITimer.TYPE_REPEATING_SLACK);
 
   // cancel timer and return result for yielding
   }).then(res => {
     timer.cancel();
     return res;
   });
-};
-
-this.element = {};
-
-element.LegacyKey = "ELEMENT";
-element.Key = "element-6066-11e4-a52e-4f735466cecf";
-
-element.Strategies = {
-  CLASS_NAME: 0,
-  SELECTOR: 1,
-  ID: 2,
-  NAME: 3,
-  LINK_TEXT: 4,
-  PARTIAL_LINK_TEXT: 5,
-  TAG: 6,
-  XPATH: 7,
-  ANON: 8,
-  ANON_ATTRIBUTE: 9,
-};
+}
 
 element.isElementCollection = function(seq) {
   if (seq === null) {
     return false;
   }
 
   const arrayLike = {
     "[object Array]": 0,
--- a/testing/marionette/listener.js
+++ b/testing/marionette/listener.js
@@ -34,17 +34,17 @@ var isB2G = false;
 
 var marionetteTestName;
 var winUtil = content.QueryInterface(Ci.nsIInterfaceRequestor)
     .getInterface(Ci.nsIDOMWindowUtils);
 var listenerId = null; // unique ID of this listener
 var curContainer = { frame: content, shadowRoot: null };
 var isRemoteBrowser = () => curContainer.frame.contentWindow !== null;
 var previousContainer = null;
-var elementManager = new ElementManager([]);
+var elementManager = new ElementManager();
 
 // Holds session capabilities.
 var capabilities = {};
 var interactions = new Interactions(() => capabilities);
 
 var actions = new action.Chain(checkForInterrupted);
 
 // Contains the last file input element that was the target of