diff --git a/Browser/browser.py b/Browser/browser.py index fa4d4c01e..24ff6ef57 100755 --- a/Browser/browser.py +++ b/Browser/browser.py @@ -444,7 +444,6 @@ class Browser(DynamicCore): _context_cache = ContextCache() _suite_cleanup_done = False _output_dir = "." - # Parameter names the Node side fills in for JavaScript extension functions. _js_injected_arguments = ( "logger", "playwright", @@ -514,29 +513,11 @@ def __init__( # noqa: PLR0915 ``playwright``, ``adoptContext`` and ``defaultTimeout`` are filled in by the library rather than taken from the keyword call. ``defaultTimeout`` is the library timeout in milliseconds. ``adoptContext(context, options)`` - registers a BrowserContext that the function created itself, for - example with ``playwright.chromium.launchPersistentContext`` or as - ``app.context()`` of an app from ``playwright._electron.launch``, as a - new active browser and makes its first page the active page, so the - other keywords work on it. It keeps its own default timeout; - ``context.setDefaultTimeout(defaultTimeout)`` gives it the library - timeout, like the contexts the library creates. It returns the new - browser and context ids, and the page id if the context has a page. - Closing that browser closes the context. The optional ``options`` are - ``name``, which `Get Browser Catalog` reports as the browser's type, - ``adopted`` by default; ``headless``, ``false`` by default; - ``onClose``, an async function that runs once after the context is - closed, so the function can release what the context does not own, - such as a device connection; ``tracing``, the absolute path of a - ``.zip`` trace file or of a folder, in which the trace is saved as - ``trace_.zip``; and ``contextOptions``, the options the - context was created with, in the form of Playwright's browser context - options. With ``tracing``, the library starts tracing the context and - saves the trace before it closes the context. With ``contextOptions``, - keywords that depend on them work on the context, such as `Download` - with ``acceptDownloads``. Can be a single path, a comma-separated list - of paths or a real list of strings. See - https://robotframework-browser.org/docs/extending/javascript-extensions + makes a BrowserContext that the function created itself, such as the + context of an Electron app, the active browser, so the other keywords + work on it. Can be a single path, a comma-separated list of paths or a + real list of strings. The options of ``adoptContext`` and examples are + in https://robotframework-browser.org/docs/extending/javascript-extensions - ``language``: Defines language which is used to translate keyword names and documentation. - ``playwright_process_host``: Hostname / Host address which should be used @@ -836,7 +817,6 @@ def _jskeyword_call( if arg_name in self._js_injected_arguments: arg_set_texts.append(f'("{arg_name}", "RESERVED")') elif arg_name == "defaultTimeout": - # The library timeout at the time of the call. arg_set_texts.append('("defaultTimeout", int(self.timeout))') else: arg_set_texts.append(f'("{arg_name}", {arg_name})') diff --git a/node/playwright-wrapper/playwright-state.ts b/node/playwright-wrapper/playwright-state.ts index 022ea280d..8f349f212 100644 --- a/node/playwright-wrapper/playwright-state.ts +++ b/node/playwright-wrapper/playwright-state.ts @@ -166,8 +166,6 @@ export async function extensionKeywordCall( const namedArguments = Object.fromEntries(args['arguments']); const apiArguments = new Map(); const argNames = getArgumentNamesFromJavascriptKeyword(keyword); - // Resolve the browser only when the keyword asks for something that needs - // one. So if it has arguments like page, context or browser. if (argNames.some((name) => name === 'page' || name === 'context' || name === 'browser')) { apiArguments.set('browser', state.getActiveBrowser().browser); } @@ -178,8 +176,7 @@ export async function extensionKeywordCall( apiArguments.set('adoptContext', (context: BrowserContext, options?: AdoptContextOptions) => state.adoptContext(context, options), ); - // Injected values can be null or undefined, for example the browser of a - // persistent or adopted context, so they must not fall back to the marker. + // Injected values can be null, for example the browser of an adopted context. const functionArguments = argNames.map((argName) => apiArguments.has(argName) ? apiArguments.get(argName) : namedArguments[argName], ); @@ -330,10 +327,7 @@ async function _createIndexedContext( return indexedContext; } -/* - * Indexes a context and every page it already has. The context's first page - * ends up last in the page stack, so it becomes the active page. - */ +// The context's first page ends up last in the page stack, so it becomes the active page. async function _indexContextWithPages( context: BrowserContext, defaultTimeout: number | undefined, @@ -563,18 +557,6 @@ export class PlaywrightState { return browserState; } - /* - * Adds a context that was created outside the library, for example by a - * JavaScript extension, as a new active browser without a browser object. - * Its first page becomes the active page. It keeps its default timeout; an - * extension gives it the library timeout with the injected defaultTimeout. - * Closing the context closes the browser, as with a persistent context. options.onClose runs once after that, - * so the creator can release what the context does not own, such as a device. - * The browser is named 'adopted' and not headless unless the creator says otherwise. - * With options.tracing, the context is traced like one created with a trace file. - * options.contextOptions are the options the context was created with, kept like - * those of New Context, so that keywords depending on them, such as Download, work. - */ public async adoptContext(context: BrowserContext, options: AdoptContextOptions = {}): Promise { const browserState = new BrowserState({ browser: null, @@ -875,7 +857,6 @@ export async function closeContext(request: Request_Bool, openBrowsers: Playwrig await _saveCoverageReport(page); } // A persistent or adopted context is the only context of a browser without a browser object. - // Closing it closes that browser too, which runs its onClose, even if closing the context fails. const closesBrowser = activeBrowser.browser === null && activeBrowser.contextStack.length === 1; try { await openBrowsers.getActiveContext()?.close(); diff --git a/utest/test_js_extension_call.py b/utest/test_js_extension_call.py index 4f8398e06..214854e05 100644 --- a/utest/test_js_extension_call.py +++ b/utest/test_js_extension_call.py @@ -2,14 +2,14 @@ import json from contextlib import contextmanager from datetime import timedelta +from types import SimpleNamespace from Browser import Browser from Browser.base import LibraryComponent +from Browser.utils.data_types import Scope class RecordingPlaywright: - """Stands in for the Node process and records the extension calls.""" - def __init__(self): self.requests = [] @@ -21,6 +21,9 @@ def CallExtensionKeyword(self, request): # noqa: N802 - the name of the gRPC me self.requests.append(request) return [] + def SetTimeout(self, request): # noqa: N802 - the name of the gRPC method + return SimpleNamespace(log="") + def payload(self) -> dict: return json.loads(self.requests[-1].arguments) @@ -57,9 +60,8 @@ def test_generated_keyword_passes_the_library_timeout_as_default_timeout( def test_generated_keyword_reads_the_library_timeout_at_each_call(tmpdir, monkeypatch): library, playwright = library_with_recorded_calls(tmpdir, monkeypatch) keyword = generated_keyword(library, "openOwnContext", "defaultTimeout") - library.scope_stack["timeout"].setter_function = None - library.scope_stack["timeout"].set(3000.0) + library.set_browser_timeout(timedelta(seconds=3), Scope.Global) keyword() assert playwright.payload() == {"arguments": [["defaultTimeout", 3000]]}