From d2105dcc35498e07b533701f5d64d177e1757af3 Mon Sep 17 00:00:00 2001 From: Tobias Schmitz Date: Fri, 18 Jul 2025 15:39:19 +0200 Subject: [PATCH] feat: report spans for missing alt text and unknown/duplicate tag ids --- Cargo.lock | 4 +- .../src/foundations/content/mod.rs | 20 +++++-- crates/typst-pdf/src/convert.rs | 57 ++++++++++++++----- crates/typst-pdf/src/link.rs | 3 + crates/typst-pdf/src/tags/mod.rs | 13 +++-- crates/typst-pdf/src/tags/table.rs | 17 ++++-- 6 files changed, 85 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0301b4324..5d22e6dcc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1373,7 +1373,7 @@ dependencies = [ [[package]] name = "krilla" version = "0.4.0" -source = "git+https://github.com/LaurenzV/krilla?branch=main#d40f81a01ca8f8654510a76effeef12518437800" +source = "git+https://github.com/LaurenzV/krilla?branch=main#32d070e737cd8ae4c3aa4ff901d15cb22bd052f3" dependencies = [ "base64", "bumpalo", @@ -1402,7 +1402,7 @@ dependencies = [ [[package]] name = "krilla-svg" version = "0.1.0" -source = "git+https://github.com/LaurenzV/krilla?branch=main#d40f81a01ca8f8654510a76effeef12518437800" +source = "git+https://github.com/LaurenzV/krilla?branch=main#32d070e737cd8ae4c3aa4ff901d15cb22bd052f3" dependencies = [ "flate2", "fontdb", diff --git a/crates/typst-library/src/foundations/content/mod.rs b/crates/typst-library/src/foundations/content/mod.rs index 84ab0c00a..1fa54b8f3 100644 --- a/crates/typst-library/src/foundations/content/mod.rs +++ b/crates/typst-library/src/foundations/content/mod.rs @@ -23,10 +23,10 @@ use serde::{Serialize, Serializer}; use typst_syntax::Span; use typst_utils::singleton; -use crate::diag::{SourceResult, StrResult}; +use crate::diag::{bail, SourceResult, StrResult}; use crate::engine::Engine; use crate::foundations::{ - func, repr, scope, ty, Context, Dict, IntoValue, Label, Property, Recipe, + func, repr, scope, ty, Args, Context, Dict, IntoValue, Label, Property, Recipe, RecipeIndex, Repr, Selector, Str, Style, StyleChain, Styles, Value, }; use crate::introspection::{Locatable, Location}; @@ -479,7 +479,7 @@ impl Content { /// Link the content somewhere. pub fn linked(self, dest: Destination, alt: Option) -> Self { let span = self.span(); - LinkMarker::new(self, dest.clone(), alt) + LinkMarker::new(self, dest.clone(), alt, span) .pack() .spanned(span) .set(LinkElem::current, Some(dest)) @@ -785,15 +785,27 @@ impl Repr for StyledElem { } /// An element that associates the body of a link with the destination. -#[elem(Locatable)] +#[elem(Locatable, Construct)] pub struct LinkMarker { /// The content. + #[internal] #[required] pub body: Content, + #[internal] #[required] pub dest: Destination, + #[internal] #[required] pub alt: Option, + #[internal] + #[required] + pub span: Span, +} + +impl Construct for LinkMarker { + fn construct(_: &mut Engine, args: &mut Args) -> SourceResult { + bail!(args.span, "cannot be constructed manually"); + } } impl IntoValue for T { diff --git a/crates/typst-pdf/src/convert.rs b/crates/typst-pdf/src/convert.rs index d826ae668..b33406816 100644 --- a/crates/typst-pdf/src/convert.rs +++ b/crates/typst-pdf/src/convert.rs @@ -9,6 +9,7 @@ use krilla::error::KrillaError; use krilla::geom::PathBuilder; use krilla::page::{PageLabel, PageSettings}; use krilla::surface::Surface; +use krilla::tagging::TagId; use krilla::{Document, SerializeSettings}; use krilla_svg::render_svg_glyph; use typst_library::diag::{bail, error, SourceDiagnostic, SourceResult}; @@ -373,11 +374,21 @@ fn finish( .collect::>(); Err(errors) } - KrillaError::DuplicateTagId(_, _) => { - unreachable!("duplicate IDs shouldn't be generated") + KrillaError::DuplicateTagId(id, loc) => { + let span = to_span(loc); + let id = display_tag_id(&id); + bail!( + span, "duplicate tag id `{id}`"; + hint: "please report this as a bug" + ) } - KrillaError::UnknownTagId(_, _) => { - unreachable!("all referenced IDs should be present in the tag tree") + KrillaError::UnknownTagId(id, loc) => { + let span = to_span(loc); + let id = display_tag_id(&id); + bail!( + span, "unknown tag id `{id}`"; + hint: "please report this as a bug" + ) } KrillaError::Image(_, loc) => { let span = to_span(loc); @@ -394,6 +405,20 @@ fn finish( } } +fn display_tag_id(id: &TagId) -> impl std::fmt::Display + use<'_> { + typst_utils::display(|f| { + if let Ok(str) = std::str::from_utf8(id.as_bytes()) { + f.write_str(str) + } else { + f.write_str("0x")?; + for b in id.as_bytes() { + write!(f, "{b:x}")?; + } + Ok(()) + } + }) +} + /// Converts a krilla error into a Typst error. fn convert_error( gc: &GlobalContext, @@ -562,16 +587,20 @@ fn convert_error( } // The below errors cannot occur yet, only once Typst supports full PDF/A // and PDF/UA. But let's still add a message just to be on the safe side. - ValidationError::MissingAnnotationAltText => error!( - Span::detached(), - "{prefix} missing annotation alt text"; - hint: "please report this as a bug" - ), - ValidationError::MissingAltText => error!( - Span::detached(), - "{prefix} missing alt text"; - hint: "make sure your images and equations have alt text" - ), + ValidationError::MissingAnnotationAltText(loc) => { + let span = to_span(*loc); + error!( + span, "{prefix} missing annotation alt text"; + hint: "please report this as a bug" + ) + } + ValidationError::MissingAltText(loc) => { + let span = to_span(*loc); + error!( + span, "{prefix} missing alt text"; + hint: "make sure your images and equations have alt text" + ) + } ValidationError::NoDocumentLanguage => error!( Span::detached(), "{prefix} missing document language"; diff --git a/crates/typst-pdf/src/link.rs b/crates/typst-pdf/src/link.rs index 7ce630516..df1e926de 100644 --- a/crates/typst-pdf/src/link.rs +++ b/crates/typst-pdf/src/link.rs @@ -6,6 +6,7 @@ use krilla::destination::XyzDestination; use krilla::geom as kg; use typst_library::layout::{Point, Position, Size}; use typst_library::model::Destination; +use typst_syntax::Span; use crate::convert::{FrameContext, GlobalContext}; use crate::tags::{self, Placeholder, TagNode}; @@ -17,6 +18,7 @@ pub(crate) struct LinkAnnotation { pub(crate) alt: Option, pub(crate) quad_points: Vec, pub(crate) target: Target, + pub(crate) span: Span, } pub(crate) fn handle_link( @@ -70,6 +72,7 @@ pub(crate) fn handle_link( quad_points: vec![quad], alt, target, + span: link.span, }); } } diff --git a/crates/typst-pdf/src/tags/mod.rs b/crates/typst-pdf/src/tags/mod.rs index 858844a57..ef7b294d6 100644 --- a/crates/typst-pdf/src/tags/mod.rs +++ b/crates/typst-pdf/src/tags/mod.rs @@ -176,6 +176,7 @@ pub(crate) fn handle_start( return Ok(()); }; + let tag = tag.with_location(Some(elem.span().into_raw().get())); push_stack(gc, loc, StackEntryKind::Standard(tag))?; Ok(()) @@ -202,7 +203,8 @@ pub(crate) fn handle_end(gc: &mut GlobalContext, surface: &mut Surface, loc: Loc // PDF/UA compliance of the structure hierarchy is checked // elsewhere. While this doesn't make a lot of sense, just // avoid crashing here. - let tag = TagKind::TOCI.into(); + let tag = TagKind::TOCI + .with_location(Some(outline_entry.span().into_raw().get())); gc.tags.push(TagNode::Group(tag, entry.nodes)); return; }; @@ -216,7 +218,8 @@ pub(crate) fn handle_end(gc: &mut GlobalContext, surface: &mut Surface, loc: Loc // PDF/UA compliance of the structure hierarchy is checked // elsewhere. While this doesn't make a lot of sense, just // avoid crashing here. - let tag = TagKind::TD(TableDataCell::new()).into(); + let tag = TagKind::TD(TableDataCell::new()) + .with_location(Some(cell.span().into_raw().get())); gc.tags.push(TagNode::Group(tag, entry.nodes)); return; }; @@ -324,11 +327,13 @@ pub(crate) fn add_annotations( annotations: Vec, ) { for annotation in annotations.into_iter() { - let LinkAnnotation { id: _, placeholder, alt, quad_points, target } = annotation; + let LinkAnnotation { id: _, placeholder, alt, quad_points, target, span } = + annotation; let annot = krilla::annotation::Annotation::new_link( krilla::annotation::LinkAnnotation::new_with_quad_points(quad_points, target), alt, - ); + ) + .with_location(Some(span.into_raw().get())); let annot_id = page.add_tagged_annotation(annot); gc.tags.placeholders.init(placeholder, Node::Leaf(annot_id)); } diff --git a/crates/typst-pdf/src/tags/table.rs b/crates/typst-pdf/src/tags/table.rs index 969440f2f..75128863f 100644 --- a/crates/typst-pdf/src/tags/table.rs +++ b/crates/typst-pdf/src/tags/table.rs @@ -9,6 +9,7 @@ use smallvec::SmallVec; use typst_library::foundations::{Packed, Smart, StyleChain}; use typst_library::model::TableCell; use typst_library::pdf::{TableCellKind, TableHeaderScope}; +use typst_syntax::Span; use crate::tags::{TableId, TagNode}; @@ -57,7 +58,7 @@ impl TableCtx { } } - pub(crate) fn insert(&mut self, cell: &TableCell, nodes: Vec) { + pub(crate) fn insert(&mut self, cell: &Packed, nodes: Vec) { let x = cell.x.get(StyleChain::default()).unwrap_or_else(|| unreachable!()); let y = cell.y.get(StyleChain::default()).unwrap_or_else(|| unreachable!()); let rowspan = cell.rowspan.get(StyleChain::default()); @@ -92,6 +93,7 @@ impl TableCtx { kind, headers: SmallVec::new(), nodes, + span: cell.span(), }); } @@ -175,13 +177,14 @@ impl TableCtx { .with_headers(cell.headers), ) .with_id(Some(id)) + .with_location(Some(cell.span.into_raw().get())) } TableCellKind::Footer | TableCellKind::Data => TagKind::TD( TableDataCell::new() .with_span(span) .with_headers(cell.headers), ) - .into(), + .with_location(Some(cell.span.into_raw().get())), }; Some(TagNode::Group(tag, cell.nodes)) }) @@ -296,6 +299,7 @@ struct TableCtxCell { kind: Smart, headers: SmallVec<[TagId; 1]>, nodes: Vec, + span: Span, } impl TableCtxCell { @@ -344,7 +348,7 @@ mod tests { fn table(cells: [TableCell; SIZE]) -> TableCtx { let mut table = TableCtx::new(TableId(324), Some("summary".into())); for cell in cells { - table.insert(&cell, Vec::new()); + table.insert(&Packed::new(cell), Vec::new()); } table } @@ -416,7 +420,9 @@ mod tests { let id = table_cell_id(TableId(324), x, y); let ids = headers.map(|(x, y)| table_cell_id(TableId(324), x, y)); TagNode::Group( - TagKind::TH(TableHeaderCell::new(scope).with_headers(ids)).with_id(Some(id)), + TagKind::TH(TableHeaderCell::new(scope).with_headers(ids)) + .with_id(Some(id)) + .with_location(Some(Span::detached().into_raw().get())), Vec::new(), ) } @@ -424,7 +430,8 @@ mod tests { fn td(headers: [(u32, u32); SIZE]) -> TagNode { let ids = headers.map(|(x, y)| table_cell_id(TableId(324), x, y)); TagNode::Group( - TagKind::TD(TableDataCell::new().with_headers(ids)).into(), + TagKind::TD(TableDataCell::new().with_headers(ids)) + .with_location(Some(Span::detached().into_raw().get())), Vec::new(), ) }