Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add presentational check when extracting images #765

Merged
merged 6 commits into from
Mar 31, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/image-alt-text/auditEngine.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ export default class AuditEngine {
}

pageTags.images.forEach((image) => {
if (image.isPresentational) {
return;
}

if (!hasText(image.alt?.trim())) {
this.auditedTags.imagesWithoutAltText.set(image.src, {
pageUrl,
Expand Down Expand Up @@ -151,6 +155,8 @@ export default class AuditEngine {
}

getAuditedTags() {
return { imagesWithoutAltText: Array.from(this.auditedTags.imagesWithoutAltText.values()) };
return {
imagesWithoutAltText: Array.from(this.auditedTags.imagesWithoutAltText.values()),
};
}
}
12 changes: 12 additions & 0 deletions src/image-alt-text/handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,17 @@ import { noopUrlResolver } from '../common/index.js';
import convertToOpportunity from './opportunityHandler.js';

const AUDIT_TYPE = AuditModel.AUDIT_TYPES.ALT_TEXT;

const isImagePresentational = (img) => {
const isHiddenForScreenReader = img.getAttribute('aria-hidden') === 'true';
const hasRolePresentation = img.getAttribute('role') === 'presentation';
const hasAltAttribute = img.hasAttribute('alt');
// For presentational images, an image MUST have the alt attribute WITH a falsy value
// Not having it at all is not the same, the image is not considered presentational
const isAltEmpty = hasAltAttribute && !img.getAttribute('alt');
return isHiddenForScreenReader || hasRolePresentation || isAltEmpty;
};

export async function fetchAndProcessPageObject(
s3Client,
bucketName,
Expand All @@ -40,6 +51,7 @@ export async function fetchAndProcessPageObject(
const dom = new JSDOM(object.scrapeResult.rawBody);
const imageElements = dom.window.document.getElementsByTagName('img');
const images = Array.from(imageElements).map((img) => ({
isPresentational: isImagePresentational(img),
src: img.getAttribute('src'),
alt: img.getAttribute('alt'),
})).filter((img) => img.src);
Expand Down
128 changes: 128 additions & 0 deletions test/audits/image-alt-text/auditEngine.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
/*
* Copyright 2025 Adobe. All rights reserved.
* This file is licensed to you under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. You may obtain a copy
* of the License at http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under
* the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
* OF ANY KIND, either express or implied. See the License for the specific language
* governing permissions and limitations under the License.
*/

/* eslint-env mocha */
import { expect } from 'chai';
import sinon from 'sinon';
import AuditEngine from '../../../src/image-alt-text/auditEngine.js';

describe('Image Alt Text Audit Engine', () => {
let logStub;
let auditEngine;

beforeEach(() => {
sinon.restore();

logStub = {
info: sinon.stub(),
debug: sinon.stub(),
error: sinon.stub(),
};

auditEngine = new AuditEngine(logStub);
});

afterEach(() => {
sinon.restore();
});

it('should skip presentational images when auditing for alt text', () => {
// Setup test data with a mix of presentational and non-presentational images
const pageUrl = '/test-page';
const pageTags = {
images: [
// Image 1: Presentational image without alt text - should be skipped
{
isPresentational: true,
src: '/image1.jpg',
alt: null,
},
// Image 2: Presentational image with alt text - should still be skipped
{
isPresentational: true,
src: '/image2.jpg',
alt: 'This alt text is redundant for presentational images',
},
// Image 3: Non-presentational image without alt text - should be flagged
{
isPresentational: false,
src: '/image3.jpg',
alt: null,
},
// Image 4: Non-presentational image with alt text - should be fine
{
isPresentational: false,
src: '/image4.jpg',
alt: 'This is proper alt text',
},
// Image 5: Non-presentational image with empty alt text - should be flagged
{
isPresentational: false,
src: '/image5.jpg',
alt: '',
},
],
};

// Perform the audit
auditEngine.performPageAudit(pageUrl, pageTags);

// Finalize audit to ensure logs are generated
auditEngine.finalizeAudit();

// Get audited tags
const auditedTags = auditEngine.getAuditedTags();

// Verify that only the non-presentational images without alt text were flagged
expect(auditedTags.imagesWithoutAltText).to.have.lengthOf(2);

// Verify that Image 3 (non-presentational without alt) is in the results
expect(auditedTags.imagesWithoutAltText.some((img) => img.src === '/image3.jpg')).to.be.true;

// Verify that Image 5 (non-presentational with empty alt) is in the results
expect(auditedTags.imagesWithoutAltText.some((img) => img.src === '/image5.jpg')).to.be.true;

// Verify that Image 1 and Image 2 (presentational) are NOT in the results
expect(auditedTags.imagesWithoutAltText.some((img) => img.src === '/image1.jpg')).to.be.false;
expect(auditedTags.imagesWithoutAltText.some((img) => img.src === '/image2.jpg')).to.be.false;

// Verify that Image 4 (with alt text) is NOT in the results
expect(auditedTags.imagesWithoutAltText.some((img) => img.src === '/image4.jpg')).to.be.false;

// Verify correct logging
expect(logStub.info.calledWith(sinon.match(/Found 2 images without alt text/))).to.be.true;
});

it('should handle empty image arrays', () => {
const pageUrl = '/empty-page';
const pageTags = { images: [] };

auditEngine.performPageAudit(pageUrl, pageTags);
auditEngine.finalizeAudit();

const auditedTags = auditEngine.getAuditedTags();
expect(auditedTags.imagesWithoutAltText).to.have.lengthOf(0);
expect(logStub.debug.calledWith(sinon.match(/No images found for page/))).to.be.true;
});

it('should handle undefined images', () => {
const pageUrl = '/no-images-page';
const pageTags = { notImages: [] }; // No images key

auditEngine.performPageAudit(pageUrl, pageTags);
auditEngine.finalizeAudit();

const auditedTags = auditEngine.getAuditedTags();
expect(auditedTags.imagesWithoutAltText).to.have.lengthOf(0);
expect(logStub.debug.calledWith(sinon.match(/No images found for page/))).to.be.true;
});
});
4 changes: 2 additions & 2 deletions test/audits/image-alt-text/handler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ describe('Image Alt Text Handler', () => {
<html>
<body>
<img src="test1.jpg" alt="Test 1">
<img src="test2.jpg" alt="">
<img src="test2.jpg">
</body>
</html>
`;
Expand Down Expand Up @@ -143,7 +143,7 @@ describe('Image Alt Text Handler', () => {
<html>
<body>
<img src="test1.jpg" alt="Test 1">
<img src="test2.jpg" alt="">
<img src="test2.jpg">
</body>
</html>
`;
Expand Down