fix(ui): a rule skipped by hand stays with its document (#1301) (#1445)

Rules skipped in the accessibility results were kept in one set for
the whole app, and every check applied it. With the options dialog
turned off, skipping Tagged PDF in one document made the next
document's check report it as Skipped, its findings cleared, in the
panel and the saved report.

The set is now keyed by document: Skip Rule records the rule for the
document whose report is shown, and a check applies only that
document's skips. Start Checking in the options dialog still starts
over, for the document it checks only.

Adds a_rule_skipped_in_one_document_is_not_skipped_in_another in
tests/accessibility.rs, which fails before this change.
This commit is contained in:
Marco Arnulfo authored and GitHub committed 2026-10-13 00:36:15 -04:00
1 parent 082768ac29
commit cced834190
4 files changed
+66 -9

No files matched your search

+7 -7
View File
@@ -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) => {
+3 -1
View File
@@ -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();
}
}
+2 -1
View File
@@ -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<pdfcraft_engine::a11y::Rule>,
/// 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,
+54
View File
@@ -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| {