fix(aria snapshot): assorted fixes (#33512)

packages/playwright-core/src/server/injected/ariaSnapshot.ts

@@ -292,7 +292,7 @@ export function renderAriaTree(ariaNode: AriaNode, options?: { mode?: 'raw' | 'r
     if (typeof ariaNode === 'string') {
       if (parentAriaNode && !includeText(parentAriaNode, ariaNode))
         return;
-      const text = renderString(ariaNode);
+      const text = yamlEscapeValueIfNeeded(renderString(ariaNode));
       if (text)
         lines.push(indent + '- text: ' + text);
       return;
@@ -399,8 +399,8 @@ function textContributesInfo(node: AriaNode, text: string): boolean {
   if (node.name.length > text.length)
     return false;
 
-  // Figure out if text adds any value.
+  // Figure out if text adds any value. "longestCommonSubstring" is expensive, so limit strings length.
-  const substr = longestCommonSubstring(text, node.name);
+  const substr = (text.length <= 200 && node.name.length <= 200) ? longestCommonSubstring(text, node.name) : '';
   let filtered = text;
   while (substr && filtered.includes(substr))
     filtered = filtered.replace(substr, '');

packages/playwright-core/src/server/injected/yaml.ts

@@ -86,6 +86,10 @@ function yamlStringNeedsQuotes(str: string): boolean {
   if (/^[>|]/.test(str))
     return true;
 
+  // Strings starting with quotes need quotes
+  if (/^["']/.test(str))
+    return true;
+
   // Strings containing special characters that could cause ambiguity
   if (/[{}`]/.test(str))
     return true;

packages/playwright-core/src/utils/isomorphic/ariaSnapshot.ts

@@ -138,14 +138,18 @@ class KeyParser {
     return this._pos >= this._length;
   }
 
+  private _isWhitespace() {
+    return !this._eof() && /\s/.test(this._peek());
+  }
+
   private _skipWhitespace() {
-    while (!this._eof() && /\s/.test(this._peek()))
+    while (this._isWhitespace())
       this._pos++;
   }
 
-  private _readIdentifier(): string {
+  private _readIdentifier(type: 'role' | 'attribute'): string {
     if (this._eof())
-      this._throwError('Unexpected end of input when expecting identifier');
+      this._throwError(`Unexpected end of input when expecting ${type}`);
     const start = this._pos;
     while (!this._eof() && /[a-zA-Z]/.test(this._peek()))
       this._pos++;
@@ -178,6 +182,7 @@ class KeyParser {
   private _readRegex(): string {
     let result = '';
     let escaped = false;
+    let insideClass = false;
     while (!this._eof()) {
       const ch = this._next();
       if (escaped) {
@@ -186,8 +191,14 @@ class KeyParser {
       } else if (ch === '\\') {
         escaped = true;
         result += ch;
-      } else if (ch === '/') {
+      } else if (ch === '/' && !insideClass) {
         return result;
+      } else if (ch === '[') {
+        insideClass = true;
+        result += ch;
+      } else if (ch === ']' && insideClass) {
+        result += ch;
+        insideClass = false;
       } else {
         result += ch;
       }
@@ -218,14 +229,14 @@ class KeyParser {
         this._next();
         this._skipWhitespace();
         errorPos = this._pos;
-        const flagName = this._readIdentifier();
+        const flagName = this._readIdentifier('attribute');
         this._skipWhitespace();
         let flagValue = '';
         if (this._peek() === '=') {
           this._next();
           this._skipWhitespace();
           errorPos = this._pos;
-          while (this._peek() !== ']' && !this._eof())
+          while (this._peek() !== ']' && !this._isWhitespace() && !this._eof())
             flagValue += this._next();
         }
         this._skipWhitespace();
@@ -243,7 +254,7 @@ class KeyParser {
   _parse(): AriaTemplateNode {
     this._skipWhitespace();
 
-    const role = this._readIdentifier() as AriaTemplateRoleNode['role'];
+    const role = this._readIdentifier('role') as AriaTemplateRoleNode['role'];
     this._skipWhitespace();
     const name = this._readStringOrRegex() || '';
     const result: AriaTemplateRoleNode = { kind: 'role', role, name };

tests/page/page-aria-snapshot.spec.ts

@@ -459,3 +459,36 @@ it('should be ok with circular ownership', async ({ page }) => {
       - region: Hello
   `);
 });
+
+it('should escape yaml text in text nodes', async ({ page }) => {
+  await page.setContent(`
+    <details>
+      <summary>one: <a href="#">link1</a> "two <a href="#">link2</a> 'three <a href="#">link3</a> \`four</summary>
+    </details>
+  `);
+
+  await checkAndMatchSnapshot(page.locator('body'), `
+    - group:
+      - text: "one:"
+      - link "link1"
+      - text: "\\\"two"
+      - link "link2"
+      - text: "'three"
+      - link "link3"
+      - text: "\`four"
+  `);
+});
+
+it.fixme('should handle long strings', async ({ page }) => {
+  await page.setContent(`
+    <a href='about:blank'>
+      <div role='region'>${'a'.repeat(100000)}</div>
+    </a>
+  `);
+
+  const trimmed = 'a'.repeat(1000);
+  await checkAndMatchSnapshot(page.locator('body'), `
+    - link "${trimmed}":
+      - region: "${trimmed}"
+  `);
+});

tests/page/to-match-aria-snapshot.spec.ts

@@ -76,10 +76,30 @@ test('should match complex', async ({ page }) => {
 });
 
 test('should match regex', async ({ page }) => {
+  {
     await page.setContent(`<h1>Issues 12</h1>`);
     await expect(page.locator('body')).toMatchAriaSnapshot(`
       - heading ${/Issues \d+/}
     `);
+  }
+  {
+    await page.setContent(`<h1>Issues 1/2</h1>`);
+    await expect(page.locator('body')).toMatchAriaSnapshot(`
+      - heading ${/Issues 1[/]2/}
+    `);
+  }
+  {
+    await page.setContent(`<h1>Issues 1[</h1>`);
+    await expect(page.locator('body')).toMatchAriaSnapshot(`
+      - heading ${/Issues 1\[/}
+    `);
+  }
+  {
+    await page.setContent(`<h1>Issues 1]]2</h1>`);
+    await expect(page.locator('body')).toMatchAriaSnapshot(`
+      - heading ${/Issues 1[\]]]2/}
+    `);
+  }
 });
 
 test('should allow text nodes', async ({ page }) => {
@@ -472,6 +492,26 @@ test('should unpack escaped names', async ({ page }) => {
       - 'button "Click '' me"'
     `);
   }
+
+  {
+    await page.setContent(`
+      <h1>heading "name" [level=1]</h1>
+    `);
+    await expect(page.locator('body')).toMatchAriaSnapshot(`
+      - heading "heading \\"name\\" [level=1]" [level=1]
+    `);
+  }
+
+  {
+    await page.setContent(`
+      <h1>heading \\" [level=2]</h1>
+    `);
+    await expect(page.locator('body')).toMatchAriaSnapshot(`
+      - |
+          heading    "heading \\\\\\" [level=2]" [
+             level  =   1   ]
+    `);
+  }
 });
 
 test('should report error in YAML', async ({ page }) => {
@@ -599,3 +639,34 @@ test('call log should contain actual snapshot', async ({ page }) => {
 
   expect(stripAnsi(error.message)).toContain(`- unexpected value "- heading "todos" [level=1]"`);
 });
+
+test.fixme('should normalize whitespace when matching accessible name', async ({ page }) => {
+  await page.setContent(`
+    <button>hello world</button>
+  `);
+  await expect(page.locator('body')).toMatchAriaSnapshot(`
+    - |
+        button "hello
+          world"
+  `);
+});
+
+test('should parse attributes', async ({ page }) => {
+  {
+    await page.setContent(`
+      <button aria-pressed="mixed">hello world</button>
+    `);
+    await expect(page.locator('body')).toMatchAriaSnapshot(`
+      - button [pressed=mixed ]
+    `);
+  }
+
+  {
+    await page.setContent(`
+      <h2>hello world</h2>
+    `);
+    await expect(page.locator('body')).not.toMatchAriaSnapshot(`
+      - heading [level =  -3 ]
+    `);
+  }
+});

tests/playwright-test/update-aria-snapshot.spec.ts

@@ -20,6 +20,10 @@ import { execSync } from 'child_process';
 
 test.describe.configure({ mode: 'parallel' });
 
+function trimPatch(patch: string) {
+  return patch.split('\n').map(line => line.trimEnd()).join('\n');
+}
+
 test('should update snapshot with the update-snapshots flag', async ({ runInlineTest }, testInfo) => {
   const result = await runInlineTest({
     'a.spec.ts': `
@@ -36,7 +40,7 @@ test('should update snapshot with the update-snapshots flag', async ({ runInline
   expect(result.exitCode).toBe(0);
   const patchPath = testInfo.outputPath('test-results/rebaselines.patch');
   const data = fs.readFileSync(patchPath, 'utf-8');
-  expect(data).toBe(`--- a/a.spec.ts
+  expect(trimPatch(data)).toBe(`--- a/a.spec.ts
 +++ b/a.spec.ts
 @@ -3,7 +3,7 @@
        test('test', async ({ page }) => {
@@ -83,7 +87,7 @@ test('should update missing snapshots', async ({ runInlineTest }, testInfo) => {
 
   const patchPath = testInfo.outputPath('test-results/rebaselines.patch');
   const data = fs.readFileSync(patchPath, 'utf-8');
-  expect(data).toBe(`--- a/a.spec.ts
+  expect(trimPatch(data)).toBe(`--- a/a.spec.ts
 +++ b/a.spec.ts
 @@ -2,6 +2,8 @@
        import { test, expect } from '@playwright/test';
@@ -127,7 +131,7 @@ test('should generate baseline with regex', async ({ runInlineTest }, testInfo)
   expect(result.exitCode).toBe(0);
   const patchPath = testInfo.outputPath('test-results/rebaselines.patch');
   const data = fs.readFileSync(patchPath, 'utf-8');
-  expect(data).toBe(`--- a/a.spec.ts
+  expect(trimPatch(data)).toBe(`--- a/a.spec.ts
 +++ b/a.spec.ts
 @@ -13,6 +13,18 @@
            <li>/Regex 1/</li>
@@ -162,6 +166,10 @@ test('should generate baseline with special characters', async ({ runInlineTest
       import { test, expect } from '@playwright/test';
       test('test', async ({ page }) => {
         await page.setContent(\`<ul>
+          <details>
+            <summary>one: <a href="#">link1</a> "two <a href="#">link2</a> 'three <a href="#">link3</a> \\\`four</summary>
+          </details>
+          <h1>heading "name" [level=1]</h1>
           <button>Click: me</button>
           <button>Click: 123</button>
           <button>Click ' me</button>
@@ -181,15 +189,24 @@ test('should generate baseline with special characters', async ({ runInlineTest
   expect(result.exitCode).toBe(0);
   const patchPath = testInfo.outputPath('test-results/rebaselines.patch');
   const data = fs.readFileSync(patchPath, 'utf-8');
-  expect(data).toBe(`--- a/a.spec.ts
+  expect(trimPatch(data)).toBe(`--- a/a.spec.ts
 +++ b/a.spec.ts
-@@ -13,6 +13,18 @@
+@@ -17,6 +17,27 @@
            <li>Item: 1</li>
            <li>Item {a: b}</li>
          </ul>\`);
 -        await expect(page.locator('body')).toMatchAriaSnapshot(\`\`);
 +        await expect(page.locator('body')).toMatchAriaSnapshot(\`
 +          - list:
++            - group:
++              - text: "one:"
++              - link "link1"
++              - text: "\\\\\"two"
++              - link "link2"
++              - text: "'three"
++              - link "link3"
++              - text: "\\\`four"
++            - heading "heading \\\\"name\\\\" [level=1]" [level=1]
 +            - 'button "Click: me"'
 +            - 'button /Click: \\\\d+/'
 +            - button "Click ' me"
@@ -234,7 +251,7 @@ test('should update missing snapshots in tsx', async ({ runInlineTest }, testInf
   expect(result.exitCode).toBe(0);
   const patchPath = testInfo.outputPath('test-results/rebaselines.patch');
   const data = fs.readFileSync(patchPath, 'utf-8');
-  expect(data).toBe(`--- a/src/button.test.tsx
+  expect(trimPatch(data)).toBe(`--- a/src/button.test.tsx
 +++ b/src/button.test.tsx
 @@ -4,6 +4,8 @@
 
@@ -296,7 +313,7 @@ test('should update multiple files', async ({ runInlineTest }, testInfo) => {
 
   const patchPath = testInfo.outputPath('test-results/rebaselines.patch');
   const data = fs.readFileSync(patchPath, 'utf-8');
-  expect(data).toBe(`--- a/src/button-1.test.tsx
+  expect(trimPatch(data)).toBe(`--- a/src/button-1.test.tsx
 +++ b/src/button-1.test.tsx
 @@ -4,6 +4,8 @@
 
@@ -342,7 +359,7 @@ test('should generate baseline for input values', async ({ runInlineTest }, test
   expect(result.exitCode).toBe(0);
   const patchPath = testInfo.outputPath('test-results/rebaselines.patch');
   const data = fs.readFileSync(patchPath, 'utf-8');
-  expect(data).toBe(`--- a/a.spec.ts
+  expect(trimPatch(data)).toBe(`--- a/a.spec.ts
 +++ b/a.spec.ts
 @@ -2,6 +2,8 @@
        import { test, expect } from '@playwright/test';