From 9722f311decbb2e8d45d79ba933bcc8ff1f11514 Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Fri, 24 Jan 2025 15:19:00 +0000 Subject: [PATCH 01/10] Add no alt text warning transform --- packages/myst-transforms/src/basic.ts | 3 ++- packages/myst-transforms/src/images.ts | 18 ++++++++++++++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/packages/myst-transforms/src/basic.ts b/packages/myst-transforms/src/basic.ts index 2a0bd2c81..832c0d864 100644 --- a/packages/myst-transforms/src/basic.ts +++ b/packages/myst-transforms/src/basic.ts @@ -7,7 +7,7 @@ import { captionParagraphTransform } from './caption.js'; import { admonitionBlockquoteTransform, admonitionHeadersTransform } from './admonitions.js'; import { blockMetadataTransform, blockNestingTransform, blockToFigureTransform } from './blocks.js'; import { htmlIdsTransform } from './htmlIds.js'; -import { imageAltTextTransform } from './images.js'; +import { imageAltTextTransform, imageNoAltTextTransform } from './images.js'; import { mathLabelTransform, mathNestingTransform, subequationTransform } from './math.js'; import { blockquoteTransform } from './blockquote.js'; import { codeBlockToDirectiveTransform, inlineCodeFlattenTransform } from './code.js'; @@ -40,6 +40,7 @@ export function basicTransformations(tree: GenericParent, file: VFile, opts?: Re containerChildrenTransform(tree, file); htmlIdsTransform(tree); imageAltTextTransform(tree); + imageNoAltTextTransform(tree, file); blockquoteTransform(tree); removeUnicodeTransform(tree); headingDepthTransform(tree, file, opts); diff --git a/packages/myst-transforms/src/images.ts b/packages/myst-transforms/src/images.ts index de0e3214d..f160112ab 100644 --- a/packages/myst-transforms/src/images.ts +++ b/packages/myst-transforms/src/images.ts @@ -1,8 +1,9 @@ import type { Plugin } from 'unified'; import type { Container, Paragraph, PhrasingContent, Image } from 'myst-spec'; +import type { VFile} from 'vfile'; import { select, selectAll } from 'unist-util-select'; import type { GenericParent } from 'myst-common'; -import { toText } from 'myst-common'; +import { fileWarn, toText } from 'myst-common'; /** * Generate image alt text from figure caption @@ -29,6 +30,19 @@ export function imageAltTextTransform(tree: GenericParent) { }); } -export const imageAltTextPlugin: Plugin<[], GenericParent, GenericParent> = () => (tree) => { +export function imageNoAltTextTransform(tree: GenericParent, file: VFile) { + const imageNodes = selectAll("image", tree) as Image[]; + imageNodes.forEach((image, index) => { + if (image.alt == null) { + fileWarn( + file, + `missing alt text for ${image.url}`, + ); + } + }); +} + +export const imageAltTextPlugin: Plugin<[], GenericParent, GenericParent> = () => (tree, file) => { imageAltTextTransform(tree); + imageNoAltTextTransform(tree, file); }; From 4ba5672676881d71f9a8bdd9a383ef189180dc8c Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Fri, 24 Jan 2025 15:31:13 +0000 Subject: [PATCH 02/10] Add image-has-alt-text rule --- packages/myst-common/src/ruleids.ts | 2 ++ packages/myst-transforms/src/images.ts | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/myst-common/src/ruleids.ts b/packages/myst-common/src/ruleids.ts index d833a0dd0..8330994d3 100644 --- a/packages/myst-common/src/ruleids.ts +++ b/packages/myst-common/src/ruleids.ts @@ -48,6 +48,8 @@ export enum RuleId { imageFormatConverts = 'image-format-converts', imageCopied = 'image-copied', imageFormatOptimizes = 'image-format-optimizes', + imageHasAltText = 'image-has-alt-text', + imageAltTextGenerated = 'image-alt-text-generated', // Math rules mathLabelLifted = 'math-label-lifted', mathEquationEnvRemoved = 'math-equation-env-removed', diff --git a/packages/myst-transforms/src/images.ts b/packages/myst-transforms/src/images.ts index f160112ab..9b6747358 100644 --- a/packages/myst-transforms/src/images.ts +++ b/packages/myst-transforms/src/images.ts @@ -3,7 +3,9 @@ import type { Container, Paragraph, PhrasingContent, Image } from 'myst-spec'; import type { VFile} from 'vfile'; import { select, selectAll } from 'unist-util-select'; import type { GenericParent } from 'myst-common'; -import { fileWarn, toText } from 'myst-common'; +import { fileWarn, toText, RuleId } from 'myst-common'; + +const TRANSFORM_SOURCE = 'myst-transforms:images'; /** * Generate image alt text from figure caption @@ -37,6 +39,7 @@ export function imageNoAltTextTransform(tree: GenericParent, file: VFile) { fileWarn( file, `missing alt text for ${image.url}`, + {ruleId: RuleId.imageHasAltText, node: image, source: TRANSFORM_SOURCE}, ); } }); From 6e0e9a369ae6d1a2bbc8347370ad2747fb12c0e5 Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Fri, 24 Jan 2025 15:38:24 +0000 Subject: [PATCH 03/10] Add warning for auto-generated alt text --- packages/myst-transforms/src/images.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/myst-transforms/src/images.ts b/packages/myst-transforms/src/images.ts index 9b6747358..0715156b3 100644 --- a/packages/myst-transforms/src/images.ts +++ b/packages/myst-transforms/src/images.ts @@ -42,6 +42,13 @@ export function imageNoAltTextTransform(tree: GenericParent, file: VFile) { {ruleId: RuleId.imageHasAltText, node: image, source: TRANSFORM_SOURCE}, ); } + if (image.data?.altTextIsAutoGenerated) { + fileWarn( + file, + `alt text for ${image.url} was auto-generated`, + {ruleId: RuleId.imageAltTextGenerated, node: image, source: TRANSFORM_SOURCE}, + ); + } }); } From bd4440220f91a7cf43d29556ec048c3de0eff22a Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Fri, 24 Jan 2025 15:43:08 +0000 Subject: [PATCH 04/10] Add note with advice for auto-generated alt text --- packages/myst-transforms/src/images.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/myst-transforms/src/images.ts b/packages/myst-transforms/src/images.ts index 0715156b3..740da3920 100644 --- a/packages/myst-transforms/src/images.ts +++ b/packages/myst-transforms/src/images.ts @@ -46,7 +46,7 @@ export function imageNoAltTextTransform(tree: GenericParent, file: VFile) { fileWarn( file, `alt text for ${image.url} was auto-generated`, - {ruleId: RuleId.imageAltTextGenerated, node: image, source: TRANSFORM_SOURCE}, + {ruleId: RuleId.imageAltTextGenerated, node: image, source: TRANSFORM_SOURCE, note: 'You can remove this warning by writing your own alt-text'}, ); } }); From 04676882745ba03c6a0df69dc64375d2e4bb41bf Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Fri, 24 Jan 2025 15:48:26 +0000 Subject: [PATCH 05/10] Add changeset file --- .changeset/big-cooks-admire.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/big-cooks-admire.md diff --git a/.changeset/big-cooks-admire.md b/.changeset/big-cooks-admire.md new file mode 100644 index 000000000..6f9cacde0 --- /dev/null +++ b/.changeset/big-cooks-admire.md @@ -0,0 +1,6 @@ +--- +"myst-transforms": patch +"myst-common": patch +--- + +Add warnings for missing alt-text and auto-generated alt-text. From 24267e983d3d2ce0e5ecdba92163fdb87d49eeb8 Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Fri, 24 Jan 2025 15:57:21 +0000 Subject: [PATCH 06/10] Use alt text more consistently --- packages/myst-transforms/src/images.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/myst-transforms/src/images.ts b/packages/myst-transforms/src/images.ts index 740da3920..b9adcd650 100644 --- a/packages/myst-transforms/src/images.ts +++ b/packages/myst-transforms/src/images.ts @@ -46,7 +46,7 @@ export function imageNoAltTextTransform(tree: GenericParent, file: VFile) { fileWarn( file, `alt text for ${image.url} was auto-generated`, - {ruleId: RuleId.imageAltTextGenerated, node: image, source: TRANSFORM_SOURCE, note: 'You can remove this warning by writing your own alt-text'}, + {ruleId: RuleId.imageAltTextGenerated, node: image, source: TRANSFORM_SOURCE, note: 'You can remove this warning by writing your own altxtext'}, ); } }); From a6624c6e9a15474ef7399d3c4bfd7ef53c3cf08d Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Fri, 24 Jan 2025 16:07:18 +0000 Subject: [PATCH 07/10] Run prettier --- packages/myst-transforms/src/images.ts | 37 +++++++++++++------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/packages/myst-transforms/src/images.ts b/packages/myst-transforms/src/images.ts index b9adcd650..496a64986 100644 --- a/packages/myst-transforms/src/images.ts +++ b/packages/myst-transforms/src/images.ts @@ -1,6 +1,6 @@ import type { Plugin } from 'unified'; import type { Container, Paragraph, PhrasingContent, Image } from 'myst-spec'; -import type { VFile} from 'vfile'; +import type { VFile } from 'vfile'; import { select, selectAll } from 'unist-util-select'; import type { GenericParent } from 'myst-common'; import { fileWarn, toText, RuleId } from 'myst-common'; @@ -33,23 +33,24 @@ export function imageAltTextTransform(tree: GenericParent) { } export function imageNoAltTextTransform(tree: GenericParent, file: VFile) { - const imageNodes = selectAll("image", tree) as Image[]; - imageNodes.forEach((image, index) => { - if (image.alt == null) { - fileWarn( - file, - `missing alt text for ${image.url}`, - {ruleId: RuleId.imageHasAltText, node: image, source: TRANSFORM_SOURCE}, - ); - } - if (image.data?.altTextIsAutoGenerated) { - fileWarn( - file, - `alt text for ${image.url} was auto-generated`, - {ruleId: RuleId.imageAltTextGenerated, node: image, source: TRANSFORM_SOURCE, note: 'You can remove this warning by writing your own altxtext'}, - ); - } - }); + const imageNodes = selectAll('image', tree) as Image[]; + imageNodes.forEach((image, index) => { + if (image.alt == null) { + fileWarn(file, `missing alt text for ${image.url}`, { + ruleId: RuleId.imageHasAltText, + node: image, + source: TRANSFORM_SOURCE, + }); + } + if (image.data?.altTextIsAutoGenerated) { + fileWarn(file, `alt text for ${image.url} was auto-generated`, { + ruleId: RuleId.imageAltTextGenerated, + node: image, + source: TRANSFORM_SOURCE, + note: 'You can remove this warning by writing your own altxtext', + }); + } + }); } export const imageAltTextPlugin: Plugin<[], GenericParent, GenericParent> = () => (tree, file) => { From bd8691738af2e76c5d11f896cf3642564c549c4f Mon Sep 17 00:00:00 2001 From: Angus Hollands Date: Fri, 24 Jan 2025 23:09:00 +0000 Subject: [PATCH 08/10] Update packages/myst-transforms/src/images.ts Co-authored-by: Rowan Cockett --- packages/myst-transforms/src/images.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/myst-transforms/src/images.ts b/packages/myst-transforms/src/images.ts index 496a64986..24b64ec2c 100644 --- a/packages/myst-transforms/src/images.ts +++ b/packages/myst-transforms/src/images.ts @@ -47,7 +47,7 @@ export function imageNoAltTextTransform(tree: GenericParent, file: VFile) { ruleId: RuleId.imageAltTextGenerated, node: image, source: TRANSFORM_SOURCE, - note: 'You can remove this warning by writing your own altxtext', + note: 'You can remove this warning by writing your own alt text', }); } }); From 689a6ad9849b61b4624f7d50469e0197e6e0c903 Mon Sep 17 00:00:00 2001 From: Angus Hollands Date: Mon, 27 Jan 2025 10:37:34 +0000 Subject: [PATCH 09/10] test: add simple alt-text tests --- packages/myst-transforms/src/images.spec.ts | 127 ++++++++++++++++++++ packages/myst-transforms/src/images.ts | 41 ++++--- 2 files changed, 152 insertions(+), 16 deletions(-) create mode 100644 packages/myst-transforms/src/images.spec.ts diff --git a/packages/myst-transforms/src/images.spec.ts b/packages/myst-transforms/src/images.spec.ts new file mode 100644 index 000000000..2a87dab55 --- /dev/null +++ b/packages/myst-transforms/src/images.spec.ts @@ -0,0 +1,127 @@ +import { describe, expect, test } from 'vitest'; +import { VFile } from 'vfile'; + +import { imageNoAltTextTransform } from './images.js'; + +describe('Test imageNoAltTextTransform', () => { + test('image without alt text generates warning', () => { + const mdast = { + type: 'root', + children: [ + { + type: 'image', + url: 'https://images.com/cats', + align: 'center', + }, + ], + }; + const file = new VFile(); + imageNoAltTextTransform(mdast, file); + expect(mdast).toEqual({ + type: 'root', + children: [ + { + type: 'image', + url: 'https://images.com/cats', + align: 'center', + }, + ], + }); + // A warning was created + expect(file.messages.length).toBe(1); + expect(file.messages[0].message.includes('missing alt text')).toBe(true); + }); + test('image without alt text does not generate a warning', () => { + const mdast = { + type: 'root', + children: [ + { + type: 'image', + url: 'https://images.com/cats', + alt: 'I have alt text', + align: 'center', + }, + ], + }; + const file = new VFile(); + imageNoAltTextTransform(mdast, file); + expect(mdast).toEqual({ + type: 'root', + children: [ + { + type: 'image', + url: 'https://images.com/cats', + alt: 'I have alt text', + align: 'center', + }, + ], + }); + // A warning was created + expect(file.messages.length).toBe(0); + }); + test('image inside captioned figure warns about generated alt text', () => { + const mdast = { + type: 'container', + kind: 'figure', + children: [ + { + type: 'image', + url: 'https://images.com/cats', + alt: 'I don’t have alt text, but I do have a caption', + data: { + altTextIsAutoGenerated: true, + }, + }, + { + type: 'caption', + children: [ + { + type: 'paragraph', + children: [ + { + type: 'text', + value: 'I don’t have alt text, but I do have a caption', + }, + ], + }, + ], + }, + ], + enumerator: '1', + }; + const file = new VFile(); + imageNoAltTextTransform(mdast, file); + expect(mdast).toEqual({ + type: 'container', + kind: 'figure', + children: [ + { + type: 'image', + url: 'https://images.com/cats', + alt: 'I don’t have alt text, but I do have a caption', + data: { + altTextIsAutoGenerated: true, + }, + }, + { + type: 'caption', + children: [ + { + type: 'paragraph', + children: [ + { + type: 'text', + value: 'I don’t have alt text, but I do have a caption', + }, + ], + }, + ], + }, + ], + enumerator: '1', + }); + // A warning was created + expect(file.messages.length).toBe(1); + expect(file.messages[0].message.includes('was auto-generated')).toBe(true); + }); +}); diff --git a/packages/myst-transforms/src/images.ts b/packages/myst-transforms/src/images.ts index 24b64ec2c..47bed3a0b 100644 --- a/packages/myst-transforms/src/images.ts +++ b/packages/myst-transforms/src/images.ts @@ -2,6 +2,7 @@ import type { Plugin } from 'unified'; import type { Container, Paragraph, PhrasingContent, Image } from 'myst-spec'; import type { VFile } from 'vfile'; import { select, selectAll } from 'unist-util-select'; +import { visit, SKIP } from 'unist-util-visit'; import type { GenericParent } from 'myst-common'; import { fileWarn, toText, RuleId } from 'myst-common'; @@ -33,22 +34,30 @@ export function imageAltTextTransform(tree: GenericParent) { } export function imageNoAltTextTransform(tree: GenericParent, file: VFile) { - const imageNodes = selectAll('image', tree) as Image[]; - imageNodes.forEach((image, index) => { - if (image.alt == null) { - fileWarn(file, `missing alt text for ${image.url}`, { - ruleId: RuleId.imageHasAltText, - node: image, - source: TRANSFORM_SOURCE, - }); - } - if (image.data?.altTextIsAutoGenerated) { - fileWarn(file, `alt text for ${image.url} was auto-generated`, { - ruleId: RuleId.imageAltTextGenerated, - node: image, - source: TRANSFORM_SOURCE, - note: 'You can remove this warning by writing your own alt text', - }); + visit(tree, ['output', 'image'], (node) => { + switch (node.type) { + // Do not recurse into outputs, as they rarely have alt-texts and are usually embedded + // into a figure that does + case 'output': { + return SKIP; + } + case 'image': { + if (node.alt == null) { + fileWarn(file, `missing alt text for ${node.url}`, { + ruleId: RuleId.imageHasAltText, + node: node, + source: TRANSFORM_SOURCE, + }); + } + if (node.data?.altTextIsAutoGenerated) { + fileWarn(file, `alt text for ${node.url} was auto-generated`, { + ruleId: RuleId.imageAltTextGenerated, + node: node, + source: TRANSFORM_SOURCE, + note: 'You can remove this warning by writing your own alt text', + }); + } + } } }); } From be7097ffff5cc4d753fd1c63ac08b282b25addb5 Mon Sep 17 00:00:00 2001 From: Angus Hollands Date: Mon, 27 Jan 2025 10:38:43 +0000 Subject: [PATCH 10/10] test: ensure that outputs are not visted by alt-text visitor --- packages/myst-transforms/src/images.spec.ts | 36 +++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/packages/myst-transforms/src/images.spec.ts b/packages/myst-transforms/src/images.spec.ts index 2a87dab55..cce71f021 100644 --- a/packages/myst-transforms/src/images.spec.ts +++ b/packages/myst-transforms/src/images.spec.ts @@ -124,4 +124,40 @@ describe('Test imageNoAltTextTransform', () => { expect(file.messages.length).toBe(1); expect(file.messages[0].message.includes('was auto-generated')).toBe(true); }); + test('image inside output does not generate warning', () => { + const mdast = { + type: 'root', + children: [ + { + type: 'output', + children: [ + { + type: 'image', + url: 'https://images.com/cats', + align: 'center', + }, + ], + }, + ], + }; + const file = new VFile(); + imageNoAltTextTransform(mdast, file); + expect(mdast).toEqual({ + type: 'root', + children: [ + { + type: 'output', + children: [ + { + type: 'image', + url: 'https://images.com/cats', + align: 'center', + }, + ], + }, + ], + }); + // A warning was created + expect(file.messages.length).toBe(0); + }); });