Skip to content

chore(bidi): add support for context locators #38129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

chore(bidi): add support for context locators #38129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions packages/playwright-core/src/server/bidi/bidiBrowser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,13 @@ export class BidiBrowser extends Browser {
const page = this._findPageForFrame(parentFrameId);
if (page) {
page._session.addFrameBrowsingContext(event.context);
page._page.frameManager.frameAttached(event.context, parentFrameId);
const frame = page._page.frameManager.frame(event.context);
if (frame)
frame._url = event.url;
const frame = page._page.frameManager.frameAttached(event.context, parentFrameId);
frame._url = event.url;
page._getFrameNode(frame).then(node => {
const attributes = node?.value?.attributes;
frame._name = attributes?.name ?? attributes?.id ?? '';
});
return;
}
return;
}
Expand Down
4 changes: 2 additions & 2 deletions packages/playwright-core/src/server/bidi/bidiExecutionContext.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,11 @@ export class BidiExecutionContext implements js.ExecutionContextDelegate {
};
}

async remoteObjectForNodeId(context: dom.FrameExecutionContext, nodeId: bidi.Script.SharedReference): Promise<js.JSHandle> {
async remoteObjectForNodeId(context: dom.FrameExecutionContext, nodeId: bidi.Script.SharedReference): Promise<dom.ElementHandle> {
const result = await this._remoteValueForReference(nodeId, true);
if (!('handle' in result))
throw new Error('Can\'t get remote object for nodeId');
return createHandle(context, result);
return createHandle(context, result) as dom.ElementHandle;
}

async contentFrameIdForFrame(handle: dom.ElementHandle) {
Expand Down
37 changes: 19 additions & 18 deletions packages/playwright-core/src/server/bidi/bidiPage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,8 @@ export class BidiPage implements PageDelegate {

private _onNavigationCommitted(params: bidi.BrowsingContext.NavigationInfo) {
const frameId = params.context;
this._page.frameManager.frameCommittedNewDocumentNavigation(frameId, params.url, '', params.navigation!, /* initial */ false);
const frame = this._page.frameManager.frame(frameId)!;
this._page.frameManager.frameCommittedNewDocumentNavigation(frameId, params.url, frame._name, params.navigation!, /* initial */ false);
}

private _onDomContentLoaded(params: bidi.BrowsingContext.NavigationInfo) {
Expand Down Expand Up @@ -587,24 +588,24 @@ export class BidiPage implements PageDelegate {
const parent = frame.parentFrame();
if (!parent)
throw new Error('Frame has been detached.');
const parentContext = await parent._mainContext();
const list = await parentContext.evaluateHandle(() => { return [...document.querySelectorAll('iframe,frame')]; });
const length = await list.evaluate(list => list.length);
let foundElement = null;
for (let i = 0; i < length; i++) {
const element = await list.evaluateHandle((list, i) => list[i], i);
const candidate = await element.contentFrame();
if (frame === candidate) {
foundElement = element;
break;
} else {
element.dispose();
}
}
list.dispose();
if (!foundElement)
const node = await this._getFrameNode(frame);
if (!node?.sharedId)
throw new Error('Frame has been detached.');
return foundElement;
const parentFrameExecutionContext = await parent._mainContext();
return await toBidiExecutionContext(parentFrameExecutionContext).remoteObjectForNodeId(parentFrameExecutionContext, { sharedId: node.sharedId });
}

async _getFrameNode(frame: frames.Frame): Promise<bidi.Script.NodeRemoteValue | undefined> {
const parent = frame.parentFrame();
if (!parent)
return undefined;

const result = await this._session.send('browsingContext.locateNodes', {
context: parent._id,
locator: { type: 'context', value: { context: frame._id } },
});
const node = result.nodes[0];
return node;
}

shouldToggleStyleSheetToSyncAnimations(): boolean {
Expand Down
2 changes: 1 addition & 1 deletion tests/library/hit-target.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ it('should click into frame inside closed shadow root', async ({ page, server })
</script>
`);

const frame = page.frame({ name: 'myframe' });
const frame = page.frames()[1];
await frame.locator('text=click me').click();
expect(await page.evaluate('window.__clicked')).toBe(true);
});
Expand Down
4 changes: 2 additions & 2 deletions tests/page/elementhandle-bounding-box.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ it('should work', async ({ page, server, browserName, headless, isLinux }) => {
it('should handle nested frames', async ({ page, server }) => {
await page.setViewportSize({ width: 616, height: 500 });
await page.goto(server.PREFIX + '/frames/nested-frames.html');
const nestedFrame = page.frames().find(frame => frame.name() === 'dos');
const elementHandle = await nestedFrame.$('div');
const nestedFrame = page.frameLocator('[name="2frames"]').frameLocator('[name=dos]');
const elementHandle = await nestedFrame.locator('div').elementHandle();
const box = await elementHandle.boundingBox();
expect(box).toEqual({ x: 24, y: 224, width: 268, height: 18 });
});
Expand Down
4 changes: 2 additions & 2 deletions tests/page/frame-frame-element.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ it('should work inside closed shadow root', async ({ page, server, browserName }
</script>
`);

const frame = page.frame({ name: 'myframe' });
const frame = page.frames()[1];
const element = await frame.frameElement();
expect(await element.getAttribute('name')).toBe('myframe');
});
Expand All @@ -87,7 +87,7 @@ it('should work inside declarative shadow root', async ({ page, server, browserN
<span>footer</span>
</div>
`);
const frame = page.frame({ name: 'myframe' });
const frame = page.frames()[1];
const element = await frame.frameElement();
expect(await element.getAttribute('name')).toBe('myframe');
});
9 changes: 7 additions & 2 deletions tests/page/frame-hierarchy.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@ function dumpFrames(frame: Frame, indentation: string = ''): string[] {
return result;
}

it('should handle nested frames @smoke', async ({ page, server, isAndroid }) => {
it('should handle nested frames @smoke', async ({ page, server, isAndroid, browserName, channel }) => {
it.skip(isAndroid, 'No cross-process on Android');
it.skip(browserName === 'firefox' && channel?.startsWith('moz-firefox'), 'frame.name() is racy with BiDi');
it.skip(channel?.startsWith('bidi-chrom'), 'frame.name() is racy with BiDi');

await page.goto(server.PREFIX + '/frames/nested-frames.html');
expect(dumpFrames(page.mainFrame())).toEqual([
Expand Down Expand Up @@ -154,7 +156,10 @@ it('should report frame from-inside shadow DOM', async ({ page, server }) => {
expect(page.frames()[1].url()).toBe(server.EMPTY_PAGE);
});

it('should report frame.name()', async ({ page, server }) => {
it('should report frame.name()', async ({ page, server, browserName, channel }) => {
it.skip(browserName === 'firefox' && channel?.startsWith('moz-firefox'), 'frame.name() is racy with BiDi');
it.skip(channel?.startsWith('bidi-chrom'), 'frame.name() is racy with BiDi');
Copy link
Collaborator Author

@hbenl hbenl Nov 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, this test could be fixed by using

expect((await (await page.frames()[1].frameElement()).getAttribute('id'))).toBe('theFrameId');
expect((await (await page.frames()[2].frameElement()).getAttribute('name'))).toBe('theFrameName');

Copy link
Member

@yury-s yury-s Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the test's title suggests it validates frame.name() behavior, so the code should invoke name() at some point.

Copy link
Collaborator Author

@hbenl hbenl Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's why I didn't include this in the PR. I was just mentioning it to show what the alternative to frame.name() for BiDi looks like.


await attachFrame(page, 'theFrameId', server.EMPTY_PAGE);
await page.evaluate(url => {
const frame = document.createElement('iframe');
Expand Down
5 changes: 4 additions & 1 deletion tests/page/page-basic.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,10 @@ it('page.close should work with window.close', async function({ page }) {
await closedPromise;
});

it('page.frame should respect name', async function({ page }) {
it('page.frame should respect name', async function({ page, browserName, channel }) {
it.skip(browserName === 'firefox' && channel?.startsWith('moz-firefox'), 'page.frame({ name }) is racy with BiDi');
it.skip(channel?.startsWith('bidi-chrom'), 'page.frame({ name }) is racy with BiDi');

await page.setContent(`<iframe name=target></iframe>`);
expect(page.frame({ name: 'bogus' })).toBe(null);
const frame = page.frame({ name: 'target' });
Expand Down
Loading