diff --git a/packages/playwright-core/src/server/injected/selectorGenerator.ts b/packages/playwright-core/src/server/injected/selectorGenerator.ts index 7aa8d1d664..51ff0c2560 100644 --- a/packages/playwright-core/src/server/injected/selectorGenerator.ts +++ b/packages/playwright-core/src/server/injected/selectorGenerator.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { cssEscape, escapeForAttributeSelector, escapeForTextSelector, normalizeWhiteSpace } from '../../utils/isomorphic/stringUtils'; +import { cssEscape, escapeForAttributeSelector, escapeForTextSelector, normalizeWhiteSpace, quoteCSSAttributeValue } from '../../utils/isomorphic/stringUtils'; import { closestCrossShadow, isInsideScope, parentElementOrShadowHost } from './domUtils'; import type { InjectedScript } from './injectedScript'; import { getAriaRole, getElementAccessibleName, beginAriaCaches, endAriaCaches } from './roleUtils'; @@ -198,7 +198,7 @@ function buildNoTextCandidates(injectedScript: InjectedScript, element: Element, { for (const attr of ['data-testid', 'data-test-id', 'data-test']) { if (attr !== options.testIdAttributeName && element.getAttribute(attr)) - candidates.push({ engine: 'css', selector: `[${attr}=${quoteAttributeValue(element.getAttribute(attr)!)}]`, score: kOtherTestIdScore }); + candidates.push({ engine: 'css', selector: `[${attr}=${quoteCSSAttributeValue(element.getAttribute(attr)!)}]`, score: kOtherTestIdScore }); } const idAttr = element.getAttribute('id'); @@ -211,12 +211,12 @@ function buildNoTextCandidates(injectedScript: InjectedScript, element: Element, if (element.nodeName === 'IFRAME') { for (const attribute of ['name', 'title']) { if (element.getAttribute(attribute)) - candidates.push({ engine: 'css', selector: `${cssEscape(element.nodeName.toLowerCase())}[${attribute}=${quoteAttributeValue(element.getAttribute(attribute)!)}]`, score: kIframeByAttributeScore }); + candidates.push({ engine: 'css', selector: `${cssEscape(element.nodeName.toLowerCase())}[${attribute}=${quoteCSSAttributeValue(element.getAttribute(attribute)!)}]`, score: kIframeByAttributeScore }); } // Locate by testId via CSS selector. if (element.getAttribute(options.testIdAttributeName)) - candidates.push({ engine: 'css', selector: `[${options.testIdAttributeName}=${quoteAttributeValue(element.getAttribute(options.testIdAttributeName)!)}]`, score: kTestIdScore }); + candidates.push({ engine: 'css', selector: `[${options.testIdAttributeName}=${quoteCSSAttributeValue(element.getAttribute(options.testIdAttributeName)!)}]`, score: kTestIdScore }); penalizeScoreForLength([candidates]); return candidates; @@ -246,11 +246,11 @@ function buildNoTextCandidates(injectedScript: InjectedScript, element: Element, candidates.push({ engine: 'internal:role', selector: ariaRole, score: kRoleWithoutNameScore }); if (element.getAttribute('name') && ['BUTTON', 'FORM', 'FIELDSET', 'FRAME', 'IFRAME', 'INPUT', 'KEYGEN', 'OBJECT', 'OUTPUT', 'SELECT', 'TEXTAREA', 'MAP', 'META', 'PARAM'].includes(element.nodeName)) - candidates.push({ engine: 'css', selector: `${cssEscape(element.nodeName.toLowerCase())}[name=${quoteAttributeValue(element.getAttribute('name')!)}]`, score: kCSSInputTypeNameScore }); + candidates.push({ engine: 'css', selector: `${cssEscape(element.nodeName.toLowerCase())}[name=${quoteCSSAttributeValue(element.getAttribute('name')!)}]`, score: kCSSInputTypeNameScore }); if (['INPUT', 'TEXTAREA'].includes(element.nodeName) && element.getAttribute('type') !== 'hidden') { if (element.getAttribute('type')) - candidates.push({ engine: 'css', selector: `${cssEscape(element.nodeName.toLowerCase())}[type=${quoteAttributeValue(element.getAttribute('type')!)}]`, score: kCSSInputTypeNameScore }); + candidates.push({ engine: 'css', selector: `${cssEscape(element.nodeName.toLowerCase())}[type=${quoteCSSAttributeValue(element.getAttribute('type')!)}]`, score: kCSSInputTypeNameScore }); } if (['INPUT', 'TEXTAREA', 'SELECT'].includes(element.nodeName) && element.getAttribute('type') !== 'hidden') @@ -378,10 +378,6 @@ function cssFallback(injectedScript: InjectedScript, targetElement: Element, opt return makeStrict(uniqueCSSSelector()!); } -function quoteAttributeValue(text: string): string { - return `"${cssEscape(text).replace(/\\ /g, ' ')}"`; -} - function penalizeScoreForLength(groups: SelectorToken[][]) { for (const group of groups) { for (const token of group) { diff --git a/packages/playwright-core/src/server/recorder.ts b/packages/playwright-core/src/server/recorder.ts index 2a4d531c20..3dc1482879 100644 --- a/packages/playwright-core/src/server/recorder.ts +++ b/packages/playwright-core/src/server/recorder.ts @@ -43,6 +43,7 @@ import { EventEmitter } from 'events'; import { raceAgainstDeadline } from '../utils/timeoutRunner'; import type { Language, LanguageGenerator } from './recorder/language'; import { locatorOrSelectorAsSelector } from '../utils/isomorphic/locatorParser'; +import { quoteCSSAttributeValue } from '../utils/isomorphic/stringUtils'; import { eventsHelper, type RegisteredListener } from './../utils/eventsHelper'; import type { Dialog } from './dialog'; @@ -530,7 +531,6 @@ class ContextRecorder extends EventEmitter { return { pageAlias: this._pageAliases.get(page)!, isMainFrame: true, - url: page.mainFrame().url(), }; } @@ -545,16 +545,6 @@ class ContextRecorder extends EventEmitter { if (chain.length === 1) return this._describeMainFrame(page); - const hasUniqueName = page.frames().filter(f => f.name() === frame.name()).length === 1; - const fallback: actions.FrameDescription = { - pageAlias, - isMainFrame: false, - url: frame.url(), - name: frame.name() && hasUniqueName ? frame.name() : undefined, - }; - if (chain.length > 3) - return fallback; - const selectorPromises: Promise[] = []; for (let i = 0; i < chain.length - 1; i++) selectorPromises.push(findFrameSelector(chain[i + 1])); @@ -562,11 +552,24 @@ class ContextRecorder extends EventEmitter { const result = await raceAgainstDeadline(() => Promise.all(selectorPromises), monotonicTime() + 2000); if (!result.timedOut && result.result.every(selector => !!selector)) { return { - ...fallback, + pageAlias, + isMainFrame: false, selectorsChain: result.result as string[], }; } - return fallback; + // Best effort to find a selector for the frame. + const selectorsChain = []; + for (let i = 0; i < chain.length - 1; i++) { + if (chain[i].name()) + selectorsChain.push(`iframe[name=${quoteCSSAttributeValue(chain[i].name())}]`); + else + selectorsChain.push(`iframe[src=${quoteCSSAttributeValue(chain[i].url())}]`); + } + return { + pageAlias, + isMainFrame: false, + selectorsChain, + }; } testIdAttributeName(): string { diff --git a/packages/playwright-core/src/server/recorder/codeGenerator.ts b/packages/playwright-core/src/server/recorder/codeGenerator.ts index e0df6aca96..ac102e1d34 100644 --- a/packages/playwright-core/src/server/recorder/codeGenerator.ts +++ b/packages/playwright-core/src/server/recorder/codeGenerator.ts @@ -141,12 +141,11 @@ export class CodeGenerator extends EventEmitter { return; } - if (signal.name === 'navigation') { + if (signal.name === 'navigation' && frame._page.mainFrame() === frame) { this.addAction({ frame: { pageAlias, - isMainFrame: frame._page.mainFrame() === frame, - url: frame.url(), + isMainFrame: true, }, committed: true, action: { diff --git a/packages/playwright-core/src/server/recorder/csharp.ts b/packages/playwright-core/src/server/recorder/csharp.ts index 5488445f1c..d4da3e1b0b 100644 --- a/packages/playwright-core/src/server/recorder/csharp.ts +++ b/packages/playwright-core/src/server/recorder/csharp.ts @@ -76,13 +76,9 @@ export class CSharpLanguageGenerator implements LanguageGenerator { let subject: string; if (actionInContext.frame.isMainFrame) { subject = pageAlias; - } else if (actionInContext.frame.selectorsChain && action.name !== 'navigate') { + } else { const locators = actionInContext.frame.selectorsChain.map(selector => `.FrameLocator(${quote(selector)})`); subject = `${pageAlias}${locators.join('')}`; - } else if (actionInContext.frame.name) { - subject = `${pageAlias}.Frame(${quote(actionInContext.frame.name)})`; - } else { - subject = `${pageAlias}.FrameByUrl(${quote(actionInContext.frame.url)})`; } const signals = toSignalMap(action); diff --git a/packages/playwright-core/src/server/recorder/java.ts b/packages/playwright-core/src/server/recorder/java.ts index 63e772ca95..48dcba0f67 100644 --- a/packages/playwright-core/src/server/recorder/java.ts +++ b/packages/playwright-core/src/server/recorder/java.ts @@ -48,14 +48,10 @@ export class JavaLanguageGenerator implements LanguageGenerator { let inFrameLocator = false; if (actionInContext.frame.isMainFrame) { subject = pageAlias; - } else if (actionInContext.frame.selectorsChain && action.name !== 'navigate') { + } else { const locators = actionInContext.frame.selectorsChain.map(selector => `.frameLocator(${quote(selector)})`); subject = `${pageAlias}${locators.join('')}`; inFrameLocator = true; - } else if (actionInContext.frame.name) { - subject = `${pageAlias}.frame(${quote(actionInContext.frame.name)})`; - } else { - subject = `${pageAlias}.frameByUrl(${quote(actionInContext.frame.url)})`; } const signals = toSignalMap(action); diff --git a/packages/playwright-core/src/server/recorder/javascript.ts b/packages/playwright-core/src/server/recorder/javascript.ts index c462b3aa6e..3c828d05c3 100644 --- a/packages/playwright-core/src/server/recorder/javascript.ts +++ b/packages/playwright-core/src/server/recorder/javascript.ts @@ -56,13 +56,9 @@ export class JavaScriptLanguageGenerator implements LanguageGenerator { let subject: string; if (actionInContext.frame.isMainFrame) { subject = pageAlias; - } else if (actionInContext.frame.selectorsChain && action.name !== 'navigate') { + } else { const locators = actionInContext.frame.selectorsChain.map(selector => `.frameLocator(${quote(selector)})`); subject = `${pageAlias}${locators.join('')}`; - } else if (actionInContext.frame.name) { - subject = `${pageAlias}.frame(${formatObject({ name: actionInContext.frame.name })})`; - } else { - subject = `${pageAlias}.frame(${formatObject({ url: actionInContext.frame.url })})`; } const signals = toSignalMap(action); diff --git a/packages/playwright-core/src/server/recorder/python.ts b/packages/playwright-core/src/server/recorder/python.ts index 18245e21b8..22a387c5df 100644 --- a/packages/playwright-core/src/server/recorder/python.ts +++ b/packages/playwright-core/src/server/recorder/python.ts @@ -63,13 +63,9 @@ export class PythonLanguageGenerator implements LanguageGenerator { let subject: string; if (actionInContext.frame.isMainFrame) { subject = pageAlias; - } else if (actionInContext.frame.selectorsChain && action.name !== 'navigate') { + } else { const locators = actionInContext.frame.selectorsChain.map(selector => `.frame_locator(${quote(selector)})`); subject = `${pageAlias}${locators.join('')}`; - } else if (actionInContext.frame.name) { - subject = `${pageAlias}.frame(${formatOptions({ name: actionInContext.frame.name }, false)})`; - } else { - subject = `${pageAlias}.frame(${formatOptions({ url: actionInContext.frame.url }, false)})`; } const signals = toSignalMap(action); diff --git a/packages/playwright-core/src/server/recorder/recorderActions.ts b/packages/playwright-core/src/server/recorder/recorderActions.ts index a0c8b2dc32..9164bb74f4 100644 --- a/packages/playwright-core/src/server/recorder/recorderActions.ts +++ b/packages/playwright-core/src/server/recorder/recorderActions.ts @@ -128,10 +128,13 @@ export type DialogSignal = BaseSignal & { export type Signal = NavigationSignal | PopupSignal | DownloadSignal | DialogSignal; -export type FrameDescription = { - pageAlias: string; - isMainFrame: boolean; - url: string; - name?: string; - selectorsChain?: string[]; +type FrameDescriptionMainFrame = { + isMainFrame: true; }; + +type FrameDescriptionChildFrame = { + isMainFrame: false; + selectorsChain: string[]; +}; + +export type FrameDescription = { pageAlias: string } & (FrameDescriptionMainFrame | FrameDescriptionChildFrame); diff --git a/packages/playwright-core/src/utils/isomorphic/stringUtils.ts b/packages/playwright-core/src/utils/isomorphic/stringUtils.ts index 27b65b4003..ee2258641a 100644 --- a/packages/playwright-core/src/utils/isomorphic/stringUtils.ts +++ b/packages/playwright-core/src/utils/isomorphic/stringUtils.ts @@ -47,6 +47,10 @@ export function cssEscape(s: string): string { return result; } +export function quoteCSSAttributeValue(text: string): string { + return `"${cssEscape(text).replace(/\\ /g, ' ')}"`; +} + function cssEscapeOne(s: string, i: number): string { // https://drafts.csswg.org/cssom/#serialize-an-identifier const c = s.charCodeAt(i); diff --git a/tests/library/inspector/cli-codegen-3.spec.ts b/tests/library/inspector/cli-codegen-3.spec.ts index c8858832f7..2c6af8be0a 100644 --- a/tests/library/inspector/cli-codegen-3.spec.ts +++ b/tests/library/inspector/cli-codegen-3.spec.ts @@ -163,44 +163,67 @@ test.describe('cli codegen', () => { ]); expect.soft(sources.get('JavaScript')!.text).toContain(` - await page.frame({ - name: 'one' - }).getByText('HelloNameOne').click();`); + await page.frameLocator('#frame1').frameLocator('iframe').frameLocator('iframe[name="one"]').getByText('HelloNameOne').click();`); expect.soft(sources.get('Java')!.text).toContain(` - page.frame("one").getByText("HelloNameOne").click();`); + page.frameLocator("#frame1").frameLocator("iframe").frameLocator("iframe[name=\\"one\\"]").getByText("HelloNameOne").click();`); expect.soft(sources.get('Python')!.text).toContain(` - page.frame(name=\"one\").get_by_text(\"HelloNameOne\").click()`); + page.frame_locator("#frame1").frame_locator("iframe").frame_locator("iframe[name=\\"one\\"]").get_by_text("HelloNameOne").click()`); expect.soft(sources.get('Python Async')!.text).toContain(` - await page.frame(name=\"one\").get_by_text(\"HelloNameOne\").click()`); + await page.frame_locator("#frame1").frame_locator("iframe").frame_locator("iframe[name=\\"one\\"]").get_by_text("HelloNameOne").click()`); expect.soft(sources.get('C#')!.text).toContain(` - await page.Frame(\"one\").GetByText(\"HelloNameOne\").ClickAsync();`); - + await page.FrameLocator("#frame1").FrameLocator("iframe").FrameLocator("iframe[name=\\"one\\"]").GetByText("HelloNameOne").ClickAsync();`); [sources] = await Promise.all([ - recorder.waitForOutput('JavaScript', 'url:'), + recorder.waitForOutput('JavaScript', 'HelloNameAnonymous'), frameAnonymous.click('text=HelloNameAnonymous'), ]); expect.soft(sources.get('JavaScript')!.text).toContain(` - await page.frame({ - url: 'about:blank' - }).getByText('HelloNameAnonymous').click();`); + await page.frameLocator('#frame1').frameLocator('iframe').frameLocator('iframe >> nth=2').getByText('HelloNameAnonymous').click();`); expect.soft(sources.get('Java')!.text).toContain(` - page.frameByUrl("about:blank").getByText("HelloNameAnonymous").click();`); + page.frameLocator("#frame1").frameLocator("iframe").frameLocator("iframe >> nth=2").getByText("HelloNameAnonymous").click();`); expect.soft(sources.get('Python')!.text).toContain(` - page.frame(url=\"about:blank\").get_by_text(\"HelloNameAnonymous\").click()`); + page.frame_locator("#frame1").frame_locator("iframe").frame_locator("iframe >> nth=2").get_by_text("HelloNameAnonymous").click()`); expect.soft(sources.get('Python Async')!.text).toContain(` - await page.frame(url=\"about:blank\").get_by_text(\"HelloNameAnonymous\").click()`); + await page.frame_locator("#frame1").frame_locator("iframe").frame_locator("iframe >> nth=2").get_by_text("HelloNameAnonymous").click()`); expect.soft(sources.get('C#')!.text).toContain(` - await page.FrameByUrl(\"about:blank\").GetByText(\"HelloNameAnonymous\").ClickAsync();`); + await page.FrameLocator("#frame1").FrameLocator("iframe").FrameLocator("iframe >> nth=2").GetByText("HelloNameAnonymous").ClickAsync();`); + }); + + test('should generate frame locators with special characters in name attribute', async ({ page, openRecorder, server }) => { + const recorder = await openRecorder(); + await recorder.setContentAndWait(` +