From d4ad520a9bf0fea78b610c065af0b0c896229666 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Tue, 5 Nov 2024 16:22:02 -0800 Subject: [PATCH] chore: fix more aria escaping edge cases (#33460) --- .../playwright-core/src/server/ariaSnapshot.ts | 1 - .../src/server/injected/ariaSnapshot.ts | 8 +++++--- .../playwright-core/src/server/injected/yaml.ts | 13 ------------- .../src/matchers/toMatchAriaSnapshot.ts | 17 ++++++----------- tests/page/to-match-aria-snapshot.spec.ts | 10 ++++++++-- .../update-aria-snapshot.spec.ts | 6 +++++- 6 files changed, 24 insertions(+), 31 deletions(-) diff --git a/packages/playwright-core/src/server/ariaSnapshot.ts b/packages/playwright-core/src/server/ariaSnapshot.ts index 54d6bb2981..6a20b2ef82 100644 --- a/packages/playwright-core/src/server/ariaSnapshot.ts +++ b/packages/playwright-core/src/server/ariaSnapshot.ts @@ -168,7 +168,6 @@ export class KeyParser { escaped = false; } else if (ch === '\\') { escaped = true; - result += ch; } else if (ch === '"') { return result; } else { diff --git a/packages/playwright-core/src/server/injected/ariaSnapshot.ts b/packages/playwright-core/src/server/injected/ariaSnapshot.ts index fa0f14343c..9078459dfb 100644 --- a/packages/playwright-core/src/server/injected/ariaSnapshot.ts +++ b/packages/playwright-core/src/server/injected/ariaSnapshot.ts @@ -18,7 +18,7 @@ import * as roleUtils from './roleUtils'; import { getElementComputedStyle } from './domUtils'; import type { AriaRole } from './roleUtils'; import { escapeRegExp, longestCommonSubstring } from '@isomorphic/stringUtils'; -import { yamlEscapeKeyIfNeeded, yamlEscapeValueIfNeeded, yamlQuoteFragment } from './yaml'; +import { yamlEscapeKeyIfNeeded, yamlEscapeValueIfNeeded } from './yaml'; type AriaProps = { checked?: boolean | 'mixed'; @@ -318,8 +318,10 @@ export function renderAriaTree(ariaNode: AriaNode, options?: { mode?: 'raw' | 'r let key = ariaNode.role; if (ariaNode.name) { const name = renderString(ariaNode.name); - if (name) - key += ' ' + (name.startsWith('/') && name.endsWith('/') ? name : yamlQuoteFragment(name)); + if (name) { + const stringifiedName = name.startsWith('/') && name.endsWith('/') ? name : JSON.stringify(name); + key += ' ' + stringifiedName; + } } if (ariaNode.checked === 'mixed') key += ` [checked=mixed]`; diff --git a/packages/playwright-core/src/server/injected/yaml.ts b/packages/playwright-core/src/server/injected/yaml.ts index 97bf3a070d..58ebd9d788 100644 --- a/packages/playwright-core/src/server/injected/yaml.ts +++ b/packages/playwright-core/src/server/injected/yaml.ts @@ -46,19 +46,6 @@ export function yamlEscapeValueIfNeeded(str: string): string { }) + '"'; } -export function yamlQuoteFragment(str: string, quote = '"'): string { - return quote + str.replace(/['"]/g, c => { - switch (c) { - case '"': - return quote === '"' ? '\\"' : '"'; - case '\'': - return quote === '\'' ? '\\\'' : '\''; - default: - return c; - } - }) + quote; -} - function yamlStringNeedsQuotes(str: string): boolean { if (str.length === 0) return true; diff --git a/packages/playwright/src/matchers/toMatchAriaSnapshot.ts b/packages/playwright/src/matchers/toMatchAriaSnapshot.ts index a475ec39d8..47e9f70441 100644 --- a/packages/playwright/src/matchers/toMatchAriaSnapshot.ts +++ b/packages/playwright/src/matchers/toMatchAriaSnapshot.ts @@ -85,19 +85,18 @@ export async function toMatchAriaSnapshot( }; } - const escapedExpected = escapePrivateUsePoints(expected); - const escapedReceived = escapePrivateUsePoints(typedReceived.raw); + const receivedText = typedReceived.raw; const message = () => { if (pass) { if (notFound) - return messagePrefix + `Expected: not ${this.utils.printExpected(escapedExpected)}\nReceived: ${escapedReceived}` + callLogText(log); - const printedReceived = printReceivedStringContainExpectedSubstring(escapedReceived, escapedReceived.indexOf(escapedExpected), escapedExpected.length); - return messagePrefix + `Expected: not ${this.utils.printExpected(escapedExpected)}\nReceived: ${printedReceived}` + callLogText(log); + return messagePrefix + `Expected: not ${this.utils.printExpected(expected)}\nReceived: ${receivedText}` + callLogText(log); + const printedReceived = printReceivedStringContainExpectedSubstring(receivedText, receivedText.indexOf(expected), expected.length); + return messagePrefix + `Expected: not ${this.utils.printExpected(expected)}\nReceived: ${printedReceived}` + callLogText(log); } else { const labelExpected = `Expected`; if (notFound) - return messagePrefix + `${labelExpected}: ${this.utils.printExpected(escapedExpected)}\nReceived: ${escapedReceived}` + callLogText(log); - return messagePrefix + this.utils.printDiffOrStringify(escapedExpected, escapedReceived, labelExpected, 'Received', false) + callLogText(log); + return messagePrefix + `${labelExpected}: ${this.utils.printExpected(expected)}\nReceived: ${receivedText}` + callLogText(log); + return messagePrefix + this.utils.printDiffOrStringify(expected, receivedText, labelExpected, 'Received', false) + callLogText(log); } }; @@ -118,10 +117,6 @@ export async function toMatchAriaSnapshot( }; } -function escapePrivateUsePoints(str: string) { - return escapeTemplateString(str).replace(/[\uE000-\uF8FF]/g, char => `\\u${char.charCodeAt(0).toString(16).padStart(4, '0')}`); -} - function unshift(snapshot: string): string { const lines = snapshot.split('\n'); let whitespacePrefixLength = 100; diff --git a/tests/page/to-match-aria-snapshot.spec.ts b/tests/page/to-match-aria-snapshot.spec.ts index 5adc7e7da5..3b226f480a 100644 --- a/tests/page/to-match-aria-snapshot.spec.ts +++ b/tests/page/to-match-aria-snapshot.spec.ts @@ -436,10 +436,16 @@ test('should unpack escaped names', async ({ page }) => { { await page.setContent(` - + `); await expect(page.locator('body')).toMatchAriaSnapshot(` - - button "Click \\ me" + - button "Click \\\" me" + `); + } + + { + await page.setContent(` + `); await expect(page.locator('body')).toMatchAriaSnapshot(` - button /Click \\\\ me/ diff --git a/tests/playwright-test/update-aria-snapshot.spec.ts b/tests/playwright-test/update-aria-snapshot.spec.ts index 1e20bc1b3d..d58bfce899 100644 --- a/tests/playwright-test/update-aria-snapshot.spec.ts +++ b/tests/playwright-test/update-aria-snapshot.spec.ts @@ -166,6 +166,8 @@ test('should generate baseline with special characters', async ({ runInlineTest + +
  • Item: 1
  • Item {a: b}
    • \`); @@ -179,7 +181,7 @@ test('should generate baseline with special characters', async ({ runInlineTest const data = fs.readFileSync(patchPath, 'utf-8'); expect(data).toBe(`--- a/a.spec.ts +++ b/a.spec.ts -@@ -9,6 +9,14 @@ +@@ -11,6 +11,16 @@
  • Item: 1
  • Item {a: b}
    • \`); @@ -190,6 +192,8 @@ test('should generate baseline with special characters', async ({ runInlineTest + - 'button /Click: \\\\d+/' + - button "Click ' me" + - 'button "Click: '' me"' ++ - button "Click \\\\" me" ++ - button "Click \\\\\\\\ me" + - listitem: \"Item: 1\" + - listitem: \"Item {a: b}\" + \`);