feat: improve xpath locator suggestions

- Add some attributes that we should consider for uniqueness
- Add search for attributes that might be unique as pairs
- Show a few more attributes by default in the XML source

app/renderer/components/Inspector/Source.js

@@ -10,6 +10,11 @@ const IMPORTANT_ATTRS = [
   'resource-id',
   'AXDescription',
   'AXIdentifier',
+  'text',
+  'label',
+  'value',
+  'id',
+  'class',
 ];
 
 /**
@@ -24,7 +29,7 @@ const Source = (props) => {
     let attrs = [];
 
     for (let attr of Object.keys(attributes)) {
-      if (IMPORTANT_ATTRS.includes(attr) || showAllAttrs) {
+      if ((IMPORTANT_ATTRS.includes(attr) && attributes[attr]) || showAllAttrs) {
         attrs.push(<span key={attr}>&nbsp;
           <i
             className={InspectorStyles.sourceAttrName}

app/renderer/util.js

@@ -9,13 +9,18 @@ const VALID_W3C_CAPS = ['platformName', 'browserName', 'browserVersion', 'accept
   'pageLoadStrategy', 'proxy', 'setWindowRect', 'timeouts', 'unhandledPromptBehavior'];
 
 
-// Attributes on nodes that we know are unique to the node
+// Attributes on nodes that are likely to be unique to the node so we should consider first when
+// suggesting xpath locators. These are considered IN ORDER.
 const UNIQUE_XPATH_ATTRIBUTES = [
   'name',
   'content-desc',
   'id',
   'accessibility-id',
+  'text',
+  'label',
+  'value',
 ];
+
 const UNIQUE_CLASS_CHAIN_ATTRIBUTES = [
   'label',
   'name',
@@ -82,44 +87,95 @@ export function xmlToJSON (source) {
   return firstChild ? translateRecursively(firstChild) : {};
 }
 
+/**
+ * Return information about whether an xpath query results in a unique element, and the non-unique
+ * index of the element in the document if not unique
+ *
+ * @param {string} xpath
+ * @param {DOMDocument} doc
+ * @param {DOMNode} domNode - the current node
+ * @returns {[boolean, number?]} tuple consisting of (1) whether the xpath is unique and (2) its index in
+ * the set of other similar nodes if not unique
+ */
+function isXpathUnique(xpath, doc, domNode) {
+  let othersWithAttr = [];
+
+  // If the XPath does not parse, move to the next unique attribute
+  try {
+    othersWithAttr = XPath.select(xpath, doc);
+  } catch (ign) {
+    return [false];
+  }
+
+  if (othersWithAttr.length > 1) {
+    return [false, othersWithAttr.indexOf(domNode)];
+  }
+
+  return [true];
+}
+
 /**
  * Get an optimal XPath for a DOMNode
  *
  * @param {DOMDocument} doc
  * @param {DOMNode} domNode
- * @param {Array<String>} uniqueAttributes Attributes we know are unique (defaults to just 'id')
+ * @param {Array<String>} uniqueAttributes Attributes we know are unique
  * @returns {string|null}
  */
-export function getOptimalXPath (doc, domNode, uniqueAttributes = ['id']) {
+export function getOptimalXPath (doc, domNode, uniqueAttributes) {
   try {
     // BASE CASE #1: If this isn't an element, we're above the root, return empty string
     if (!domNode.tagName || domNode.nodeType !== 1) {
       return '';
     }
 
-    // BASE CASE #2: If this node has a unique attribute, return an absolute XPath with that attribute
-    for (let attrName of uniqueAttributes) {
+    const tagForXpath = domNode.tagName || '*';
+    let semiUniqueXpath = null;
+
+    // BASE CASE #2: If this node has a unique attribute or content attribute, return an absolute XPath with that attribute
+    for (const attrName of uniqueAttributes) {
       const attrValue = domNode.getAttribute(attrName);
-      if (attrValue) {
-        let xpath = `//${domNode.tagName || '*'}[@${attrName}="${attrValue}"]`;
-        let othersWithAttr;
+      if (!attrValue) {
+        continue;
+      }
+      const xpath = `//${tagForXpath}[@${attrName}="${attrValue}"]`;
+      const [isUnique, indexIfNotUnique] = isXpathUnique(xpath, doc, domNode);
+      if (isUnique) {
+        return xpath;
+      }
 
-        // If the XPath does not parse, move to the next unique attribute
-        try {
-          othersWithAttr = XPath.select(xpath, doc);
-        } catch (ign) {
-          continue;
-        }
+      // if the xpath wasn't totally unique it might still be our best bet. Store a less unique
+      // version qualified by an index for later in semiUniqueXpath. If we can't find a better
+      // unique option down the road, we'll fall back to this
+      if (!semiUniqueXpath && !_.isUndefined(indexIfNotUnique)) {
+        semiUniqueXpath = `(${xpath})[${indexIfNotUnique + 1}]`;
+      }
+    }
 
-        // If the attribute isn't actually unique, get it's index too
-        if (othersWithAttr.length > 1) {
-          let index = othersWithAttr.indexOf(domNode);
-          xpath = `(${xpath})[${index + 1}]`;
-        }
+    // BASE CASE #3: If this node has a unique pair of attributes, return an xpath based on that pair
+    const pairAttributes = uniqueAttributes.flatMap((v1, i) =>
+      uniqueAttributes.slice(i + 1).map((v2) => [v1, v2]));
+    for (const [attr1Name, attr2Name] of pairAttributes) {
+      const attr1Value = domNode.getAttribute(attr1Name);
+      const attr2Value = domNode.getAttribute(attr2Name);
+      if (!attr1Value || !attr2Value) {
+        continue;
+      }
+      const xpath = `//${tagForXpath}[@${attr1Name}="${attr1Value}" and @${attr2Name}="${attr2Value}"]`;
+      if (isXpathUnique(xpath, doc, domNode)[0]) {
         return xpath;
       }
     }
 
+    // if we couldn't find any good totally unique or pairwise unique attributes, but we did find
+    // almost unique attributes qualified by an index, return that instead
+    if (semiUniqueXpath) {
+      return semiUniqueXpath;
+    }
+
+    // Otherwise fall back to a purely hierarchical expression of this dom node's position in the
+    // document as a last resort.
+
     // Get the relative xpath of this node using tagName
     let xpath = `/${domNode.tagName}`;
 

test/unit/util-specs.js

@@ -10,7 +10,7 @@ chai.use(chaiAsPromised);
 
 // Helper that checks that the optimal xpath for a node is the one that we expect and also
 // checks that the XPath successfully locates the node in it's doc
-function testXPath (doc, node, expectedXPath, uniqueAttributes) {
+function testXPath (doc, node, expectedXPath, uniqueAttributes = ['id']) {
   getOptimalXPath(doc, node, uniqueAttributes).should.equal(expectedXPath);
   xpath.select(expectedXPath, doc)[0].should.equal(node);
 }
@@ -602,6 +602,20 @@ describe('util.js', function () {
         testXPath(doc, children[3], '(//child[@id="foo"])[4]');
         testXPath(doc, children[4], '(//child[@id="foo"])[5]');
       });
+      it('should return conjunctively unique xpath locators if they exist', function () {
+        doc = new DOMParser().parseFromString(`<root>
+          <child text='bar'></child>
+          <child text='yo'></child>
+          <child id='foo' text='yo'></child>
+          <child id='foo'></child>
+        </root>`);
+        const children = doc.getElementsByTagName('child');
+        const attrs = ['id', 'text'];
+        testXPath(doc, children[0], '//child[@text="bar"]', attrs);
+        testXPath(doc, children[1], '(//child[@text="yo"])[1]', attrs);
+        testXPath(doc, children[2], '//child[@id="foo" and @text="yo"]', attrs);
+        testXPath(doc, children[3], '(//child[@id="foo"])[2]', attrs);
+      });
     });
 
     describe('when exceptions are thrown', function () {
@@ -613,7 +627,7 @@ describe('util.js', function () {
             <grandchild id='hello'></grandchild>
           </child>
         </node>`);
-        getOptimalXPath(doc, doc.getElementById('hello')).should.equal('/node/child[2]/grandchild');
+        getOptimalXPath(doc, doc.getElementById('hello'), ['id']).should.equal('/node/child[2]/grandchild');
         xpathSelectStub.restore();
       });
 
@@ -626,7 +640,7 @@ describe('util.js', function () {
         </node>`);
         const node = doc.getElementById('hello');
         node.getAttribute = () => { throw new Error('Some unexpected error'); };
-        should.not.exist(getOptimalXPath(doc, node));
+        should.not.exist(getOptimalXPath(doc, node, ['id']));
       });
     });
   });