Clean up AutomationPredicate

chrome/browser/resources/chromeos/chromevox/cvox2/background/automation_predicate.js

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * @fileoverview ChromeVox predicates for the automation extension API.
  */
 
 goog.provide('AutomationPredicate');
 goog.provide('AutomationPredicate.Binary');
 goog.provide('AutomationPredicate.Unary');
 
 goog.require('constants');
 
 goog.scope(function() {
 var AutomationNode = chrome.automation.AutomationNode;
 var Dir = constants.Dir;
-var RoleType = chrome.automation.RoleType;
+var Role = chrome.automation.RoleType;
 
 /**
  * @constructor
  */
 AutomationPredicate = function() {};
 
 /**
  * @typedef {function(!AutomationNode) : boolean}
  */
 AutomationPredicate.Unary;
 
 /**
  * @typedef {function(!AutomationNode, !AutomationNode) : boolean}
  */
 AutomationPredicate.Binary;
 
 /**
- * Constructs a predicate given a role.
- * @param {RoleType} role
+ * Constructs a predicate given a list of roles or predicates.
+ * @param {!Array<AutomationPredicate.Unary|Role>} roles
  * @return {AutomationPredicate.Unary}
+ * @private
  */
-AutomationPredicate.withRole = function(role) {
+AutomationPredicate.roles_ = function(roles) {

[dmazzoni, 2016/09/23 17:52:57]

I think it's a little surprising to have a function
called roles_ that can take predicates too. I don't want
to hurt the nice brevity that this achieves, but it might
be nice to do it in a way that's a bit more explicit.

How about this idea:

AutomationPredicate.roles_ matches one role or an array of roles (that doesn't
seem unclear to me)

AutomationPredicate.anyPredicate_ matches if any of the predicates match

Then you could write formField like this:

AutomationPredicate.formField = AutomationPredicate.anyPredicate_(
    AutomationPredicate.button,
    AutomationPredicate.comboBox,
    AutomationPredicate.editText,
    AutomationPredicate.roles_(
        [Role.checkBox,
         Role.listBox,
         Role.slider,
         Role.tab,
         Role.tree]));

I think that'd be just as concise but a little easier to read.

+  if (!roles.length)
+    throw 'Expected at least one predicate.';
   return function(node) {
-    return node.role == role;
+    return roles.some(function(role) {
+      return role == node.role ||
+          (typeof role == 'function' && role(node));
+    });
   };
 };
 
 /** @type {AutomationPredicate.Unary} */
-AutomationPredicate.checkBox = AutomationPredicate.withRole(RoleType.checkBox);
+AutomationPredicate.checkBox = AutomationPredicate.roles_([Role.checkBox]);
 /** @type {AutomationPredicate.Unary} */
-AutomationPredicate.comboBox = AutomationPredicate.withRole(RoleType.comboBox);
+AutomationPredicate.comboBox = AutomationPredicate.roles_(
+    [Role.comboBox, Role.popUpButton, Role.menuListPopup]);
 /** @type {AutomationPredicate.Unary} */
-AutomationPredicate.heading = AutomationPredicate.withRole(RoleType.heading);
+AutomationPredicate.heading = AutomationPredicate.roles_([Role.heading]);
 /** @type {AutomationPredicate.Unary} */
 AutomationPredicate.inlineTextBox =
-    AutomationPredicate.withRole(RoleType.inlineTextBox);
+    AutomationPredicate.roles_([Role.inlineTextBox]);
 /** @type {AutomationPredicate.Unary} */
-AutomationPredicate.link = AutomationPredicate.withRole(RoleType.link);
+AutomationPredicate.link = AutomationPredicate.roles_([Role.link]);
 /** @type {AutomationPredicate.Unary} */
-AutomationPredicate.row = AutomationPredicate.withRole(RoleType.row);
+AutomationPredicate.row = AutomationPredicate.roles_([Role.row]);
 /** @type {AutomationPredicate.Unary} */
-AutomationPredicate.table = AutomationPredicate.withRole(RoleType.table);
+AutomationPredicate.table = AutomationPredicate.roles_([Role.table]);
 
 /**
  * @param {!AutomationNode} node
  * @return {boolean}
  */
 AutomationPredicate.button = function(node) {
   return /button/i.test(node.role);
 };
 
-
 /**
  * @param {!AutomationNode} node
  * @return {boolean}
  */
 AutomationPredicate.editText = function(node) {
   return node.state.editable &&
       node.parent &&
       !node.parent.state.editable;
 };
 
-/**
- * @param {!AutomationNode} node
- * @return {boolean}
- */
-AutomationPredicate.formField = function(node) {
-  switch (node.role) {
-    case 'button':
-    case 'buttonDropDown':
-    case 'checkBox':
-    case 'comboBox':
-    case 'date':
-    case 'dateTime':
-    case 'details':
-    case 'disclosureTriangle':
-    case 'form':
-    case 'menuButton':
-    case 'menuListPopup':
-    case 'popUpButton':
-    case 'radioButton':
-    case 'searchBox':
-    case 'slider':
-    case 'spinButton':
-    case 'switch':
-    case 'tab':
-    case 'textField':
-    case 'time':
-    case 'toggleButton':
-    case 'tree':
-      return true;
-  }
-  return false;
-};
+/** @type {AutomationPredicate.Unary} */
+AutomationPredicate.formField = AutomationPredicate.roles_([
+    AutomationPredicate.button,
+    Role.checkBox,
+    AutomationPredicate.comboBox,
+    AutomationPredicate.editText,
+    Role.listBox,
+    Role.slider,
+    Role.tab,
+    Role.tree]);
+
+/** @type {AutomationPredicate.Unary} */
+AutomationPredicate.landmark = AutomationPredicate.roles_([
+    Role.application,
+    Role.banner,
+    Role.complementary,
+    Role.contentInfo,
+    Role.form,
+    Role.group,
+    Role.main,
+    Role.navigation,
+    Role.region,
+    Role.search]);
 
 /**
  * @param {!AutomationNode} node
- * @return {boolean}
- */
-AutomationPredicate.landmark = function(node) {
-  switch (node.role) {
-    case 'application':
-    case 'banner':
-    case 'complementary':
-    case 'contentInfo':
-    case 'form':
-    case 'main':
-    case 'navigation':
-    case 'search':
-      return true;
-  }
-  return false;
-};
-
-/**
- * @param {!AutomationNode} node
  * @return {boolean}
  */
 AutomationPredicate.visitedLink = function(node) {
   return node.state.visited;
 };
 
 /**
  * @param {!AutomationNode} node
  * @return {boolean}
  */
 AutomationPredicate.focused = function(node) {
   return node.state.focused;
 };
 
 /**
  * @param {!AutomationNode} node
  * @return {boolean}
  */
 AutomationPredicate.leaf = function(node) {
   return !node.firstChild ||
-      node.role == RoleType.button ||
-      node.role == RoleType.buttonDropDown ||
-      node.role == RoleType.popUpButton ||
-      node.role == RoleType.slider ||
-      node.role == RoleType.textField ||
+      node.role == Role.button ||
+      node.role == Role.buttonDropDown ||
+      node.role == Role.popUpButton ||
+      node.role == Role.slider ||
+      node.role == Role.textField ||
       node.state.invisible ||
       node.children.every(function(n) {
         return n.state.invisible;
       });
 };
 
 /**
  * @param {!AutomationNode} node
  * @return {boolean}
  */
 AutomationPredicate.leafWithText = function(node) {
   return AutomationPredicate.leaf(node) &&
       !!(node.name || node.value);
 };
 
 /**
  * Matches against leaves or static text nodes. Useful when restricting
  * traversal to non-inline textboxes while still allowing them if navigation
  * already entered into an inline textbox.
  * @param {!AutomationNode} node
  * @return {boolean}
  */
 AutomationPredicate.leafOrStaticText = function(node) {
   return AutomationPredicate.leaf(node) ||
-      node.role == RoleType.staticText;
+      node.role == Role.staticText;
 };
 
 /**
  * Matches against nodes visited during object navigation. An object as
  * defined below, are all nodes that are focusable or static text. When used in
  * tree walking, it should visit all nodes that tab traversal would as well as
  * non-focusable static text.
  * @param {!AutomationNode} node
  * @return {boolean}
  */
 AutomationPredicate.object = function(node) {
   // Editable nodes are within a text-like field and don't make sense when
   // performing object navigation. Users should use line, word, or character
   // navigation. Only navigate to the top level node.
   if (node.parent && node.parent.state.editable)
     return false;
 
   return node.state.focusable ||
       (AutomationPredicate.leafOrStaticText(node) &&
        (/\S+/.test(node.name) ||
-        (node.role != RoleType.lineBreak &&
-         node.role != RoleType.staticText &&
-         node.role != RoleType.inlineTextBox)));
+        (node.role != Role.lineBreak &&
+         node.role != Role.staticText &&
+         node.role != Role.inlineTextBox)));
 };
 
 /**
  * @param {!AutomationNode} first
  * @param {!AutomationNode} second
  * @return {boolean}
  */
 AutomationPredicate.linebreak = function(first, second) {
   // TODO(dtseng): Use next/previousOnLin once available.
   var fl = first.location;
   var sl = second.location;
   return fl.top != sl.top ||
       (fl.top + fl.height != sl.top + sl.height);
 };
 
 /**
  * Matches against a node that contains other interesting nodes.
  * These nodes should always have their subtrees scanned when navigating.
  * @param {!AutomationNode} node
  * @return {boolean}
  */
 AutomationPredicate.container = function(node) {
   return AutomationPredicate.structuralContainer(node) ||
-      node.role == RoleType.div ||
-      node.role == RoleType.document ||
-      node.role == RoleType.group ||
-      node.role == RoleType.listItem ||
-      node.role == RoleType.toolbar ||
-      node.role == RoleType.window ||
+      node.role == Role.div ||
+      node.role == Role.document ||
+      node.role == Role.group ||
+      node.role == Role.listItem ||
+      node.role == Role.toolbar ||
+      node.role == Role.window ||
       // For example, crosh.
-      (node.role == RoleType.textField && node.state.readOnly) ||
+      (node.role == Role.textField && node.state.readOnly) ||
       (node.state.editable && node.parent && !node.parent.state.editable);
 };
 
 /**
  * Matches against nodes that contain interesting nodes, but should never be
  * visited.
  * @param {!AutomationNode} node
  * @return {boolean}
  */
 AutomationPredicate.structuralContainer = function(node) {
-  return node.role == RoleType.rootWebArea ||
-      node.role == RoleType.embeddedObject ||
-      node.role == RoleType.iframe ||
-      node.role == RoleType.iframePresentational;
+  return node.role == Role.rootWebArea ||
+      node.role == Role.embeddedObject ||
+      node.role == Role.iframe ||
+      node.role == Role.iframePresentational;
 };
 
 /**
  * Returns whether the given node should not be crossed when performing
  * traversals up the ancestry chain.
  * @param {AutomationNode} node
  * @return {boolean}
  */
 AutomationPredicate.root = function(node) {
   switch (node.role) {
-    case RoleType.dialog:
-    case RoleType.window:
+    case Role.dialog:
+    case Role.window:
       return true;
-    case RoleType.toolbar:
-      return node.root.role == RoleType.desktop;
-    case RoleType.rootWebArea:
-      return !node.parent || node.parent.root.role == RoleType.desktop;
+    case Role.toolbar:
+      return node.root.role == Role.desktop;
+    case Role.rootWebArea:
+      return !node.parent || node.parent.root.role == Role.desktop;
     default:
       return false;
   }
 };
 
 /**
  * Nodes that should be ignored while traversing the automation tree. For
  * example, apply this predicate when moving to the next object.
  * @param {!AutomationNode} node
  * @return {boolean}
  */
 AutomationPredicate.shouldIgnoreNode = function(node) {
   // Ignore invisible nodes.
   if (node.state.invisible ||
       (node.location.height == 0 && node.location.width == 0))
     return true;
 
   // Ignore structural containres.
   if (AutomationPredicate.structuralContainer(node))
     return true;
 
   // Ignore list markers since we already announce listitem role.
-  if (node.role == RoleType.listMarker)
+  if (node.role == Role.listMarker)
     return true;
 
   // Don't ignore nodes with names.
   if (node.name || node.value || node.description)
     return false;
 
   // Ignore some roles.
   return AutomationPredicate.leaf(node) &&
-      (node.role == RoleType.client ||
-       node.role == RoleType.column ||
-       node.role == RoleType.div ||
-       node.role == RoleType.group ||
-       node.role == RoleType.image ||
-       node.role == RoleType.staticText ||
-       node.role == RoleType.tableHeaderContainer);
+      (node.role == Role.client ||

[dmazzoni, 2016/09/23 17:52:57]

This looks like a great opportunity to make it more concise
by using AutomationPredicate.roles_().

+       node.role == Role.column ||
+       node.role == Role.div ||
+       node.role == Role.group ||
+       node.role == Role.image ||
+       node.role == Role.staticText ||
+       node.role == Role.tableHeaderContainer);
 };
 
 
 /**
  * Returns if the node has a meaningful checked state.
  * @param {!AutomationNode} node
  * @return {boolean}
  */
 AutomationPredicate.checkable = function(node) {
-  return node.role == RoleType.checkBox ||
-      node.role == RoleType.radioButton ||
-      node.role == RoleType.menuItemCheckBox ||
-      node.role == RoleType.menuItemRadio ||
-      node.role == RoleType.treeItem;
+  return node.role == Role.checkBox ||
+      node.role == Role.radioButton ||
+      node.role == Role.menuItemCheckBox ||
+      node.role == Role.menuItemRadio ||
+      node.role == Role.treeItem;
 };
 
 // Table related predicates.
 /**
  * Returns if the node has a cell like role.
  * @param {!AutomationNode} node
  * @return {boolean}
  */
 AutomationPredicate.cellLike = function(node) {
-  return node.role == RoleType.cell ||
-      node.role == RoleType.rowHeader ||
-      node.role == RoleType.columnHeader;
+  return node.role == Role.cell ||
+      node.role == Role.rowHeader ||
+      node.role == Role.columnHeader;
 };
 
 /**
  * Returns a predicate that will match against the directed next cell taking
  * into account the current ancestor cell's position in the table.
  * @param {AutomationNode} start
  * @param {{dir: (Dir|undefined),
 *           row: (boolean|undefined),
  *          col: (boolean|undefined)}} opts
  * |dir|, specifies direction for |row or/and |col| movement by one cell.
@@ skipping 60 matching lines @@
 };
 
 /**
  * Returns a predicate that will match against a heading with a specific
  * hierarchical level.
  * @param {number} level 1-6
  * @return {AutomationPredicate.Unary}
  */
 AutomationPredicate.makeHeadingPredicate = function(level) {
   return function(node) {
-    return node.role == RoleType.heading && node.hierarchicalLevel == level;
+    return node.role == Role.heading && node.hierarchicalLevel == level;
   };
 };
 
 });  // goog.scope