From dd4e4cf8d6adafd30e844f0791a8b5dce558db91 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Tue, 22 Sep 2026 15:07:59 -0700 Subject: [PATCH] fix(highlight): resolve highlights relative to the frame of the locator HighlightController resolved every selector from the main frame, so frame.locator(...).highlight() highlighted elements in the page instead of the frame. Thread the frame through the highlight entry and key the entries by frame, so the same selector can be highlighted from different frames. Fixes: https://github.com/microsoft/playwright/issues/42850 --- .../src/server/dispatchers/frameDispatcher.ts | 4 +-- .../src/server/highlightController.ts | 25 +++++++++++++++---- tests/library/locator-highlight.spec.ts | 21 ++++++++++++++++ 3 files changed, 43 insertions(+), 7 deletions(-) diff --git a/packages/playwright-core/src/server/dispatchers/frameDispatcher.ts b/packages/playwright-core/src/server/dispatchers/frameDispatcher.ts index 5cd51a82a0336..b624c4e44ae25 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._page.highlightController.addHighlight(params.selector, { style: params.style })); + return await progress.race(this._frame._page.highlightController.addHighlight(params.selector, { style: params.style, frame: this._frame })); } async hideHighlight(params: channels.FrameHideHighlightParams, progress: Progress): Promise { - return await progress.race(this._frame._page.highlightController.removeHighlight(params.selector)); + return await progress.race(this._frame._page.highlightController.removeHighlight(params.selector, this._frame)); } async expect(params: channels.FrameExpectParams, progress: Progress): Promise { diff --git a/packages/playwright-core/src/server/highlightController.ts b/packages/playwright-core/src/server/highlightController.ts index 69b829bccdd0f..37358aaf76d24 100644 --- a/packages/playwright-core/src/server/highlightController.ts +++ b/packages/playwright-core/src/server/highlightController.ts @@ -14,18 +14,22 @@ * limitations under the License. */ +import { Page } from './page'; + import type { FrameExecutionContext } from './dom'; -import type { Page } from './page'; +import type { Frame } from './frames'; import type * as types from './types'; import type { ParsedSelector } from '@isomorphic/selectorParser'; export type HighlightOptions = { style?: string; anyFrame?: boolean; // Highlight in all the frames the selector could resolve to, instead of a single one. + frame?: Frame; // Resolve the selector relative to this frame, defaults to the main frame. }; type HighlightEntry = HighlightOptions & { selector: string; + frame: Frame; }; // Custom selector engines run in the main world, so highlights may live in either world. @@ -39,20 +43,31 @@ export class HighlightController { constructor(page: Page) { this._page = page; + page.on(Page.Events.FrameDetached, frame => { + for (const [key, entry] of this._entries) { + if (entry.frame === frame) + this._entries.delete(key); + } + }); } 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 }); + const frame = options.frame ?? this._page.mainFrame(); + this._entries.set(this._key(frame, selector), { selector, ...options, frame }); await this._resolveNow(); } - async removeHighlight(selector: string) { - this._entries.delete(selector); + async removeHighlight(selector: string, frame?: Frame) { + this._entries.delete(this._key(frame ?? this._page.mainFrame(), selector)); await this._resolveNow(); } + private _key(frame: Frame, selector: string) { + return frame.guid + ':' + selector; + } + dispose() { if (this._resolutionTimer) { clearTimeout(this._resolutionTimer); @@ -85,7 +100,7 @@ export class HighlightController { const perContext = new Map(); for (const entry of this._entries.values()) { - const results = await this._page.mainFrame().selectors.resolveFramesForSelector(entry.selector, { strict: false, anyFrame: entry.anyFrame }).catch(() => []); + const results = await entry.frame.selectors.resolveFramesForSelector(entry.selector, { strict: false, anyFrame: entry.anyFrame }).catch(() => []); for (const { frame, info } of results) { const context = frame.existingContext(info.world); if (!context) diff --git a/tests/library/locator-highlight.spec.ts b/tests/library/locator-highlight.spec.ts index 7b18e1bf20dd3..35f3c4fc0f406 100644 --- a/tests/library/locator-highlight.spec.ts +++ b/tests/library/locator-highlight.spec.ts @@ -151,3 +151,24 @@ test('highlight should work with a custom selector engine that runs in the main await context.close(); }); + +test('highlight should resolve relative to the frame of the locator', async ({ browser }) => { + const context = await browser.newContext(); + const page = await context.newPage(); + await page.setContent(''); + const frame = page.frame('frame')!; + + await page.locator('button').highlight(); + await frame.locator('button').highlight(); + await expect(page.locator('x-pw-highlight')).toHaveCount(1); + await expect(frame.locator('x-pw-highlight')).toHaveCount(1); + + await page.locator('button').hideHighlight(); + await expect(page.locator('x-pw-highlight')).toHaveCount(0); + await expect(frame.locator('x-pw-highlight')).toHaveCount(1); + + await frame.locator('button').hideHighlight(); + await expect(frame.locator('x-pw-highlight')).toHaveCount(0); + + await context.close(); +});