From d89d507e4a34213e57675f9e2a0c81ba1da5f1ba Mon Sep 17 00:00:00 2001
From: jlp-craigmorten <craig.morten@waitrose.co.uk>
Date: Mon, 26 Jun 2023 09:17:13 +0100
Subject: [PATCH] fix: correctly handle img with empty alt

---
 .../__snapshots__/role-helpers.js.snap        | 26 +++++++++++++++++++
 src/__tests__/role-helpers.js                 | 26 ++++++++++++++-----
 src/role-helpers.js                           | 15 +++++++----
 3 files changed, 55 insertions(+), 12 deletions(-)

diff --git a/src/__tests__/__snapshots__/role-helpers.js.snap b/src/__tests__/__snapshots__/role-helpers.js.snap
index a917f225..75bbed7f 100644
--- a/src/__tests__/__snapshots__/role-helpers.js.snap
+++ b/src/__tests__/__snapshots__/role-helpers.js.snap
@@ -190,5 +190,31 @@ Name "":
   data-testid="a-dd"
 />
 
+--------------------------------------------------
+img:
+
+Name "":
+<img
+  data-testid="a-img-1"
+  src="http://example.com/image.png"
+/>
+
+Name "a meaningful description":
+<img
+  alt="a meaningful description"
+  data-testid="a-img-3"
+  src="http://example.com/image.png"
+/>
+
+--------------------------------------------------
+presentation:
+
+Name "":
+<img
+  alt=""
+  data-testid="a-img-2"
+  src="http://example.com/image.png"
+/>
+
 --------------------------------------------------
 `;
diff --git a/src/__tests__/role-helpers.js b/src/__tests__/role-helpers.js
index 5d60f55b..0cec4ea6 100644
--- a/src/__tests__/role-helpers.js
+++ b/src/__tests__/role-helpers.js
@@ -58,11 +58,15 @@ function setup() {
 
     <form data-testid="a-form" />
     <section data-testid="a-section" />
-   </article>
-   <dl>
+  </article>
+  <dl>
     <dt data-testid="a-dt">Term</dt>
     <dd data-testid="a-dd">Definition</dd>
-   </dl>
+  </dl>
+
+  <img src="http://example.com/image.png" data-testid='a-img-1'/>
+  <img alt="" src="http://example.com/image.png" data-testid='a-img-2'/>
+  <img alt="a meaningful description" src="http://example.com/image.png" data-testid='a-img-3'/>
 </section>
   `)
 
@@ -70,7 +74,6 @@ function setup() {
     unnamedSection: getByTestId('a-section'),
     namedSection: getByTestId('named-section'),
     anchor: getByTestId('a-link'),
-    invalidAnchor: getByTestId('invalid-link'),
     h1: getByTestId('a-h1'),
     h2: getByTestId('a-h2'),
     h3: getByTestId('a-h3'),
@@ -98,14 +101,16 @@ function setup() {
     dt: getByTestId('a-dt'),
     dd: getByTestId('a-dd'),
     header: getByTestId('a-header'),
+    invalidAnchor: getByTestId('invalid-link'),
+    unnamedImg: getByTestId('a-img-1'),
+    presentationImg: getByTestId('a-img-2'),
+    namedImg: getByTestId('a-img-3'),
   }
 }
 
 test('getRoles returns expected roles for various dom nodes', () => {
   const {
-    unnamedSection,
     anchor,
-    invalidAnchor,
     h1,
     h2,
     h3,
@@ -133,11 +138,15 @@ test('getRoles returns expected roles for various dom nodes', () => {
     dd,
     dt,
     header,
+    invalidAnchor,
+    unnamedSection,
+    unnamedImg,
+    presentationImg,
+    namedImg,
   } = setup()
 
   expect(getRoles(namedSection)).toEqual({
     link: [anchor],
-    generic: [invalidAnchor, unnamedSection],
     heading: [h1, h2, h3],
     navigation: [nav],
     radio: [radio, radio2],
@@ -153,6 +162,9 @@ test('getRoles returns expected roles for various dom nodes', () => {
     region: [namedSection],
     term: [dt],
     definition: [dd],
+    generic: [invalidAnchor, unnamedSection],
+    img: [unnamedImg, namedImg],
+    presentation: [presentationImg],
   })
   expect(getRoles(header)).toEqual({
     banner: [header],
diff --git a/src/role-helpers.js b/src/role-helpers.js
index bc134f27..8d7e7cb0 100644
--- a/src/role-helpers.js
+++ b/src/role-helpers.js
@@ -82,13 +82,18 @@ function buildElementRoleList(elementRolesMap) {
     return `${name}${attributes
       .map(({name: attributeName, value, constraints = []}) => {
         const shouldNotExist = constraints.indexOf('undefined') !== -1
-        if (shouldNotExist) {
-          return `:not([${attributeName}])`
-        } else if (value) {
+        const shouldBeNonEmpty = constraints.indexOf('set') !== -1
+        const hasExplicitValue = typeof value !== 'undefined'
+
+        if (hasExplicitValue) {
           return `[${attributeName}="${value}"]`
-        } else {
-          return `[${attributeName}]`
+        } else if (shouldNotExist) {
+          return `:not([${attributeName}])`
+        } else if (shouldBeNonEmpty) {
+          return `[${attributeName}]:not([${attributeName}=""])`
         }
+
+        return `[${attributeName}]`
       })
       .join('')}`
   }