From cf107fca92edcba0260e16047513acfce37c85b4 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Mon, 17 Aug 2026 14:37:39 -0700 Subject: [PATCH 1/4] fix(chromium): dispose worker sessions when their frame session is disposed (#42282) --- .../src/server/chromium/crPage.ts | 21 +++++++++----- tests/library/chromium/oopif.spec.ts | 29 +++++++++++++++++++ 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/packages/playwright-core/src/server/chromium/crPage.ts b/packages/playwright-core/src/server/chromium/crPage.ts index 52aab753da552..f53e0ecec637c 100644 --- a/packages/playwright-core/src/server/chromium/crPage.ts +++ b/packages/playwright-core/src/server/chromium/crPage.ts @@ -580,6 +580,8 @@ class FrameSession { this._firstNonInitialNavigationCommittedReject(new TargetClosedError(this._page.closeReason())); for (const childSession of this._childSessions) childSession.dispose(); + for (const sessionId of this._workerSessions.keys()) + this._removeWorkerSession(sessionId); if (this._parentSession) this._parentSession._childSessions.delete(this); eventsHelper.removeEventListeners(this._eventListeners); @@ -780,16 +782,21 @@ class FrameSession { session.on('Runtime.exceptionThrown', exception => this._page.addPageError(exceptionToError(exception.exceptionDetails), stackTraceToLocation(exception.exceptionDetails.stackTrace))); } + private _removeWorkerSession(sessionId: string): boolean { + const workerSession = this._workerSessions.get(sessionId); + if (!workerSession) + return false; + this._workerSessions.delete(sessionId); + this._crPage._networkManager.removeSession(workerSession); + workerSession.dispose(); + this._page.removeWorker(sessionId); + return true; + } + _onDetachedFromTarget(event: Protocol.Target.detachedFromTargetPayload) { // This might be a worker... - const workerSession = this._workerSessions.get(event.sessionId); - if (workerSession) { - this._workerSessions.delete(event.sessionId); - this._crPage._networkManager.removeSession(workerSession); - workerSession.dispose(); - this._page.removeWorker(event.sessionId); + if (this._removeWorkerSession(event.sessionId)) return; - } // ... or an oopif. const childFrameSession = this._crPage._sessions.get(event.targetId!); diff --git a/tests/library/chromium/oopif.spec.ts b/tests/library/chromium/oopif.spec.ts index a8971fe638a4c..2a9f525472778 100644 --- a/tests/library/chromium/oopif.spec.ts +++ b/tests/library/chromium/oopif.spec.ts @@ -15,6 +15,7 @@ */ import { contextTest as it, expect } from '../../config/browserTest'; +import { attachFrame } from '../../config/utils'; import type { Frame, Browser } from '@playwright/test'; it.use({ @@ -43,6 +44,34 @@ it('should handle oopif detach', async function({ page, browser, server }) { expect(detachedFrame).toBe(frame); }); +it('should remove workers of a detached oopif', async function({ page, browser, server }) { + await page.goto(server.EMPTY_PAGE); + const [worker] = await Promise.all([ + page.waitForEvent('worker'), + attachFrame(page, 'frame1', server.CROSS_PROCESS_PREFIX + '/worker/worker.html'), + ]); + await assertOOPIFCount(browser, 1); + expect(page.workers().length).toBe(1); + await Promise.all([ + worker.waitForEvent('close'), + page.goto(server.PREFIX + '/title.html'), + ]); + expect(page.workers().length).toBe(0); +}); + +it('should not hang in unrouteAll when oopif worker is gone', async function({ page, context, browser, server }) { + it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/42278' }); + await context.route('**/*', route => route.continue()); + await page.goto(server.EMPTY_PAGE); + await Promise.all([ + page.waitForEvent('worker'), + attachFrame(page, 'frame1', server.CROSS_PROCESS_PREFIX + '/worker/worker.html'), + ]); + await assertOOPIFCount(browser, 1); + await page.goto(server.CROSS_PROCESS_PREFIX + '/title.html'); + await context.unrouteAll(); +}); + it('should handle remote -> local -> remote transitions', async function({ page, browser, server }) { await page.goto(server.PREFIX + '/dynamic-oopif.html'); expect(page.frames().length).toBe(2); From 04fb72b4f0f77e50c88689d436980f68f4c40a98 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Mon, 17 Aug 2026 22:47:21 +0100 Subject: [PATCH 2/4] chore(highlight): resolve highlights periodically in HighlightController (#42277) --- packages/injected/src/highlight.ts | 23 ++-- packages/injected/src/injectedScript.ts | 11 +- .../src/server/debugController.ts | 2 +- .../src/server/dispatchers/frameDispatcher.ts | 4 +- .../src/server/dispatchers/pageDispatcher.ts | 2 +- .../src/server/frameSelectors.ts | 10 +- packages/playwright-core/src/server/frames.ts | 22 +--- .../src/server/highlightController.ts | 117 ++++++++++++++++++ packages/playwright-core/src/server/page.ts | 14 +-- .../playwright-core/src/server/recorder.ts | 20 +-- .../src/server/screenshotter.ts | 4 +- packages/trace-viewer/src/ui/snapshotTab.tsx | 15 +-- tests/library/debug-controller.spec.ts | 3 +- tests/library/locator-highlight.spec.ts | 18 +++ 14 files changed, 187 insertions(+), 78 deletions(-) create mode 100644 packages/playwright-core/src/server/highlightController.ts diff --git a/packages/injected/src/highlight.ts b/packages/injected/src/highlight.ts index 0c0054f00a53f..6d1d9f04616b6 100644 --- a/packages/injected/src/highlight.ts +++ b/packages/injected/src/highlight.ts @@ -64,7 +64,7 @@ export class Highlight { private _injectedScript: InjectedScript; private _rafRequest: number | undefined; private _language: Language = 'javascript'; - private _elementHighlightSelectors = new Map(); + private _elementHighlights: { selector: ParsedSelector, cssStyle?: string }[] = []; constructor(injectedScript: InjectedScript) { this._injectedScript = injectedScript; @@ -126,17 +126,12 @@ export class Highlight { this._language = language; } - addElementHighlight(selector: ParsedSelector, cssStyle?: string) { - const key = stringifySelector(selector); - this._elementHighlightSelectors.set(key, { selector, cssStyle }); - this._ensureElementHighlightRaf(); - } - - removeElementHighlight(selector: ParsedSelector) { - const key = stringifySelector(selector); - if (!this._elementHighlightSelectors.delete(key)) - return; - if (this._elementHighlightSelectors.size === 0) { + setElementHighlights(highlights: { selector: ParsedSelector, cssStyle?: string }[]) { + const hadHighlights = this._elementHighlights.length > 0; + this._elementHighlights = highlights; + if (this._elementHighlights.length) { + this._ensureElementHighlightRaf(); + } else if (hadHighlights) { if (this._rafRequest) { this._injectedScript.utils.builtins.cancelAnimationFrame(this._rafRequest); this._rafRequest = undefined; @@ -151,7 +146,7 @@ export class Highlight { const tick = () => { const entries: HighlightEntry[] = []; const glassPanes = [...this._injectedScript.document.querySelectorAll('x-pw-glass')]; - for (const { selector, cssStyle } of this._elementHighlightSelectors.values()) { + for (const { selector, cssStyle } of this._elementHighlights) { let elements: Element[] = []; try { elements = this._injectedScript.querySelectorAll(selector, this._injectedScript.document.documentElement); @@ -178,7 +173,7 @@ export class Highlight { this._injectedScript.utils.builtins.cancelAnimationFrame(this._rafRequest); this._rafRequest = undefined; } - this._elementHighlightSelectors.clear(); + this._elementHighlights = []; this._glassPaneElement.remove(); } diff --git a/packages/injected/src/injectedScript.ts b/packages/injected/src/injectedScript.ts index 3d5f52805665c..674c605f8851f 100644 --- a/packages/injected/src/injectedScript.ts +++ b/packages/injected/src/injectedScript.ts @@ -1344,14 +1344,11 @@ export class InjectedScript { return this._highlight; } - addHighlight(selector: ParsedSelector, style?: string) { - const highlight = this._ensureHighlight(); - highlight.addElementHighlight(selector, style); - } - - removeHighlight(selector: ParsedSelector) { + setHighlights(highlights: { selector: ParsedSelector, cssStyle?: string }[]) { + if (!highlights.length && !this._highlight) + return; const highlight = this._ensureHighlight(); - highlight.removeElementHighlight(selector); + highlight.setElementHighlights(highlights); } setScreencastAnnotation(annotation: { point?: Point, box?: Rect, actionTitle?: string, duration?: number, position?: string, fontSize?: number, cursor?: 'none' | 'pointer' } | null) { diff --git a/packages/playwright-core/src/server/debugController.ts b/packages/playwright-core/src/server/debugController.ts index 99b51cb77f0c6..04779ddde2e5a 100644 --- a/packages/playwright-core/src/server/debugController.ts +++ b/packages/playwright-core/src/server/debugController.ts @@ -133,7 +133,7 @@ export class DebugController extends SdkObject { for (const recorder of await progress.race(this._allRecorders())) promises.push(recorder.hideHighlightedSelector()); // Hide all locator.highlight highlights. - promises.push(...this._playwright.allPages().map(p => p.hideHighlight().catch(() => {}))); + promises.push(...this._playwright.allPages().map(p => p.highlightController.hideHighlights().catch(() => {}))); await progress.race(Promise.all(promises)); } diff --git a/packages/playwright-core/src/server/dispatchers/frameDispatcher.ts b/packages/playwright-core/src/server/dispatchers/frameDispatcher.ts index e05d6d34a5be1..42352d12f2a7c 100644 --- a/packages/playwright-core/src/server/dispatchers/frameDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/frameDispatcher.ts @@ -272,11 +272,11 @@ export class FrameDispatcher extends Dispatcher { - return await progress.race(this._frame.addHighlight(params.selector, params.style)); + return await progress.race(this._frame._page.highlightController.addHighlight(params.selector, { style: params.style })); } async hideHighlight(params: channels.FrameHideHighlightParams, progress: Progress): Promise { - return await progress.race(this._frame.removeHighlight(params.selector)); + return await progress.race(this._frame._page.highlightController.removeHighlight(params.selector)); } async expect(params: channels.FrameExpectParams, progress: Progress): Promise { diff --git a/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts b/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts index dcf92c5a970be..69bb01f0a8d28 100644 --- a/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/pageDispatcher.ts @@ -366,7 +366,7 @@ export class PageDispatcher extends Dispatcher { - await progress.race(this._page.hideHighlight()); + await progress.race(this._page.highlightController.hideHighlights()); } async screencastShowOverlay(params: channels.PageScreencastShowOverlayParams): Promise { diff --git a/packages/playwright-core/src/server/frameSelectors.ts b/packages/playwright-core/src/server/frameSelectors.ts index c585a1229458c..3b254611c77d8 100644 --- a/packages/playwright-core/src/server/frameSelectors.ts +++ b/packages/playwright-core/src/server/frameSelectors.ts @@ -120,8 +120,8 @@ export class FrameSelectors { return jumptToFrame; } - private async _resolveFramesForSelector(selector: string, options: types.StrictOptions & { noDefaultPierce?: boolean } = {}, scope?: ElementHandle): Promise { - const pierceByDefault = !!this.frame._page.browserContext._options.pierceFrames && !options.noDefaultPierce; + async resolveFramesForSelector(selector: string, options: types.StrictOptions & { pierce?: 'default' | 'pierce' | 'no-pierce' } = {}, scope?: ElementHandle): Promise { + const pierceByDefault = options.pierce === 'pierce' || (options.pierce !== 'no-pierce' && !!this.frame._page.browserContext._options.pierceFrames); const { pierce, chunks } = splitSelectorByFrame(selector, pierceByDefault); for (const chunk of chunks) { visitAllSelectorParts(chunk, (part, nested) => { @@ -280,12 +280,12 @@ export class FrameSelectors { private async _callOnSelectorInternal( selector: string, - options: types.StrictOptions & { mainWorld?: boolean, callWithoutMatches?: boolean, scope?: ElementHandle, markTargets?: 'all' | 'first' | 'none', noDefaultPierce?: boolean }, + options: types.StrictOptions & { mainWorld?: boolean, callWithoutMatches?: boolean, scope?: ElementHandle, markTargets?: 'all' | 'first' | 'none', pierce?: 'default' | 'pierce' | 'no-pierce' }, pageFunction: MatchedElementsCallback, arg: Arg, returnByValue: boolean, ): Promise<{ frame: Frame, info: SelectorInfo, result: R | SmartHandle } | null> { - const resolved = await this._resolveFramesForSelector(selector, options, options.scope); + const resolved = await this.resolveFramesForSelector(selector, options, options.scope); let aggregatedResult: { frame: Frame, info: SelectorInfo, result: R | SmartHandle } | null = null; const noStall = resolved.length > 1; for (const { frame, info, scope } of resolved) { @@ -336,7 +336,7 @@ export class FrameSelectors { async callOnSelector( selector: string, - options: types.StrictOptions & { mainWorld?: boolean, callWithoutMatches?: boolean, scope?: ElementHandle, markTargets?: 'all' | 'first' | 'none', noDefaultPierce?: boolean }, + options: types.StrictOptions & { mainWorld?: boolean, callWithoutMatches?: boolean, scope?: ElementHandle, markTargets?: 'all' | 'first' | 'none', pierce?: 'default' | 'pierce' | 'no-pierce' }, pageFunction: MatchedElementsCallback, arg: Arg, ): Promise<{ frame: Frame, info: SelectorInfo, result: R } | null> { diff --git a/packages/playwright-core/src/server/frames.ts b/packages/playwright-core/src/server/frames.ts index e5bd06fe2ee16..c146615162eb5 100644 --- a/packages/playwright-core/src/server/frames.ts +++ b/packages/playwright-core/src/server/frames.ts @@ -1373,26 +1373,6 @@ export class Frame extends SdkObject { return result; } - async addHighlight(selector: string, style?: string) { - await this.selectors.callOnSelector(selector, { strict: false, callWithoutMatches: true }, ({ injected, info }, style) => { - return injected.addHighlight(info.parsed, style); - }, style); - } - - async removeHighlight(selector: string) { - await this.selectors.callOnSelector(selector, { strict: false, callWithoutMatches: true }, ({ injected, info }) => { - return injected.removeHighlight(info.parsed); - }, {}); - } - - async hideHighlight() { - return this.raceAgainstEvaluationStallingEvents(async () => { - const context = this._contextData.get('utility')?.context; - const injectedScript = await context?.injectedScript(); - await injectedScript?.evaluate(injected => injected.hideHighlight()); - }); - } - private async _elementState(progress: Progress, selector: string, state: ElementStateWithoutStable, options: types.QueryOnSelectorOptions, scope?: dom.ElementHandle): Promise { const { result } = await this._waitForFunctionOnSelector(progress, selector, (injected, element, data) => { return { result: injected.elementState(element, data.state) }; @@ -1568,7 +1548,7 @@ export class Frame extends SdkObject { let missingReceived = false; // Non-array expectations are strict (callOnSelector throws on multiple); array ones are not. - const resolved = await progress.race(this.selectors.callOnSelector(effectiveSelector, { strict: !isArray, mainWorld, markTargets: 'all', noDefaultPierce: !selector }, async ({ injected, elements }, options) => { + const resolved = await progress.race(this.selectors.callOnSelector(effectiveSelector, { strict: !isArray, mainWorld, markTargets: 'all', pierce: selector ? 'default' : 'no-pierce' }, async ({ injected, elements }, options) => { const isArray = options.expression === 'to.have.count' || options.expression.endsWith('.array'); const log = isArray ? ` locator resolved to ${elements.length} element${elements.length === 1 ? '' : 's'}` diff --git a/packages/playwright-core/src/server/highlightController.ts b/packages/playwright-core/src/server/highlightController.ts new file mode 100644 index 0000000000000..be2db046252f1 --- /dev/null +++ b/packages/playwright-core/src/server/highlightController.ts @@ -0,0 +1,117 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { Frame } from './frames'; +import type { Page } from './page'; +import type { ParsedSelector } from '@isomorphic/selectorParser'; + +export type HighlightOptions = { + style?: string; + pierce?: boolean; // Highlight in all the frames the selector could resolve to, instead of a single one. +}; + +type HighlightEntry = HighlightOptions & { + selector: string; +}; + +export class HighlightController { + private _page: Page; + private _entries = new Map(); + private _resolutionTimer: NodeJS.Timeout | undefined; + private _resolutionChain: Promise = Promise.resolve(); + + constructor(page: Page) { + this._page = page; + } + + async addHighlight(selector: string, options: HighlightOptions = {}) { + // Validate the selector upfront, so that the caller gets a synchronous error. + this._page.browserContext.selectors().parseSelector(selector, false); + this._entries.set(selector, { selector, ...options }); + await this._resolveNow(); + } + + async removeHighlight(selector: string) { + this._entries.delete(selector); + await this._resolveNow(); + } + + dispose() { + if (this._resolutionTimer) { + clearTimeout(this._resolutionTimer); + this._resolutionTimer = undefined; + } + } + + async hideHighlights() { + this._entries.clear(); + await Promise.all(this._page.frames().map(frame => frame.raceAgainstEvaluationStallingEvents(async () => { + const context = frame.existingContext('utility'); + const injectedScript = await context?.injectedScript(); + await injectedScript?.evaluate(injected => injected.hideHighlight()); + }).catch(() => {}))); + } + + private _resolveNow(): Promise { + if (this._resolutionTimer) { + clearTimeout(this._resolutionTimer); + this._resolutionTimer = undefined; + } + this._resolutionChain = this._resolutionChain.then(() => this._resolve()).catch(() => {}); + return this._resolutionChain; + } + + private async _resolve() { + if (this._page.isClosed()) + return; + + const perFrame = new Map(); + for (const entry of this._entries.values()) { + const results = await this._resolveEntry(entry); + for (const { frame, info } of results) { + let list = perFrame.get(frame); + if (!list) { + list = []; + perFrame.set(frame, list); + } + list.push({ selector: info.parsed, cssStyle: entry.style }); + } + } + + await Promise.all(this._page.frames().map(async frame => { + const highlights = perFrame.get(frame) || []; + await frame.raceAgainstEvaluationStallingEvents(async () => { + const context = frame.existingContext('utility'); + const injectedScript = await context?.injectedScript(); + await injectedScript?.evaluate((injected, highlights) => injected.setHighlights(highlights), highlights); + }).catch(() => {}); + })); + + if (this._entries.size && !this._resolutionTimer && !this._page.isClosed()) + this._resolutionTimer = setTimeout(() => this._resolveNow(), 1000); + } + + private async _resolveEntry(entry: HighlightEntry) { + try { + return await this._page.mainFrame().selectors.resolveFramesForSelector(entry.selector, { strict: false, pierce: entry.pierce ? 'pierce' : 'default' }); + } catch (error) { + if (!entry.pierce) + return []; + // Some selectors do not support piercing frames, e.g. composite ones - resolve without piercing. + return await this._page.mainFrame().selectors.resolveFramesForSelector(entry.selector, { strict: false }).catch(() => []); + } + } +} diff --git a/packages/playwright-core/src/server/page.ts b/packages/playwright-core/src/server/page.ts index ea8874fa9cf4d..82befdb710c41 100644 --- a/packages/playwright-core/src/server/page.ts +++ b/packages/playwright-core/src/server/page.ts @@ -35,6 +35,7 @@ import * as input from './input'; import { SdkObject } from './instrumentation'; import * as js from './javascript'; import { Screenshotter, validateScreenshotOptions } from './screenshotter'; +import { HighlightController } from './highlightController'; import { compressCallLog } from './callLog'; import * as rawBindingsControllerSource from '../generated/bindingsControllerSource'; import { Overlay } from './overlay'; @@ -186,6 +187,7 @@ export class Page extends SdkObject { private readonly _pageBindings = new Map(); initScripts: InitScript[] = []; readonly screenshotter: Screenshotter; + readonly highlightController: HighlightController; readonly frameManager: frames.FrameManager; private _workers = new Map(); readonly pdf: ((options: channels.PagePdfParams) => Promise) | undefined; @@ -213,6 +215,7 @@ export class Page extends SdkObject { this.mouse = new input.Mouse(delegate.rawMouse, this); this.touchscreen = new input.Touchscreen(delegate.rawTouchscreen, this); this.screenshotter = new Screenshotter(this); + this.highlightController = new HighlightController(this); this.frameManager = new frames.FrameManager(this); this.overlay = new Overlay(this); this.screencast = new Screencast(this); @@ -300,6 +303,7 @@ export class Page extends SdkObject { this.frameManager.dispose(error); this.screencast.dispose(); this.overlay.dispose(); + this.highlightController.dispose(); this.openScope.close(error); } @@ -935,10 +939,6 @@ export class Page extends SdkObject { })); } - async hideHighlight() { - await Promise.all(this.frames().map(frame => frame.hideHighlight().catch(() => {}))); - } - async setDockTile(image: Buffer) { await this.delegate.setDockTile(image); } @@ -1112,13 +1112,13 @@ export class InitScript extends DisposableObject { } } -export async function ariaSnapshotJSONForFrame(progress: Progress, frame: frames.Frame, selector: string | undefined, options: { mode?: 'ai' | 'default', doNotRenderActive?: boolean, depth?: number, boxes?: boolean, strict?: boolean, noDefaultPierce?: boolean } = {}): Promise { +export async function ariaSnapshotJSONForFrame(progress: Progress, frame: frames.Frame, selector: string | undefined, options: { mode?: 'ai' | 'default', doNotRenderActive?: boolean, depth?: number, boxes?: boolean, strict?: boolean, pierce?: 'default' | 'pierce' | 'no-pierce' } = {}): Promise { const snapshot = await frame.retryWithProgressAndTimeouts(progress, [1000, 2000, 4000, 8000], async (progress, continuePolling) => { try { // Note: the resolved frame might differ from the original |frame|. // See https://developer.mozilla.org/en-US/docs/Web/API/Document/body for body/frameset explanation. // Non-strict, because pages with nested framesets have multiple "frameset" elements. - const resolved = await progress.race(frame.selectors.callOnSelector(selector || 'body,frameset', { strict: options.strict ?? !!selector, noDefaultPierce: !selector || options.noDefaultPierce }, ({ injected, elements }, ariaOptions) => { + const resolved = await progress.race(frame.selectors.callOnSelector(selector || 'body,frameset', { strict: options.strict ?? !!selector, pierce: selector ? options.pierce : 'no-pierce' }, ({ injected, elements }, ariaOptions) => { return injected.ariaSnapshotJSON(elements[0], ariaOptions); }, { mode: options.mode ?? 'default', @@ -1148,7 +1148,7 @@ export async function ariaSnapshotJSONForFrame(progress: Progress, frame: frames // Non-strict, because child frameset documents have multiple "frameset" elements. const frameRootSelector = `aria-ref=${ref} >> internal:control=enter-frame >> body,frameset`; try { - return await ariaSnapshotJSONForFrame(progress, snapshot.resolvedFrame, frameRootSelector, { ...options, depth: childDepth, strict: false, noDefaultPierce: true }); + return await ariaSnapshotJSONForFrame(progress, snapshot.resolvedFrame, frameRootSelector, { ...options, depth: childDepth, strict: false, pierce: 'no-pierce' }); } catch { return []; } diff --git a/packages/playwright-core/src/server/recorder.ts b/packages/playwright-core/src/server/recorder.ts index 32e576a109cec..5080e11dac0d3 100644 --- a/packages/playwright-core/src/server/recorder.ts +++ b/packages/playwright-core/src/server/recorder.ts @@ -76,7 +76,7 @@ export class Recorder extends EventEmitter implements Instrume private _context: BrowserContext; private _params: RecorderParams; private _mode: Mode; - private _highlightedSelector: string | undefined; + private _highlightedSelector: { selector: string, pierce: boolean } | undefined; private _overlayState: OverlayState = { offsetX: 0 }; private _currentCallsMetadata = new Map(); private _actionPoints = new Map(); @@ -304,11 +304,11 @@ export class Recorder extends EventEmitter implements Instrume async setHighlightedSelector(selector: string) { const converted = locatorOrSelectorAsSelector(this._currentLanguage, selector, this._context.selectors().testIdAttributeName()); - await this._updateHighlightedSelector(converted || undefined); + await this._updateHighlightedSelector(converted || undefined, true /* pierce */); } async setHighlightedAriaTemplate(ariaTemplate: AriaTemplateNode) { - await this._updateHighlightedSelector('aria-template=' + JSON.stringify(ariaTemplate)); + await this._updateHighlightedSelector('aria-template=' + JSON.stringify(ariaTemplate), true /* pierce */); } step() { @@ -358,16 +358,18 @@ export class Recorder extends EventEmitter implements Instrume return this._callLogs; } - private async _updateHighlightedSelector(selector: string | undefined) { + private async _updateHighlightedSelector(selector: string | undefined, pierce = false) { const previous = this._highlightedSelector; - if (previous === selector) + if (!previous && !selector) return; - this._highlightedSelector = selector; + if (previous && previous.selector === selector && previous.pierce === pierce) + return; + this._highlightedSelector = selector === undefined ? undefined : { selector, pierce }; await Promise.all(this._context.pages().map(async page => { if (previous) - await page.mainFrame().removeHighlight(previous).catch(() => {}); + await page.highlightController.removeHighlight(previous.selector).catch(() => {}); if (selector) - await page.mainFrame().addHighlight(selector).catch(() => {}); + await page.highlightController.addHighlight(selector, { pierce }).catch(() => {}); })); } @@ -476,7 +478,7 @@ export class Recorder extends EventEmitter implements Instrume private async _onPage(page: Page) { const frame = page.mainFrame(); if (this._highlightedSelector) - frame.addHighlight(this._highlightedSelector).catch(() => {}); + page.highlightController.addHighlight(this._highlightedSelector.selector, { pierce: this._highlightedSelector.pierce }).catch(() => {}); page.on(Page.Events.Close, () => { this._signalProcessor.addAction({ pageGuid: page.guid, diff --git a/packages/playwright-core/src/server/screenshotter.ts b/packages/playwright-core/src/server/screenshotter.ts index d96c32c5fb135..33db059bbb111 100644 --- a/packages/playwright-core/src/server/screenshotter.ts +++ b/packages/playwright-core/src/server/screenshotter.ts @@ -273,9 +273,9 @@ export class Screenshotter { if (!options.mask || !options.mask.length) return () => Promise.resolve(); - const cleanup = () => this._page.hideHighlight(); + const cleanup = () => this._page.highlightController.hideHighlights(); try { - await progress.race(this._page.hideHighlight()); + await progress.race(this._page.highlightController.hideHighlights()); await progress.race(Promise.all((options.mask || []).map(async ({ frame, selector }) => { await frame.selectors.callOnSelector(selector, { strict: false }, ({ injected, elements }, color) => { injected.addMaskedElements(elements, color); diff --git a/packages/trace-viewer/src/ui/snapshotTab.tsx b/packages/trace-viewer/src/ui/snapshotTab.tsx index c3417343fc797..57757b79de0ba 100644 --- a/packages/trace-viewer/src/ui/snapshotTab.tsx +++ b/packages/trace-viewer/src/ui/snapshotTab.tsx @@ -295,13 +295,14 @@ export const InspectModeController: React.FunctionComponent<{ for (const { recorder, frameSelector } of recorders) { const actionSelector = fullSelector?.startsWith(frameSelector) ? fullSelector.substring(frameSelector.length).trim() : undefined; const ariaTemplate = parsedSnapshot?.errors.length === 0 ? parsedSnapshot.fragment : undefined; - const highlightSelector = actionSelector || (ariaTemplate ? 'aria-template=' + JSON.stringify(ariaTemplate) : undefined); - recorder.injectedScript.hideHighlight(); - if (highlightSelector) { - try { - recorder.injectedScript.addHighlight(recorder.injectedScript.parseSelector(highlightSelector)); - } catch { - } + try { + const highlightSelector = actionSelector || (ariaTemplate ? 'aria-template=' + JSON.stringify(ariaTemplate) : undefined); + if (highlightSelector) + recorder.injectedScript.setHighlights([{ selector: recorder.injectedScript.parseSelector(highlightSelector) }]); + else + recorder.injectedScript.setHighlights([]); + } catch { + recorder.injectedScript.setHighlights([]); } recorder.setUIState({ mode: isInspecting ? 'inspecting' : 'none', diff --git a/tests/library/debug-controller.spec.ts b/tests/library/debug-controller.spec.ts index fe3677ac9e138..784b5e00253c1 100644 --- a/tests/library/debug-controller.spec.ts +++ b/tests/library/debug-controller.spec.ts @@ -318,8 +318,7 @@ test('should highlight inside iframe', async ({ backend, connectedBrowser }, tes await expect(highlight).toHaveCount(0); await backend.highlight({ selector: `getByText('bar')` }, undefined); - // TODO: highlight inside the iframe as well when piercing frames is supported. - await expect(highlight).toHaveCount(0); + await expect(highlight).toHaveCount(1); await expect(page.locator('x-pw-highlight')).toHaveCount(1); }); diff --git a/tests/library/locator-highlight.spec.ts b/tests/library/locator-highlight.spec.ts index 6e4fd5bf7ab0f..7ed5ca94e07b6 100644 --- a/tests/library/locator-highlight.spec.ts +++ b/tests/library/locator-highlight.spec.ts @@ -86,6 +86,24 @@ test('hideHighlight removes a styled highlight', async ({ browser, server }) => await context.close(); }); +test('highlight should survive navigation', async ({ browser, server }) => { + const context = await browser.newContext(); + const page = await context.newPage(); + await page.setContent(``); + + await page.getByRole('button').highlight(); + await expect(page.locator('x-pw-highlight')).toHaveCount(1); + + // Highlights are resolved again after the navigation. + await page.goto(server.PREFIX + '/input/button.html'); + await expect(page.locator('x-pw-highlight')).toHaveCount(1); + + await page.hideHighlight(); + await expect(page.locator('x-pw-highlight')).toHaveCount(0); + + await context.close(); +}); + test('Page.hideHighlight clears all locator highlights', async ({ browser, server }) => { const context = await browser.newContext(); const page = await context.newPage(); From be6e7cc9f832223193d7063082b3afbe946348d8 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Mon, 17 Aug 2026 16:30:33 -0700 Subject: [PATCH 3/4] fix(mcp): do not clobber chromiumSandbox from the config file (#42288) --- .../playwright-core/src/tools/mcp/program.ts | 4 -- tests/mcp/config.spec.ts | 37 +++++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/packages/playwright-core/src/tools/mcp/program.ts b/packages/playwright-core/src/tools/mcp/program.ts index 4fb5d4302d47b..12fdf17f6ee4a 100644 --- a/packages/playwright-core/src/tools/mcp/program.ts +++ b/packages/playwright-core/src/tools/mcp/program.ts @@ -81,10 +81,6 @@ export function decorateMCPCommand(command: Command) { .option('--viewport-size ', 'specify browser viewport size in pixels, for example "1280x720"', resolutionParser.bind(null, '--viewport-size')) .addOption(new ProgramOption('--vision', 'Legacy option, use --caps=vision instead').hideHelp()) .action(async options => { - - // normalize the --no-sandbox option: sandbox = true => nothing was passed, sandbox = false => --no-sandbox was passed. - options.sandbox = options.sandbox === true ? undefined : false; - setupExitWatchdog(); if (options.vision) { diff --git a/tests/mcp/config.spec.ts b/tests/mcp/config.spec.ts index cb42d5214555c..dcb4f83c17a1f 100644 --- a/tests/mcp/config.spec.ts +++ b/tests/mcp/config.spec.ts @@ -173,3 +173,40 @@ test('browser_get_config returns merged config from file, env and cli', async ({ // From CLI arg (--isolated). expect(config.browser.isolated).toBe(true); }); + +test.describe('chromiumSandbox', () => { + test.skip(({ mcpBrowser }) => mcpBrowser !== 'chrome', 'Channel-agnostic tests.'); + + test('config file value is respected', { annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright-mcp/issues/1716' } }, async ({ startClient }) => { + const { client } = await startClient({ + config: { + capabilities: ['config'], + browser: { launchOptions: { chromiumSandbox: true } }, + }, + }); + const config = JSON.parse(parseResponse(await client.callTool({ name: 'browser_get_config' })).result); + expect(config.browser.launchOptions.chromiumSandbox).toBe(true); + }); + + test('--no-sandbox overrides config file value', async ({ startClient }) => { + const { client } = await startClient({ + config: { + capabilities: ['config'], + browser: { launchOptions: { chromiumSandbox: true } }, + }, + args: ['--no-sandbox'], + }); + const config = JSON.parse(parseResponse(await client.callTool({ name: 'browser_get_config' })).result); + expect(config.browser.launchOptions.chromiumSandbox).toBe(false); + }); + + test('--sandbox enables the sandbox', async ({ startClient }) => { + const { client } = await startClient({ + config: { capabilities: ['config'] }, + args: ['--browser=chromium', '--sandbox'], + }); + const config = JSON.parse(parseResponse(await client.callTool({ name: 'browser_get_config' })).result); + expect(config.browser.launchOptions.channel).toBe('chrome-for-testing'); + expect(config.browser.launchOptions.chromiumSandbox).toBe(true); + }); +}); From 51023b427acf92ed3f416fd10d8efd7a47184f6b Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Mon, 17 Aug 2026 20:46:31 -0700 Subject: [PATCH 4/4] chore(trace-viewer): show only the screencast frame on timeline hover (#42285) --- packages/trace-viewer/src/ui/filmStrip.css | 8 -------- packages/trace-viewer/src/ui/filmStrip.tsx | 17 ++++++----------- packages/trace-viewer/src/ui/timeline.tsx | 10 +++------- packages/trace-viewer/src/ui/workbench.tsx | 1 - 4 files changed, 9 insertions(+), 27 deletions(-) diff --git a/packages/trace-viewer/src/ui/filmStrip.css b/packages/trace-viewer/src/ui/filmStrip.css index a81450c69e7dc..385691ed25609 100644 --- a/packages/trace-viewer/src/ui/filmStrip.css +++ b/packages/trace-viewer/src/ui/filmStrip.css @@ -52,11 +52,3 @@ z-index: 200; pointer-events: none; } - -.film-strip-hover-title { - padding: 2px 4px; - display: flex; - align-items: center; - overflow: hidden; - line-height: 28px; -} diff --git a/packages/trace-viewer/src/ui/filmStrip.tsx b/packages/trace-viewer/src/ui/filmStrip.tsx index 5974d9efbc44d..a264b28d4c64a 100644 --- a/packages/trace-viewer/src/ui/filmStrip.tsx +++ b/packages/trace-viewer/src/ui/filmStrip.tsx @@ -18,16 +18,12 @@ import './filmStrip.css'; import type { Boundaries, Size } from './geometry'; import * as React from 'react'; import { useMeasure, upperBound } from '@web/uiUtils'; -import type { ActionEntry, PageEntry } from '@isomorphic/trace/entries'; -import { renderAction } from './actionList'; -import type { Language } from '@isomorphic/locatorGenerators'; +import type { PageEntry } from '@isomorphic/trace/entries'; import { useTraceModel } from './traceModelContext'; export type FilmStripPreviewPoint = { x: number; clientY: number; - action?: ActionEntry; - sdkLanguage: Language; }; const tileSize = { width: 200, height: 45 }; @@ -70,15 +66,14 @@ export const FilmStrip: React.FunctionComponent<{ key={index} /> : null) } - {model && previewPoint?.x !== undefined && + {model && previewPoint && previewImage && previewSize &&
- {previewImage && previewSize &&
- -
} - {previewPoint.action &&
{renderAction(previewPoint.action, previewPoint)}
} +
} ; diff --git a/packages/trace-viewer/src/ui/timeline.tsx b/packages/trace-viewer/src/ui/timeline.tsx index 787a407aa9bc5..849eb924dafef 100644 --- a/packages/trace-viewer/src/ui/timeline.tsx +++ b/packages/trace-viewer/src/ui/timeline.tsx @@ -24,7 +24,6 @@ import type { FilmStripPreviewPoint } from './filmStrip'; import type { TraceModel } from '@isomorphic/trace/traceModel'; import type { ActionEntry } from '@isomorphic/trace/entries'; import './timeline.css'; -import type { Language } from '@isomorphic/locatorGenerators'; import type { ActionGroup } from '@isomorphic/protocolFormatter'; export const Timeline: React.FunctionComponent<{ @@ -34,9 +33,8 @@ export const Timeline: React.FunctionComponent<{ selectedTime: Boundaries | undefined, setSelectedTime: (time: Boundaries | undefined) => void, highlightedTime?: Boundaries, - sdkLanguage: Language, scrubber?: React.ReactNode, -}> = ({ model, boundaries, onSelected, selectedTime, setSelectedTime, highlightedTime, sdkLanguage, scrubber }) => { +}> = ({ model, boundaries, onSelected, selectedTime, setSelectedTime, highlightedTime, scrubber }) => { const [measure, ref] = useMeasure(); const [dragWindow, setDragWindow] = React.useState<{ startX: number, endX: number, pivot?: number, type: 'resize' | 'move' } | undefined>(); const [previewPoint, setPreviewPoint] = React.useState(); @@ -156,10 +154,8 @@ export const Timeline: React.FunctionComponent<{ if (!ref.current) return; const x = event.clientX - ref.current.getBoundingClientRect().left; - const time = positionToTime(measure.width, boundaries, x); - const action = actions?.findLast(action => action.startTime <= time); - setPreviewPoint({ x, clientY: event.clientY, action, sdkLanguage }); - }, [boundaries, measure, actions, ref, sdkLanguage]); + setPreviewPoint({ x, clientY: event.clientY }); + }, [ref]); const onMouseLeave = React.useCallback(() => { setPreviewPoint(undefined); diff --git a/packages/trace-viewer/src/ui/workbench.tsx b/packages/trace-viewer/src/ui/workbench.tsx index bc65849aeca60..ab31fc558b638 100644 --- a/packages/trace-viewer/src/ui/workbench.tsx +++ b/packages/trace-viewer/src/ui/workbench.tsx @@ -369,7 +369,6 @@ const PartitionedWorkbench: React.FunctionComponent