diff --git a/crates/ui-egui/src/a11y_ui.rs b/crates/ui-egui/src/a11y_ui.rs index 0fc8e2b2..70d14d0d 100644 --- a/crates/ui-egui/src/a11y_ui.rs +++ b/crates/ui-egui/src/a11y_ui.rs @@ -364,7 +364,7 @@ impl PdfCraftApp { }; // Rules skipped by hand stay skipped when checking again. let mut report = report; - for r in report.results.iter_mut().filter(|r| self.a11y_skipped.contains(&r.rule)) { + for r in report.results.iter_mut().filter(|r| self.a11y_skipped.contains(&(id, r.rule))) { r.status = Status::Skipped; r.findings.clear(); } @@ -448,12 +448,12 @@ impl PdfCraftApp { } } PanelAction::Skip(rule) => { - self.a11y_skipped.insert(rule); - if let Some((_, r)) = self.a11y.report.as_mut() - && let Some(x) = r.results.iter_mut().find(|x| x.rule == rule) - { - x.status = Status::Skipped; - x.findings.clear(); + if let Some((id, r)) = self.a11y.report.as_mut() { + self.a11y_skipped.insert((*id, rule)); + if let Some(x) = r.results.iter_mut().find(|x| x.rule == rule) { + x.status = Status::Skipped; + x.findings.clear(); + } } } PanelAction::Fix(rule) => { diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 9de94341..308a88c1 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1333,7 +1333,9 @@ fn dialogs(app: &mut PdfCraftApp, ctx: &egui::Context) { app.run_compare(); } if a11y_now { - app.a11y_skipped.clear(); + if let Some((_, id)) = app.active_ids() { + app.a11y_skipped.retain(|(doc, _)| *doc != id); + } app.run_accessibility_check(); } } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index ff84c3ee..fe861eda 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -647,7 +647,8 @@ pub struct PdfCraftApp { pub doc_js: js_ui::DocJsDraft, pub a11y: a11y_ui::A11yState, pub(crate) tags_state: tags_ui::State, - pub a11y_skipped: std::collections::BTreeSet, + /// Rules skipped by hand, for the document they were skipped in. + pub a11y_skipped: std::collections::HashSet<(DocId, pdfcraft_engine::a11y::Rule)>, pub alt_draft: a11y_ui::AltDraft, /// List the OS key store's signing identities among the digital IDs (the desktop app). pub os_key_store_ids: bool, diff --git a/crates/ui-egui/tests/accessibility.rs b/crates/ui-egui/tests/accessibility.rs index cc235c9f..b050dd07 100644 --- a/crates/ui-egui/tests/accessibility.rs +++ b/crates/ui-egui/tests/accessibility.rs @@ -92,6 +92,60 @@ fn checking_fixing_skipping_and_reporting() { assert!(html.contains("Accessibility Report") && html.contains("Skipped")); } +/// #1301: a rule skipped by hand is skipped for that document only. Another document checked +/// without the options dialog reports its own result, and the first keeps its skip, even after +/// the other is checked again through the dialog. +#[test] +fn a_rule_skipped_in_one_document_is_not_skipped_in_another() { + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(move |_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("a.pdf", None, FIXTURE.to_vec()).unwrap(); + app.open_bytes("b.pdf", None, FIXTURE.to_vec()).unwrap(); + app.active = Some(0); + app.a11y_options.show_dialog = false; + app + }); + h.run_steps(4); + let tagged = |h: &Harness<'static, PdfCraftApp>| { + let (id, report) = h.state().a11y.report.as_ref().unwrap(); + assert_eq!(Some(*id), h.state().active_ids().map(|(_, id)| id), "the report is the active document's"); + let r = report.result(Rule::TaggedPdf).unwrap(); + (r.status, r.findings.len()) + }; + assert!(h.state_mut().execute("a11y.check")); + h.run_steps(3); + assert_eq!(h.state().dialog, None); + right_click(&mut h, "Tagged PDF - Failed"); + h.get_by_label("Skip Rule").click(); + h.run_steps(2); + assert_eq!(tagged(&h), (Status::Skipped, 0)); + + h.state_mut().active = Some(1); + h.run_steps(2); + assert!(h.state_mut().execute("a11y.check")); + h.run_steps(3); + assert_eq!(h.state().dialog, None); + assert_eq!(tagged(&h), (Status::Failed, 2), "b.pdf's own result"); + // Starting again from the options dialog forgets b.pdf's skips, not a.pdf's. + right_click(&mut h, "Tagged PDF - Failed"); + h.get_by_label("Skip Rule").click(); + h.run_steps(2); + assert_eq!(tagged(&h), (Status::Skipped, 0)); + h.state_mut().a11y_options.show_dialog = true; + assert!(h.state_mut().execute("a11y.check")); + h.run_steps(2); + h.get_by_label("Start Checking").click(); + h.run_steps(3); + assert_eq!(tagged(&h), (Status::Failed, 2)); + + h.state_mut().active = Some(0); + h.run_steps(2); + h.state_mut().run_accessibility_check(); + h.run_steps(2); + assert_eq!(tagged(&h), (Status::Skipped, 0), "a.pdf keeps its skip"); +} + #[test] fn checker_summary_does_not_hide_manual_review() { let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(move |_cc| {