From a3778b5c399a792d125eb5155f36b675da375c87 Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Fri, 8 Nov 2024 14:05:40 +0000
Subject: [PATCH] fix(aria snapshot): assorted fixes

- RegExp parsing
- text value escaping
- limit string lengths in `longestCommonSubstring`
- failing tests for unresolved edge cases
---
 .../src/server/ariaSnapshot.ts                | 25 ++++--
 .../src/server/injected/ariaSnapshot.ts       |  6 +-
 .../src/server/injected/yaml.ts               |  4 +
 tests/page/page-aria-snapshot.spec.ts         | 33 ++++++++
 tests/page/to-match-aria-snapshot.spec.ts     | 79 ++++++++++++++++++-
 5 files changed, 133 insertions(+), 14 deletions(-)

diff --git a/packages/playwright-core/src/server/ariaSnapshot.ts b/packages/playwright-core/src/server/ariaSnapshot.ts
index 6a20b2ef82a5b..5e3933e37c0f6 100644
--- a/packages/playwright-core/src/server/ariaSnapshot.ts
+++ b/packages/playwright-core/src/server/ariaSnapshot.ts
@@ -144,14 +144,18 @@ export 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++;
@@ -184,6 +188,7 @@ export class KeyParser {
   private _readRegex(): string {
     let result = '';
     let escaped = false;
+    let insideClass = false;
     while (!this._eof()) {
       const ch = this._next();
       if (escaped) {
@@ -192,8 +197,14 @@ export 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;
       }
@@ -223,13 +234,13 @@ export class KeyParser {
       if (this._peek() === '[') {
         this._next();
         this._skipWhitespace();
-        const flagName = this._readIdentifier();
+        const flagName = this._readIdentifier('attribute');
         this._skipWhitespace();
         let flagValue = '';
         if (this._peek() === '=') {
           this._next();
           this._skipWhitespace();
-          while (this._peek() !== ']' && !this._eof())
+          while (this._peek() !== ']' && !this._isWhitespace() && !this._eof())
             flagValue += this._next();
         }
         this._skipWhitespace();
@@ -248,7 +259,7 @@ export 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 };
diff --git a/packages/playwright-core/src/server/injected/ariaSnapshot.ts b/packages/playwright-core/src/server/injected/ariaSnapshot.ts
index 9078459dfb0c7..f3947600fee56 100644
--- a/packages/playwright-core/src/server/injected/ariaSnapshot.ts
+++ b/packages/playwright-core/src/server/injected/ariaSnapshot.ts
@@ -309,7 +309,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;
@@ -416,8 +416,8 @@ function textContributesInfo(node: AriaNode, text: string): boolean {
   if (node.name.length > text.length)
     return false;
 
-  // Figure out if text adds any value.
-  const substr = longestCommonSubstring(text, node.name);
+  // Figure out if text adds any value. "longestCommonSubstring" is expensive, so limit strings length.
+  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, '');
diff --git a/packages/playwright-core/src/server/injected/yaml.ts b/packages/playwright-core/src/server/injected/yaml.ts
index 58ebd9d7889cd..977591c1cc26b 100644
--- a/packages/playwright-core/src/server/injected/yaml.ts
+++ b/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;
diff --git a/tests/page/page-aria-snapshot.spec.ts b/tests/page/page-aria-snapshot.spec.ts
index e9156ebad87c0..22ae280b4d701 100644
--- a/tests/page/page-aria-snapshot.spec.ts
+++ b/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}"
+  `);
+});
diff --git a/tests/page/to-match-aria-snapshot.spec.ts b/tests/page/to-match-aria-snapshot.spec.ts
index 754f3fd2a9e62..d7d9295617c70 100644
--- a/tests/page/to-match-aria-snapshot.spec.ts
+++ b/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 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 }) => {
@@ -579,3 +619,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 ]
+    `);
+  }
+});