feat(image-alt): require alt text or empty strings #1260
Conversation
The image-alt rule now checks for space characters, and fails if they are present. Empty alt attributes are still fine, and text content is still fine. This fixes cases where ATs do not skip decorative images because of the space characters in the alt attribute. Closes #1174
marcysutton
force-pushed
the
image-alt-space
branch
from
37962e2
to
c5682ce
Compare
(there are no actual changes)
lib/checks/label/alt-empty-space.js
Outdated
| @@ -0,0 +1,5 @@ | |||
| let nn = node.nodeName.toLowerCase(); | |||
| let validSetup = | |||
| node.hasAttribute('alt') && (nn === 'img' || nn === 'input' || nn === 'area'); | |||
There was a problem hiding this comment.
The rule selector will not include <input> and <area> elements. Why not make this a new, separate rule altogether?
There was a problem hiding this comment.
This check doesn't need to include input or area, so that part can be removed. This only applies to image alt, so in my opinion it should not be a separate rule. input and area both use the existing check non-empty-alt.
There was a problem hiding this comment.
I agree with Marcy on this one. I think this one is better as a check that's part of image-alt, provided that we split off the [role=img], which we have a separate support ticket for.
|
Waiting for @marcysutton to update. Putting this into WIP. |
WilcoFiers
changed the title
|
PR updated and ready to go. |
lib/checks/label/alt-empty-space.js
Outdated
| let validSetup = | ||
| node.hasAttribute('alt') && (nn === 'img' || nn === 'input' || nn === 'area'); | ||
| let validAttrValue = /^\s+$/.test(node.getAttribute('alt')); | ||
| return validSetup && validAttrValue; |
There was a problem hiding this comment.
Nitpick. Best to use const instead of let when you're not reassigning the variable. It's just as easy to do and it creates greater security.
| "impact": "critical", | ||
| "messages": { | ||
| "pass": "Element has a valid alt attribute value", | ||
| "fail": "Element has an alt attribute containing only a space character" |
There was a problem hiding this comment.
I'd prefer that we be explicit that alt attributes with white space only is not ignored by all screen readers. WDYT?
There was a problem hiding this comment.
That's fine by me, I've updated the PR.
WilcoFiers
changed the title
|
I've updated this PR, I don't understand what's happening with the rule-descriptions though. Git is acting really weird with that file, I tried adding/committing it and it keeps popping up again. |
This change to the image-alt rule accounts for space characters, and fails if they are present. Empty alt attributes are still fine, and text content is still fine. This fixes cases where ATs do not skip decorative images because of the space characters in the alt attribute.
I'm open to name changes for the check, so please review.
Closes #1174
Reviewer checks
Required fields, to be filled out by PR reviewer(s)