From b6637c16be66622a32067e5c64130ae44d7e1244 Mon Sep 17 00:00:00 2001 From: Maksim Sadym <69349599+sadym-chromium@users.noreply.github.com> Date: Thu, 29 Aug 2024 14:19:40 +0200 Subject: [PATCH] fix: css and xPath selector start node is document (#2543) Addressing https://github.com/GoogleChromeLabs/chromium-bidi/issues/2539 According to the spec, the default `startNodes` of the `browsingContext.locateNodes` is `document`, not `document.body`. --- .../modules/context/BrowsingContextImpl.ts | 11 +++- tests/browsing_context/test_locate_nodes.py | 66 ++++++++++++++++++- 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/src/bidiMapper/modules/context/BrowsingContextImpl.ts b/src/bidiMapper/modules/context/BrowsingContextImpl.ts index c59a17118d..15997c44dd 100644 --- a/src/bidiMapper/modules/context/BrowsingContextImpl.ts +++ b/src/bidiMapper/modules/context/BrowsingContextImpl.ts @@ -1255,7 +1255,7 @@ export class BrowsingContextImpl { return [...element.querySelectorAll(cssSelector)]; }; - startNodes = startNodes.length > 0 ? startNodes : [document.body]; + startNodes = startNodes.length > 0 ? startNodes : [document]; const returnedNodes = startNodes .map((startNode) => // TODO: stop search early if `maxNodeCount` is reached. @@ -1298,7 +1298,7 @@ export class BrowsingContextImpl { } return returnedNodes; }; - startNodes = startNodes.length > 0 ? startNodes : [document.body]; + startNodes = startNodes.length > 0 ? startNodes : [document]; const returnedNodes = startNodes .map((startNode) => // TODO: stop search early if `maxNodeCount` is reached. @@ -1530,7 +1530,12 @@ export class BrowsingContextImpl { } } - startNodes = startNodes.length > 0 ? startNodes : [document.body]; + startNodes = + startNodes.length > 0 + ? startNodes + : Array.from(document.documentElement.children).filter( + (c) => c instanceof HTMLElement + ); collect(startNodes, { role, name, diff --git a/tests/browsing_context/test_locate_nodes.py b/tests/browsing_context/test_locate_nodes.py index 22c042b20e..b238f7cacc 100644 --- a/tests/browsing_context/test_locate_nodes.py +++ b/tests/browsing_context/test_locate_nodes.py @@ -15,7 +15,7 @@ import re import pytest -from test_helpers import ANY_SHARED_ID, execute_command, goto_url +from test_helpers import ANY_SHARED_ID, AnyExtending, execute_command, goto_url @pytest.mark.parametrize('locator', [ @@ -143,3 +143,67 @@ async def test_locate_nodes_locator_invalid(websocket, context_id, html, 'locator': locator } }) + + +@pytest.mark.parametrize('locator', [ + { + 'type': 'css', + 'value': 'html' + }, + { + 'type': 'xpath', + 'value': 'html' + }, +]) +@pytest.mark.parametrize('start_node_expression,should_find', + [(None, True), ('document', True), + ('document.body', False)]) +@pytest.mark.asyncio +async def test_locate_nodes_select_html(websocket, context_id, html, locator, + start_node_expression, should_find): + await goto_url( + websocket, context_id, + html( + '
foobarBARbaz
foobarBARbaz
' + )) + + start_nodes = None + if start_node_expression: + resp = await execute_command( + websocket, { + 'method': 'script.evaluate', + 'params': { + 'expression': start_node_expression, + 'target': { + 'context': context_id + }, + 'awaitPromise': False + } + }) + start_nodes = [{'sharedId': resp['result']['sharedId']}] + + resp = await execute_command( + websocket, { + 'method': 'browsingContext.locateNodes', + 'params': { + 'context': context_id, + 'locator': locator, + **({ + 'startNodes': start_nodes + } if start_nodes else {}) + } + }) + + if should_find: + assert resp == { + "nodes": [ + AnyExtending({ + 'type': 'node', + 'value': { + 'localName': 'html', + } + }) + ] + } + else: + assert resp == {"nodes": []}