From fb83d4b42c1ee6671aa1462637e03db7dc0f0404 Mon Sep 17 00:00:00 2001
From: Brian Rhoten <brhoten@users.noreply.github.com>
Date: Tue, 22 Mar 2022 16:56:33 -0400
Subject: [PATCH] fix(codegen): do not consider empty attributes for selector
 generation (#12880)

Co-authored-by: Brian Rhoten <brhoten@appriss.com>
---
 .../src/server/injected/selectorGenerator.ts  |  6 ++--
 tests/selector-generator.spec.ts              | 31 +++++++++++++++++++
 2 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/packages/playwright-core/src/server/injected/selectorGenerator.ts b/packages/playwright-core/src/server/injected/selectorGenerator.ts
index b2b61cb943..a22a16fa13 100644
--- a/packages/playwright-core/src/server/injected/selectorGenerator.ts
+++ b/packages/playwright-core/src/server/injected/selectorGenerator.ts
@@ -144,7 +144,7 @@ function generateSelectorFor(injectedScript: InjectedScript, targetElement: Elem
 function buildCandidates(injectedScript: InjectedScript, element: Element): SelectorToken[] {
   const candidates: SelectorToken[] = [];
   for (const attribute of ['data-testid', 'data-test-id', 'data-test']) {
-    if (element.hasAttribute(attribute))
+    if (element.getAttribute(attribute))
       candidates.push({ engine: 'css', selector: `[${attribute}=${quoteAttributeValue(element.getAttribute(attribute)!)}]`, score: 1 });
   }
 
@@ -153,12 +153,12 @@ function buildCandidates(injectedScript: InjectedScript, element: Element): Sele
     if (input.placeholder)
       candidates.push({ engine: 'css', selector: `[placeholder=${quoteAttributeValue(input.placeholder)}]`, score: 10 });
   }
-  if (element.hasAttribute('aria-label'))
+  if (element.getAttribute('aria-label'))
     candidates.push({ engine: 'css', selector: `[aria-label=${quoteAttributeValue(element.getAttribute('aria-label')!)}]`, score: 10 });
   if (element.getAttribute('alt') && ['APPLET', 'AREA', 'IMG', 'INPUT'].includes(element.nodeName))
     candidates.push({ engine: 'css', selector: `${cssEscape(element.nodeName.toLowerCase())}[alt=${quoteAttributeValue(element.getAttribute('alt')!)}]`, score: 10 });
 
-  if (element.hasAttribute('role'))
+  if (element.getAttribute('role'))
     candidates.push({ engine: 'css', selector: `${cssEscape(element.nodeName.toLowerCase())}[role=${quoteAttributeValue(element.getAttribute('role')!)}]` , score: 50 });
 
   if (element.getAttribute('name') && ['BUTTON', 'FORM', 'FIELDSET', 'IFRAME', 'INPUT', 'KEYGEN', 'OBJECT', 'OUTPUT', 'SELECT', 'TEXTAREA', 'MAP', 'META', 'PARAM'].includes(element.nodeName))
diff --git a/tests/selector-generator.spec.ts b/tests/selector-generator.spec.ts
index dd62ae6470..8e31efd025 100644
--- a/tests/selector-generator.spec.ts
+++ b/tests/selector-generator.spec.ts
@@ -306,4 +306,35 @@ it.describe('selector generator', () => {
     });
     expect(await generate(page, 'button')).toBe(`button[name="-tricky\\1 name"]`);
   });
+
+  it('should ignore empty aria-label for candidate consideration', async ({ page }) => {
+    await page.setContent(`<button aria-label="" id="buttonId"></button>`);
+    expect(await generate(page, 'button')).toBe('#buttonId');
+  });
+
+  it('should accept valid aria-label for candidate consideration', async ({ page }) => {
+    await page.setContent(`<button aria-label="ariaLabel" id="buttonId"></button>`);
+    expect(await generate(page, 'button')).toBe('[aria-label="ariaLabel"]');
+  });
+
+  it('should ignore empty role for candidate consideration', async ({ page }) => {
+    await page.setContent(`<button role="" id="buttonId"></button>`);
+    expect(await generate(page, 'button')).toBe('#buttonId');
+  });
+
+  it('should accept valid role for candidate consideration', async ({ page }) => {
+    await page.setContent(`<button role="roleDescription" id="buttonId"></button>`);
+    expect(await generate(page, 'button')).toBe('button[role="roleDescription"]');
+  });
+
+  it('should ignore empty data-test-id for candidate consideration', async ({ page }) => {
+    await page.setContent(`<button data-test-id="" id="buttonId"></button>`);
+    expect(await generate(page, 'button')).toBe('#buttonId');
+  });
+
+  it('should accept valid data-test-id for candidate consideration', async ({ page }) => {
+    await page.setContent(`<button data-test-id="testId" id="buttonId"></button>`);
+    expect(await generate(page, 'button')).toBe('[data-test-id="testId"]');
+  });
+
 });