feat: support startNodes in locators (#2042)

Additionally, added `invalidArgument` exception if the startNode is not an html element in case of css selector. --------- Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: sadym-chromium <sadym-chromium@users.noreply.github.com>
GoogleChromeLabs · Mar 21, 2024 · 62d58a9 · 62d58a9
1 parent ba68a85
commit 62d58a9
Show file tree

Hide file tree

Showing 10 changed files with 82 additions and 154 deletions.
diff --git a/src/bidiMapper/domains/context/BrowsingContextImpl.ts b/src/bidiMapper/domains/context/BrowsingContextImpl.ts
@@ -1017,24 +1017,20 @@ export class BrowsingContextImpl {
         `serializationOptions is not supported`
       );
     }
-    if (params.startNodes !== undefined) {
-      // TODO: implement.
-      throw new UnsupportedOperationException(
-        `serializationOptions is not supported`
-      );
-    }
 
     // TODO: create a dedicated sandbox instead of `#defaultRealm`.
     return await this.#locateNodesByLocator(
       this.#defaultRealm,
       params.locator,
+      params.startNodes ?? [],
       params.maxNodeCount
     );
   }
 
   #getLocatorDelegate(
     locator: BrowsingContext.Locator,
-    maxNodeCount?: number
+    maxNodeCount: number | undefined,
+    startNodes: Script.SharedReference[]
   ): {
     functionDeclaration: string;
     argumentsLocalValues: Script.LocalValue[];
@@ -1043,12 +1039,27 @@ export class BrowsingContextImpl {
       case 'css':
         return {
           functionDeclaration: String(
-            (cssSelector: string, maxNodeCount: number) => {
-              const results = document.querySelectorAll(cssSelector);
-              const returnedNodes = [];
-              for (const item of results) {
-                returnedNodes.push(item);
-              }
+            (
+              cssSelector: string,
+              maxNodeCount: number,
+              ...startNodes: HTMLElement[]
+            ) => {
+              const locateNodesUsingCss = (element: HTMLElement) => {
+                if (!(element instanceof HTMLElement)) {
+                  throw new Error(
+                    'startNodes in css selector should be HTMLElement'
+                  );
+                }
+                return [...element.querySelectorAll(cssSelector)];
+              };
+
+              startNodes = startNodes.length > 0 ? startNodes : [document.body];
+              const returnedNodes = startNodes
+                .map((startNode) =>
+                  // TODO: stop search early if `maxNodeCount` is reached.
+                  locateNodesUsingCss(startNode)
+                )
+                .flat(1);
               return maxNodeCount === 0
                 ? returnedNodes
                 : returnedNodes.slice(0, maxNodeCount);
@@ -1059,23 +1070,39 @@ export class BrowsingContextImpl {
             {type: 'string', value: locator.value},
             // `maxNodeCount` with `0` means no limit.
             {type: 'number', value: maxNodeCount ?? 0},
+            // `startNodes`
+            ...startNodes,
           ],
         };
       case 'xpath':
         return {
           functionDeclaration: String(
-            (xPathSelector: string, maxNodeCount: number) => {
+            (
+              xPathSelector: string,
+              maxNodeCount: number,
+              ...startNodes: HTMLElement[]
+            ) => {
               // https://w3c.github.io/webdriver-bidi/#locate-nodes-using-xpath
               const evaluator = new XPathEvaluator();
               const expression = evaluator.createExpression(xPathSelector);
-              const xPathResult = expression.evaluate(
-                document,
-                XPathResult.ORDERED_NODE_SNAPSHOT_TYPE
-              );
-              const returnedNodes = [];
-              for (let i = 0; i < xPathResult.snapshotLength; i++) {
-                returnedNodes.push(xPathResult.snapshotItem(i));
-              }
+              const locateNodesUsingXpath = (element: HTMLElement) => {
+                const xPathResult = expression.evaluate(
+                  element,
+                  XPathResult.ORDERED_NODE_SNAPSHOT_TYPE
+                );
+                const returnedNodes = [];
+                for (let i = 0; i < xPathResult.snapshotLength; i++) {
+                  returnedNodes.push(xPathResult.snapshotItem(i));
+                }
+                return returnedNodes;
+              };
+              startNodes = startNodes.length > 0 ? startNodes : [document.body];
+              const returnedNodes = startNodes
+                .map((startNode) =>
+                  // TODO: stop search early if `maxNodeCount` is reached.
+                  locateNodesUsingXpath(startNode)
+                )
+                .flat(1);
               return maxNodeCount === 0
                 ? returnedNodes
                 : returnedNodes.slice(0, maxNodeCount);
@@ -1086,6 +1113,8 @@ export class BrowsingContextImpl {
             {type: 'string', value: locator.value},
             // `maxNodeCount` with `0` means no limit.
             {type: 'number', value: maxNodeCount ?? 0},
+            // `startNodes`
+            ...startNodes,
           ],
         };
       case 'innerText':
@@ -1101,7 +1130,8 @@ export class BrowsingContextImpl {
               innerTextSelector: string,
               fullMatch: boolean,
               ignoreCase: boolean,
-              maxNodeCount: number
+              maxNodeCount: number,
+              ...startNodes: HTMLElement[]
             ) => {
               const searchText = ignoreCase
                 ? innerTextSelector.toUpperCase()
@@ -1143,13 +1173,21 @@ export class BrowsingContextImpl {
                   }
                 }
                 // TODO: stop search early if `maxNodeCount` is reached.
-                return maxNodeCount === 0
-                  ? returnedNodes
-                  : returnedNodes.slice(0, maxNodeCount);
+                return returnedNodes;
               };
               // TODO: add maxDepth.
               // TODO: provide proper start node.
-              return locateNodesUsingInnerText(document.body);
+              // TODO: stop search early if `maxNodeCount` is reached.
+              startNodes = startNodes.length > 0 ? startNodes : [document.body];
+              const returnedNodes = startNodes
+                .map((startNode) =>
+                  // TODO: stop search early if `maxNodeCount` is reached.
+                  locateNodesUsingInnerText(startNode)
+                )
+                .flat(1);
+              return maxNodeCount === 0
+                ? returnedNodes
+                : returnedNodes.slice(0, maxNodeCount);
             }
           ),
           argumentsLocalValues: [
@@ -1161,6 +1199,8 @@ export class BrowsingContextImpl {
             {type: 'boolean', value: locator.ignoreCase === true},
             // `maxNodeCount` with `0` means no limit.
             {type: 'number', value: maxNodeCount ?? 0},
+            // `startNodes`
+            ...startNodes,
           ],
         };
     }
@@ -1169,9 +1209,14 @@ export class BrowsingContextImpl {
   async #locateNodesByLocator(
     realm: Realm,
     locator: BrowsingContext.Locator,
+    startNodes: Script.SharedReference[],
     maxNodeCount?: number
   ): Promise<BrowsingContext.LocateNodesResult> {
-    const locatorDelegate = this.#getLocatorDelegate(locator, maxNodeCount);
+    const locatorDelegate = this.#getLocatorDelegate(
+      locator,
+      maxNodeCount,
+      startNodes
+    );
     const locatorResult = await realm.callFunction(
       locatorDelegate.functionDeclaration,
       {type: 'undefined'},
@@ -1204,6 +1249,15 @@ export class BrowsingContextImpl {
           `Not valid selector ${locator.value}`
         );
       }
+      // Heuristic to detect if the `startNode` is not an `HTMLElement` in css selector.
+      if (
+        locatorResult.exceptionDetails.text ===
+        'Error: startNodes in css selector should be HTMLElement'
+      ) {
+        throw new InvalidArgumentException(
+          `startNodes in css selector should be HTMLElement`
+        );
+      }
       throw new UnknownErrorException(
         `Unexpected error in selector script: ${locatorResult.exceptionDetails.text}`
       );

diff --git a/...ta/chromedriver/headful/webdriver/tests/bidi/browsing_context/locate_nodes/invalid.py.ini b/...ta/chromedriver/headful/webdriver/tests/bidi/browsing_context/locate_nodes/invalid.py.ini
diff --git a/...medriver/headful/webdriver/tests/bidi/browsing_context/locate_nodes/max_node_count.py.ini b/...medriver/headful/webdriver/tests/bidi/browsing_context/locate_nodes/max_node_count.py.ini
diff --git a/...hromedriver/headful/webdriver/tests/bidi/browsing_context/locate_nodes/start_nodes.py.ini b/...hromedriver/headful/webdriver/tests/bidi/browsing_context/locate_nodes/start_nodes.py.ini
@@ -13,27 +13,3 @@
 
   [test_locate_with_multiple_context_nodes[xpath-.//div[@data-class='one'\]\]]
     expected: FAIL
-
-  [test_locate_with_multiple_context_nodes[innerText-foo\]]
-    expected: FAIL
-
-  [test_locate_with_context_nodes[css-p-expected0\]]
-    expected: FAIL
-
-  [test_locate_with_context_nodes[css-a span-expected1\]]
-    expected: FAIL
-
-  [test_locate_with_context_nodes[css-#text-expected2\]]
-    expected: FAIL
-
-  [test_locate_with_context_nodes[xpath-//p-expected3\]]
-    expected: FAIL
-
-  [test_locate_with_context_nodes[innerText-foo-expected4\]]
-    expected: FAIL
-
-  [test_locate_with_multiple_context_nodes[css-p[data-class='one'\]\]]
-    expected: FAIL
-
-  [test_locate_with_multiple_context_nodes[xpath-.//p[@data-class='one'\]\]]
-    expected: FAIL
diff --git a/...a/chromedriver/headless/webdriver/tests/bidi/browsing_context/locate_nodes/invalid.py.ini b/...a/chromedriver/headless/webdriver/tests/bidi/browsing_context/locate_nodes/invalid.py.ini
diff --git a/...edriver/headless/webdriver/tests/bidi/browsing_context/locate_nodes/max_node_count.py.ini b/...edriver/headless/webdriver/tests/bidi/browsing_context/locate_nodes/max_node_count.py.ini
diff --git a/...romedriver/headless/webdriver/tests/bidi/browsing_context/locate_nodes/start_nodes.py.ini b/...romedriver/headless/webdriver/tests/bidi/browsing_context/locate_nodes/start_nodes.py.ini
@@ -13,27 +13,3 @@
 
   [test_locate_with_multiple_context_nodes[xpath-.//div[@data-class='one'\]\]]
     expected: FAIL
-
-  [test_locate_with_multiple_context_nodes[innerText-foo\]]
-    expected: FAIL
-
-  [test_locate_with_context_nodes[css-p-expected0\]]
-    expected: FAIL
-
-  [test_locate_with_context_nodes[css-a span-expected1\]]
-    expected: FAIL
-
-  [test_locate_with_context_nodes[css-#text-expected2\]]
-    expected: FAIL
-
-  [test_locate_with_context_nodes[xpath-//p-expected3\]]
-    expected: FAIL
-
-  [test_locate_with_context_nodes[innerText-foo-expected4\]]
-    expected: FAIL
-
-  [test_locate_with_multiple_context_nodes[css-p[data-class='one'\]\]]
-    expected: FAIL
-
-  [test_locate_with_multiple_context_nodes[xpath-.//p[@data-class='one'\]\]]
-    expected: FAIL
diff --git a/...etadata/mapper/headless/webdriver/tests/bidi/browsing_context/locate_nodes/invalid.py.ini b/...etadata/mapper/headless/webdriver/tests/bidi/browsing_context/locate_nodes/invalid.py.ini
diff --git a/.../mapper/headless/webdriver/tests/bidi/browsing_context/locate_nodes/max_node_count.py.ini b/.../mapper/headless/webdriver/tests/bidi/browsing_context/locate_nodes/max_node_count.py.ini
diff --git a/...ata/mapper/headless/webdriver/tests/bidi/browsing_context/locate_nodes/start_nodes.py.ini b/...ata/mapper/headless/webdriver/tests/bidi/browsing_context/locate_nodes/start_nodes.py.ini
@@ -13,27 +13,3 @@
 
   [test_locate_with_multiple_context_nodes[xpath-.//div[@data-class='one'\]\]]
     expected: FAIL
-
-  [test_locate_with_multiple_context_nodes[innerText-foo\]]
-    expected: FAIL
-
-  [test_locate_with_context_nodes[css-p-expected0\]]
-    expected: FAIL
-
-  [test_locate_with_context_nodes[css-a span-expected1\]]
-    expected: FAIL
-
-  [test_locate_with_context_nodes[css-#text-expected2\]]
-    expected: FAIL
-
-  [test_locate_with_context_nodes[xpath-//p-expected3\]]
-    expected: FAIL
-
-  [test_locate_with_context_nodes[innerText-foo-expected4\]]
-    expected: FAIL
-
-  [test_locate_with_multiple_context_nodes[css-p[data-class='one'\]\]]
-    expected: FAIL
-
-  [test_locate_with_multiple_context_nodes[xpath-.//p[@data-class='one'\]\]]
-    expected: FAIL