fix: handle more nested button group cases

packages/sandbox/src/pages/Buttons/HtmlButtons.js

@@ -1,6 +1,15 @@
 import React from 'react';
 
+const buttonImage = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAADIAAAAjCAYAAADWtVmPAAAABGdBTUEAALGPC/xhBQAAACBjSFJNAAB6JgAAgIQAAPoAAACA6AAAdTAAAOpgAAA6mAAAF3CculE8AAAAhGVYSWZNTQAqAAAACAAFARIAAwAAAAEAAQAAARoABQAAAAEAAABKARsABQAAAAEAAABSASgAAwAAAAEAAgAAh2kABAAAAAEAAABaAAAAAAAAAEgAAAABAAAASAAAAAEAA6ABAAMAAAABAAEAAKACAAQAAAABAAAAMqADAAQAAAABAAAAIwAAAADbcXyaAAAACXBIWXMAAAsTAAALEwEAmpwYAAABWWlUWHRYTUw6Y29tLmFkb2JlLnhtcAAAAAAAPHg6eG1wbWV0YSB4bWxuczp4PSJhZG9iZTpuczptZXRhLyIgeDp4bXB0az0iWE1QIENvcmUgNS40LjAiPgogICA8cmRmOlJERiB4bWxuczpyZGY9Imh0dHA6Ly93d3cudzMub3JnLzE5OTkvMDIvMjItcmRmLXN5bnRheC1ucyMiPgogICAgICA8cmRmOkRlc2NyaXB0aW9uIHJkZjphYm91dD0iIgogICAgICAgICAgICB4bWxuczp0aWZmPSJodHRwOi8vbnMuYWRvYmUuY29tL3RpZmYvMS4wLyI+CiAgICAgICAgIDx0aWZmOk9yaWVudGF0aW9uPjE8L3RpZmY6T3JpZW50YXRpb24+CiAgICAgIDwvcmRmOkRlc2NyaXB0aW9uPgogICA8L3JkZjpSREY+CjwveDp4bXBtZXRhPgpMwidZAAAIwklEQVRYCdVZ3W8dRxU/s7v3+vomdhwTNdSE2HkoTaBtILclKhLgPgCVUEUl5IjyD/AGEhLwzFMfQLRS8oIqVQUFqaoBtYRKfUAkaoLUKnGStqSUoIqU5tPYjhPf73t3ht/vzI69dtz42uahHXt3Zs6eOd/nzOxeI1k7U6kUHp6a6nB6ef/ez7muPBgZGbEinzLGbTNOBkTMFjGuLOL6RaI+J9JnxBXQF4xI4pwkYiTGOMI4MgbYWESauDnAHGAWMCtOUoy7gHetmC5Q2qDVBmYTiE3M68CrAee2ceYWns2ZSK62rL0w9vd//YM03fh4Yk6c4FrPxE1MxGZyMv3wgfsfAuwXuL4xGEeGUoGQNggB3l4C9nplMMjncTI4J8RfrQWt2GekyUXH5BXgUQ5GOiSXgujtlDaQE1D2p7svXDztJmC4SRgleOLyA/d92zrz5+EklptpSuQubnDIUnNLepGhykEjE8OLogrofGnV6iMIraoGI2B90D2Dg2Z4GEgYeBmeH4pjWYBCHSuHRt/956R6hjhX9+0b7UTpe/2RKTWsrUO2EiTjojWbcl0T6+4IvfAhBfKCcrC3a/aZqNyCW2wkn9/zzsX3IiK0jP3xcByX6tY1nDNlLGCM+2BeoyfeZi9NmjX4EEdlgmwwdBmJ1Ngex8ak7ifUwVytjJRbja3vlowZxUMkuyvwwce9ORSIkpEEXpmO4tbepF0r7y9EMoqQgs6oOp+Y5uIWKl9/FN1T65YqSVeiBwdQ11o2ZektbkqPCJFqUx+TSMiPLF2bYrK4mKU9LYqJa849lIDZbiYbyxN7uqXX5APq8tbGNrBtCERArVpFNGsKLsf5P8yCjOzZkPO7E1S4T1tkESYeyAc68regXA6kwzvgWG/KZUmnzqkhokcqIp2OuGZDjEXtDxsSVucNledFwnm6d8MjLtdyb6EONNkOTtjY6XBFH2BcGXAWe6bW4DZJL74vhW8+LveePyfJj34ondNTYs+/LbJ1QBwuoTK4KOri2hV88vTz48A/3ysNEOtigHq8I4Ic27sgiJ4xx/UaZqEn65XjMGdPT7oYEcoxiAzu/6KM/uoZGT5+XOT7T0l65qzYs+fFDQ2LlMpi4SXSDFee1kpYmIc+j6v8WLowgARDCcJqQN0DoILAZN2N1sYiZQRB40JBdo6Py8CBAzL7vaek9tvfiP39HzRsTOWAuEYdYYcjEs2aj591MxZsI1DDua0ot67fTyzOZqpL7+QoBIVhl12G1YpjHHPKg4NSeuIJuVWpyPzEIWn97qikfzomZmufmPv2KY7UUBT0kLU+3jwYadlSRaQEj5giXccjkxeJkx5bkB7oi8NMMVYs5gSPwNtHRqT/ySdVoRoUar/4oqSvvorCiRNW5RFxt+ZxsoOHcgVhTQnAEP+GKYrWR48YHKMBpIto4g00EKMxPE2/Ximx/FIxXKViUQpje6S0Y4dUEXKtiQlpPv+8dF9/XaJdnxEpb4Ey2Mqwp/XaKDV2LfIuJCwkLmLmA6Lp3iuZDC/zgFcir0r2PLMyi0IcR7IN4VZMElkYHpbSwYPSOndOar/Em8P0tMgWKIMc69kztBHY0AXYR9RgatGN6BHc4Almwq/SGSjky69IPwTmNdNsSh0Xqx2k91It9qsQWQHiKvUfBnyrY3poBKzA631KGsD+yNAEEyoRCkEHu/4cQmrm6aelfeqUxGNjiPIS3hHX4Y1MOvJleEEROoZC4KaBnWH02pESnbt49ws556sS6RrkCpXo1usye/KkTD/7rLRee01Y36IvoRzP34Q0EEf5K0FPZK07UJUvLMgjCraSrGJlD4I+ROKYPduqYwiKFMsU8Xh6Z/JBeIZU2mrJDCw/ffiwNF95RRUoPPqouJvzYpEbBvuO8soY6ThHKgMrJMgQZCQQzzssvx1KSIdkwEXBw9w/WVJoJZwVi5d6F5al9XlxF5994w25fviINCZfUgWSysOa0OmHV+AOMC4UfTRwPQlnLT8OMPZ5OMbZNi4teqShewijgwrlV/UyxgJNZOJGsUQos8yHGShw47nnpPrCC/qSkxzAIbLeEDs758MIimoLFvSzdd01bBlNTppM9gWtkAho0ly3ImBNy3OdTbsy8+abcuPoUbl95Ih6oPjlg9jwbou9dh1vO3j5JCIZbkKBoC0zEJRwt1Vz+rNjLw/E0XcWrMXLhBQ3oogKReFg5fTf7yufBElMod0N7A/YP+gtjxfE2FyvCiBI8VJYqDr3lwTnq//ikKUbYvZwAxywkhZA+Yz3fQG1MBGLRNZyirG24IENWWp1kWgn2g9nxVkku71OVzNFAq/Vl/UA5fmqWvOWJ3o4ovSwdEMoWbGF6Ne5j/yHp19adNPG2rQleldHZUV5YmJTh8S46J0ajgj452cgPmeEfRIaZU0aEBx/byXWdN9KbXwJ37XGcOph+aFCH/sG4VN8baQiN9rFaCr6yuXLDXyCfBmf1ekivDne+cf3P/6xz193Ym4cEnjk+dyNGizd6afM4o49dunSvD/8u+4zN1Nbx3tbP0Kujovv9Aw/XwByPVb6AMxgAWezPekGGoFHmIeeh2SM+dGnXoCsc6lNnUnxDoB8OCP4XUSmOifv3fU4jivH8KU7Yc5AE546cN4IZJdyB4RY9bSBtg55C+PsUVgQUAmmTMsaHiqMX+OVAp+GarQMExP/m0vCD4pViAfhvvv1ax/88dfQQZmAAn+cSU+N7Lk/td2fg/K3+iIz1Adx+TtFaN7VnjMFCkItH3to/llYv0TJax9+iiB82YWJn/PuG3mnIFrD+Qf7+V+j2Pzsa1c/OPsSZD8E2RcxqdUP4Bku+9vOnfd0o+JeGH4USbELv1gNw0kDYBx+sSpBWehpcLCSAvbTAuzJX65wWnT4NmR4kOJWjld2w8MxPYhQdRY01NPYyVLUTsrWwUMUGdcGvzY0aIJWA2N8anFVzBfA4xZw5kD7Why5C1+9cuUi5Twu48lj4n+x+h+6c1B46JQBvQAAAABJRU5ErkJggg==';
+
 function HtmlButtons() {
+  const inputButtonsStyle = {
+    alignItems: 'end',
+    display: 'inline-flex',
+    justifyContent: 'space-between',
+    width: 380,
+  };
+
   return (
     <div className="container">
       <h3>Native HTML</h3>
@@ -46,7 +55,41 @@ function HtmlButtons() {
       </button>
       <br />
       <br />
-      <input id="submit-input" type="submit" value="Submit Input" />
+      <button type="button">
+        <svg xmlns="http://www.w3.org/2000/svg" width="10" height="10" style={{ marginRight: 6 }}>
+          <circle cx="5" cy="5" r="5" stroke="black" strokeWidth="0" fill="red"></circle>
+        </svg>
+        <span>SVG text button</span>
+      </button>
+      <br />
+      <br />
+      <button data-qa="nested-svg" type="button" style={{ display: 'inline-flex', alignItems: 'center' }}>
+        <svg xmlns="http://www.w3.org/2000/svg" width="10" height="10" style={{ marginRight: 6 }}>
+          <circle cx="5" cy="5" r="5" stroke="black" strokeWidth="0" fill="red"></circle>
+        </svg>
+        <div>
+          <span>Nested SVG text button</span>
+        </div>
+      </button>
+      <br />
+      <br />
+      <button data-qa="nested-svg-with-nested-link" type="button" style={{ display: 'inline-flex', alignItems: 'center' }}>
+        <svg xmlns="http://www.w3.org/2000/svg" width="10" height="10" style={{ marginRight: 6 }}>
+          <circle cx="5" cy="5" r="5" stroke="black" strokeWidth="0" fill="red"></circle>
+        </svg>
+        <div>
+          <span>Nested SVG text button&nbsp;</span>
+        </div>
+        <a href="#"><span>with nested link</span></a>
+      </button>
+      <br />
+      <br />
+      <div style={inputButtonsStyle}>
+        <input id="submit-input" type="submit" value="Submit Input" />
+        <input id="button-input" type="button" value="Button Input" />
+        <input id="image-input" type="image" src={buttonImage} alt="Play" width="30" />
+        <input id="reset-input" type="reset" value="Reset Input" />
+      </div>
       <br />
       <br />
       <button data-qa="reload-top" onClick={() => window.top.location.reload()}>

src/web/PageEventCollector.ts

@@ -1,7 +1,6 @@
 import { DEFAULT_ATTRIBUTE_LIST } from './attribute';
 import {
   getInputElementValue,
-  getClickableGroup,
   getTopmostEditableElement,
   isVisible,
 } from './element';
@@ -40,7 +39,11 @@ export class PageEventCollector {
     });
 
     this._onDispose.push(() =>
-      document.removeEventListener(eventName, handler),
+      document.removeEventListener(eventName, handler, {
+        // `capture` value must match for proper removal
+        // https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/removeEventListener#Matching_event_listeners_for_removal
+        capture: true,
+      }),
     );
   }
 
@@ -56,18 +59,12 @@ export class PageEventCollector {
 
     const target = getTopmostEditableElement(event.target as HTMLElement);
 
-    let targetGroup: HTMLElement[];
-    if (isClick) {
-      targetGroup = getClickableGroup(target);
-    }
-
     const isTargetVisible = isVisible(target, window.getComputedStyle(target));
 
     const selector = buildSelector({
       attributes: this._attributes,
       isClick,
       target,
-      targetGroup,
     });
 
     const elementEvent: types.ElementEvent = {

src/web/cues.ts

@@ -13,7 +13,6 @@ export type BuildCues = {
   attributes: string[];
   isClick: boolean;
   target: HTMLElement;
-  targetGroup?: HTMLElement[];
 };
 
 type CueTypeConfig = {

src/web/element.ts

@@ -1,5 +1,8 @@
 import { getXpath } from './serialize';
 
+const BUTTON_INPUT_TYPES = ['button', 'image', 'reset', 'submit'];
+const MAX_CLICKABLE_ELEMENT_TRAVERSE_DEPTH = 10;
+
 export const isVisible = (
   element: Element,
   computedStyle?: CSSStyleDeclaration,
@@ -22,14 +25,79 @@ export const isVisible = (
 
 export const isClickable = (
   element: HTMLElement,
-  computedStyle: CSSStyleDeclaration,
+  computedStyle: CSSStyleDeclaration = window.getComputedStyle(element),
 ): boolean => {
   // assume it is clickable if the cursor is a pointer
   const clickable = computedStyle.cursor === 'pointer';
 
   return clickable && isVisible(element, computedStyle);
 };
 
+export const isLikelyTopOfClickGroup = (element: HTMLElement): boolean => {
+  const tagName = element.tagName.toLowerCase();
+  return (
+    ['a', 'button'].includes(tagName) ||
+    element.getAttribute('role') === 'button' ||
+    (tagName === 'input' && BUTTON_INPUT_TYPES.includes(element.getAttribute('type'))) ||
+    !element.parentElement ||
+    !isClickable(element.parentElement)
+  );
+};
+
+/**
+ * @summary Traverse the DOM in both directions, adding clickable elements to the array
+ *   until we've determined the full set of elements that are in the clickable area.
+ *   This is not foolproof because we can't know where exactly click handlers might
+ *   be attached, but we can do a pretty good job of guessing.
+ */
+const traverseClickableElements = (
+  element: HTMLElement,
+  group: HTMLElement[],
+  direction: 'up' | 'down' = 'up',
+  maxDepth: number = MAX_CLICKABLE_ELEMENT_TRAVERSE_DEPTH,
+  depth = 0,
+  ancestorChain: string[] = [],
+): void => {
+  // Regardless of which direction we're moving, stop if we hit a non-clickable element
+  if (!isClickable(element)) return;
+
+  // When moving up, when we reach the topmost clickable element, we
+  // stop traversing up and begin traversing down from there.
+  if (direction === 'up' && isLikelyTopOfClickGroup(element)) {
+    traverseClickableElements(element, group, 'down', maxDepth);
+    return;
+  }
+
+  // When moving down, stop if we hit the top of another click group (nested buttons)
+  if (direction === 'down' && depth > 0 && isLikelyTopOfClickGroup(element)) return;
+
+  const newDepth = depth + 1;
+  if (newDepth > maxDepth) return;
+
+  const lowerTagName = element.tagName.toLowerCase();
+
+  if (direction === 'up') {
+    // Call self for the parent element, incrementing depth
+    traverseClickableElements(element.parentElement, group, direction, maxDepth, newDepth, [lowerTagName, ...ancestorChain]);
+  } else {
+    // If we make it this far, this element should be part of the current group.
+    // We add elements to the group only on the way down to avoid adding any twice.
+    group.push(element);
+
+    const newAncestorChain = [...ancestorChain, lowerTagName];
+    console.debug('qawolf: added %s to click group', newAncestorChain.join(' > '));
+
+    // For now, let's skip going down into SVG descendants to keep this fast. If anyone
+    // finds a situation in which this causes problems, we can remove this.
+    if (lowerTagName !== 'svg') {
+      for (const child of element.children) {
+        // Call self for each child element, incrementing depth
+        traverseClickableElements(child as HTMLElement, group, direction, maxDepth, newDepth, newAncestorChain);
+      }
+    }
+  }
+};
+
 /**
  * @summary Sometimes there is a group of elements that together make up what appears
  *   to be a single button, link, image, etc. Examples: a > span | button > div > span
@@ -45,21 +113,10 @@ export const getClickableGroup = (element: HTMLElement): HTMLElement[] => {
   console.debug('qawolf: get clickable ancestor for', getXpath(element));
 
   const clickableElements = [];
-  let checkElement = element;
 
-  while (isClickable(checkElement, window.getComputedStyle(checkElement))) {
-    clickableElements.push(checkElement);
-
-    if (['a', 'button', 'input'].includes(checkElement.tagName.toLowerCase())) {
-      // stop crawling when the checkElement is a good clickable tag
-      break;
-    }
-
-    checkElement = checkElement.parentElement;
-
-    // stop crawling at the root
-    if (!checkElement) break;
-  }
+  // Recursive function that will mutate clickableElements array. A recursive
+  // function is better than loops to avoid blocking UI paint.
+  traverseClickableElements(element, clickableElements);
 
   return clickableElements;
 };

src/web/optimizeCues.ts

@@ -7,7 +7,7 @@ import {
 import { buildSelectorParts, isMatch } from './selectorEngine';
 import { SelectorPart } from './types';
 
-type CueGroup = {
+export type CueGroup = {
   cues: Cue[];
   penalty: number;
   selectorParts: SelectorPart[];
@@ -183,9 +183,8 @@ export const trimExcessCues = (
 // Pick the cues that match the target with the lowest penalty
 export const findBestCueGroup = (
   seedGroup: CueGroup,
-  target: HTMLElement,
   maxSize: number,
-  targetGroup?: HTMLElement[],
+  targetGroup: HTMLElement[],
 ): CueGroup => {
   let bestGroup = seedGroup;
 
@@ -219,16 +218,9 @@ export const findBestCueGroup = (
 
       const selectorParts = buildSelectorParts(cues);
 
-      // If these selector parts match the `target` element or any element in the
-      // target group, if there is one, then it's currently the best group.
-      if (
-        (
-          targetGroup &&
-          targetGroup.length &&
-          targetGroup.some((groupElement) => isMatch({ selectorParts, target: groupElement }))
-        ) ||
-        isMatch({ selectorParts, target })
-      ) {
+      // If these selector parts match any element that we are targeting,
+      // then it's currently the best group.
+      if (targetGroup.some((target) => isMatch({ selectorParts, target }))) {
         bestGroup = {
           cues,
           penalty,
@@ -243,14 +235,12 @@ export const findBestCueGroup = (
 };
 
 export const optimizeCues = (
-  cues: Cue[],
+  cueSets: Cue[][],
   target: HTMLElement,
-  targetGroup?: HTMLElement[],
-): CueGroup | null => {
-  const cueSets = buildCueSets(cues);
-
+  targetGroup: HTMLElement[],
+): CueGroup[] => {
   // Only use the first 50 cue sets (there should never be this many, usually just ~2-3)
-  const cueGroups = cueSets
+  return cueSets
     .slice(0, 50)
     .map((cueSet) => {
       // Trim down the cue group to 10 if possible
@@ -261,20 +251,28 @@ export const optimizeCues = (
       // 16 cues, samples of 5 is ~7000 combinations which took ~100ms on my machine
       if (!cueGroup || cueGroup.cues.length > 16) return null;
 
-      return findBestCueGroup(cueGroup, target, 5, targetGroup);
+      return findBestCueGroup(cueGroup, 5, targetGroup);
     })
     // Ignore invalid groups
-    .filter((a) => !!a)
-    // Rank by the total penalty then by value length
-    .sort((a, b) => {
-      if (a.penalty < b.penalty) return -1;
-      if (a.penalty > b.penalty) return 1;
+    .filter((a) => !!a);
+};
 
-      if (a.valueLength < b.valueLength) return -1;
-      if (a.valueLength > b.valueLength) return 1;
+export const pickBestCueGroup = (cueGroups: CueGroup[]): CueGroup | null => {
+  let bestCueGroup: CueGroup;
 
-      return 0;
-    });
+  if (cueGroups.length === 0) return null;
+
+  // Rank by the total penalty then by value length. This will take less
+  // time than .sort and will not mutate the cueGroups array.
+  for (const cueGroup of cueGroups) {
+    if (
+      !bestCueGroup ||
+      cueGroup.penalty < bestCueGroup.penalty ||
+      (cueGroup.penalty === bestCueGroup.penalty && cueGroup.valueLength < bestCueGroup.valueLength)
+    ) {
+      bestCueGroup = cueGroup;
+    }
+  }
 
-  return cueGroups.length ? cueGroups[0] : null;
+  return bestCueGroup;
 };

src/web/selector.ts

@@ -1,5 +1,9 @@
-import { buildCues, BuildCues } from './cues';
-import { optimizeCues } from './optimizeCues';
+import {
+  buildCues,
+  BuildCues,
+} from './cues';
+import { getClickableGroup } from './element';
+import { buildCueSets, CueGroup, optimizeCues, pickBestCueGroup } from './optimizeCues';
 import { getXpath } from './serialize';
 import { isMatch } from './selectorEngine';
 import { SelectorPart } from './types';
@@ -28,7 +32,15 @@ export const toSelector = (selectorParts: SelectorPart[]): string => {
 };
 
 export const buildSelector = (options: BuildCues): string => {
-  const { isClick, target, targetGroup } = options;
+  const { isClick, target } = options;
+
+  let targetGroup: HTMLElement[];
+  if (isClick) {
+    targetGroup = getClickableGroup(target);
+    if (targetGroup.length === 0) targetGroup = [target];
+  } else {
+    targetGroup = [target];
+  }
 
   // To save looping, see if we have already figured out a unique
   // selector for this target.
@@ -50,9 +62,17 @@ export const buildSelector = (options: BuildCues): string => {
     }
   }
 
-  const cues = buildCues(options);
+  const cueGroups: CueGroup[] = [];
+
+  targetGroup.forEach((targetElement) => {
+    const cues = buildCues({ ...options, target: targetElement });
+    const cueSets = buildCueSets(cues);
+    cueGroups.push(
+      ...optimizeCues(cueSets, targetElement, targetGroup)
+    );
+  });
 
-  const { selectorParts } = optimizeCues(cues, target, targetGroup) || {};
+  const { selectorParts } = pickBestCueGroup(cueGroups) || {};
   if (selectorParts) {
     // First cache it so that we don't need to do all the looping for this
     // same target next time. We cache `selectorParts` rather than `selector`