Skip to content

Commit c77de10

Browse files
Detect track changes; fail yamlchecker if a document with track changes is in the templates directory
1 parent f909b28 commit c77de10

5 files changed

Lines changed: 254 additions & 2 deletions

File tree

README.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,14 @@ table header and merged-cell risks, explicitly low-contrast text, and
114114
floating objects or text boxes that disturb reading order. These run by
115115
default; use `--no-docx-accessibility` to skip them.
116116

117+
DOCX files are also checked for embedded comments and tracked-change markup.
118+
These are errors by default because drafting material can leak into published
119+
documents or change their output. Accept or reject all changes and remove all
120+
comments before committing a template. Use `--no-docx-review-markup` to disable
121+
only this rule while keeping the accessibility checks, or suppress `EG130` with
122+
the standard `--suppress` option. The existing `--no-docx-accessibility` master
123+
switch disables all DOCX checks, including this one.
124+
117125
**Every finding is capped at warning severity by default**, so turning these
118126
checks on reports problems without failing the build. Most existing
119127
templates have findings today, and the intent is for authors to work through

src/dayamlchecker/docx_accessibility.py

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,51 @@
7272
# a caption fragment) that having no heading styles is not a real finding.
7373
_HEADING_REQUIRED_TEXT_LENGTH = 1500
7474

75+
_WORDPROCESSINGML_NAMESPACES = {
76+
"http://schemas.openxmlformats.org/wordprocessingml/2006/main",
77+
"http://purl.oclc.org/ooxml/wordprocessingml/main",
78+
}
79+
_COMMENT_MARKUP_NAMES = {
80+
"comment",
81+
"commentRangeStart",
82+
"commentRangeEnd",
83+
"commentReference",
84+
}
85+
_TRACKED_CHANGE_NAMES = {
86+
"ins",
87+
"del",
88+
"delText",
89+
"delInstrText",
90+
"moveFrom",
91+
"moveTo",
92+
"moveFromRangeStart",
93+
"moveFromRangeEnd",
94+
"moveToRangeStart",
95+
"moveToRangeEnd",
96+
"customXmlInsRangeStart",
97+
"customXmlInsRangeEnd",
98+
"customXmlDelRangeStart",
99+
"customXmlDelRangeEnd",
100+
"customXmlMoveFromRangeStart",
101+
"customXmlMoveFromRangeEnd",
102+
"customXmlMoveToRangeStart",
103+
"customXmlMoveToRangeEnd",
104+
"conflictIns",
105+
"conflictDel",
106+
"numberingChange",
107+
"cellIns",
108+
"cellDel",
109+
"cellMerge",
110+
"pPrChange",
111+
"rPrChange",
112+
"sectPrChange",
113+
"tblPrChange",
114+
"tblPrExChange",
115+
"tblGridChange",
116+
"trPrChange",
117+
"tcPrChange",
118+
}
119+
75120
_SEVERITY_RANK = {Severity.INFO: 0, Severity.WARNING: 1, Severity.ERROR: 2}
76121

77122

@@ -161,6 +206,60 @@ def check_docx_accessibility(
161206
]
162207

163208

209+
def check_docx_review_markup(path: str | Path) -> list[Finding]:
210+
"""Return an error when a DOCX still contains comments or revisions.
211+
212+
This is intentionally independent from the accessibility severity ceiling:
213+
unresolved review markup can expose drafting material or produce unintended
214+
output, so it is a default-on release-safety check.
215+
"""
216+
path = Path(path)
217+
comment_parts: set[str] = set()
218+
revision_parts: set[str] = set()
219+
220+
try:
221+
with zipfile.ZipFile(path) as package:
222+
for part_name in package.namelist():
223+
if not part_name.startswith("word/") or not part_name.endswith(".xml"):
224+
continue
225+
try:
226+
root = ET.fromstring(package.read(part_name))
227+
except (ET.ParseError, KeyError):
228+
continue
229+
for element in root.iter():
230+
if _namespace(element.tag) not in _WORDPROCESSINGML_NAMESPACES:
231+
continue
232+
local_name = _local_name(element.tag)
233+
if local_name in _COMMENT_MARKUP_NAMES:
234+
comment_parts.add(part_name)
235+
if local_name in _TRACKED_CHANGE_NAMES:
236+
revision_parts.add(part_name)
237+
except (OSError, zipfile.BadZipFile):
238+
# The accessibility package check owns the existing unreadable-DOCX
239+
# diagnostic. Avoid reporting a misleading review-markup error too.
240+
return []
241+
242+
markup = []
243+
if comment_parts:
244+
markup.append("embedded comments")
245+
if revision_parts:
246+
markup.append("tracked changes")
247+
if not markup:
248+
return []
249+
250+
parts = sorted(comment_parts | revision_parts)
251+
return [
252+
Finding(
253+
message_id=MessageId.DOCX_REVIEW_MARKUP,
254+
file_name=str(path),
255+
context={
256+
"markup": " and ".join(markup),
257+
"parts": ", ".join(parts),
258+
},
259+
)
260+
]
261+
262+
164263
def collect_docx_files(
165264
paths: list[Path], include_default_ignores: bool = True
166265
) -> list[Path]:
@@ -1110,6 +1209,12 @@ def _local_name(tag: str) -> str:
11101209
return tag
11111210

11121211

1212+
def _namespace(tag: str) -> str:
1213+
if tag.startswith("{") and "}" in tag:
1214+
return tag[1:].split("}", 1)[0]
1215+
return ""
1216+
1217+
11131218
def _attr(element: Optional[ET.Element], local_name: str) -> str:
11141219
if element is None:
11151220
return ""

src/dayamlchecker/messages.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ class MessageId(StrEnum):
8080
MULTIPLE_MANDATORY_BLOCKS = "multiple_mandatory_blocks"
8181
MISSING_METADATA_FIELDS = "missing_metadata_fields"
8282
ATTACHMENT_CONDITIONAL_VARIABLE = "attachment_conditional_variable"
83+
DOCX_REVIEW_MARKUP = "docx_review_markup"
8384

8485
ACCESSIBILITY_COMBOBOX_NOT_ACCESSIBLE = "accessibility_combobox_not_accessible"
8586
ACCESSIBILITY_NO_LABEL_MULTI_FIELD = "accessibility_no_label_multi_field"
@@ -965,6 +966,16 @@ class MessageDefinition:
965966
"displayed content"
966967
),
967968
),
969+
MessageId.DOCX_REVIEW_MARKUP: MessageDefinition(
970+
code="EG130",
971+
severity=Severity.ERROR,
972+
finding_class=FindingClass.GENERAL,
973+
summary="DOCX contains review markup",
974+
template=(
975+
"the document contains {markup}; accept or reject tracked changes and "
976+
"remove comments before publishing (found in {parts})"
977+
),
978+
),
968979
# DOCX template accessibility
969980
MessageId.ACCESSIBILITY_DOCX_DOCX_UNREADABLE: MessageDefinition(
970981
code="EA540",

src/dayamlchecker/yaml_structure.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
from dayamlchecker.docx_accessibility import (
4747
DocxAccessibilityOptions,
4848
check_docx_accessibility,
49+
check_docx_review_markup,
4950
collect_docx_files,
5051
)
5152

@@ -2842,6 +2843,15 @@ def main(argv: Optional[list[str]] = None) -> int:
28422843
"error to fail on documents with accessibility errors"
28432844
),
28442845
)
2846+
parser.add_argument(
2847+
"--docx-review-markup",
2848+
action=argparse.BooleanOptionalAction,
2849+
default=True,
2850+
help=(
2851+
"Fail when DOCX templates contain comments or tracked changes "
2852+
"(default: on)"
2853+
),
2854+
)
28452855
parser.add_argument(
28462856
"--format",
28472857
choices=("text", "github"),
@@ -2907,6 +2917,8 @@ def main(argv: Optional[list[str]] = None) -> int:
29072917
docx_options = runtime_options.docx_accessibility_options()
29082918
for docx_file in docx_files:
29092919
all_findings.extend(check_docx_accessibility(docx_file, docx_options))
2920+
if args.docx_review_markup:
2921+
all_findings.extend(check_docx_review_markup(docx_file))
29102922

29112923
if args.url_check and yaml_files:
29122924
url_check_root = (

tests/test_docx_accessibility.py

Lines changed: 118 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from dayamlchecker.docx_accessibility import (
88
DocxAccessibilityOptions,
99
check_docx_accessibility,
10+
check_docx_review_markup,
1011
)
1112
from dayamlchecker.messages import Severity
1213
from dayamlchecker.yaml_structure import main
@@ -259,6 +260,71 @@ def test_unreadable_docx_reports_a_package_finding():
259260
assert strict[0].severity == ERROR
260261

261262

263+
# ---------------------------------------------------------------------------
264+
# Comments and tracked changes
265+
# ---------------------------------------------------------------------------
266+
267+
268+
def test_docx_comments_are_errors_by_default():
269+
with TemporaryDirectory() as tmp:
270+
path = _build(
271+
tmp,
272+
"commented",
273+
'<w:p><w:commentRangeStart w:id="0"/><w:r><w:t>Draft</w:t></w:r>'
274+
'<w:commentRangeEnd w:id="0"/><w:r><w:commentReference w:id="0"/>'
275+
"</w:p>",
276+
)
277+
with zipfile.ZipFile(path, "a") as package:
278+
package.writestr(
279+
"word/comments.xml",
280+
'<?xml version="1.0" encoding="UTF-8"?>'
281+
'<w:comments xmlns:w="http://schemas.openxmlformats.org/'
282+
'wordprocessingml/2006/main"><w:comment w:id="0">'
283+
"<w:p><w:r><w:t>Do not publish this note</w:t></w:r></w:p>"
284+
"</w:comment></w:comments>",
285+
)
286+
287+
findings = check_docx_review_markup(path)
288+
289+
assert len(findings) == 1
290+
assert findings[0].code == "EG130"
291+
assert findings[0].severity == ERROR
292+
assert findings[0].finding_class == "general"
293+
assert "embedded comments" in findings[0].message
294+
assert "word/comments.xml" in findings[0].message
295+
296+
297+
def test_docx_tracked_changes_are_errors_by_default():
298+
with TemporaryDirectory() as tmp:
299+
path = _build(
300+
tmp,
301+
"revisions",
302+
'<w:p><w:del w:id="1"><w:r><w:delText>Old</w:delText></w:r></w:del>'
303+
'<w:ins w:id="2"><w:r><w:t>New</w:t></w:r></w:ins></w:p>',
304+
)
305+
306+
findings = check_docx_review_markup(path)
307+
308+
assert len(findings) == 1
309+
assert findings[0].code == "EG130"
310+
assert "tracked changes" in findings[0].message
311+
assert "word/document.xml" in findings[0].message
312+
313+
314+
def test_track_changes_setting_without_revision_markup_is_allowed():
315+
with TemporaryDirectory() as tmp:
316+
path = _build(tmp, "tracking-on", f"<w:p><w:r><w:t>{PROSE}</w:t></w:r></w:p>")
317+
with zipfile.ZipFile(path, "a") as package:
318+
package.writestr(
319+
"word/settings.xml",
320+
'<?xml version="1.0" encoding="UTF-8"?>'
321+
'<w:settings xmlns:w="http://schemas.openxmlformats.org/'
322+
'wordprocessingml/2006/main"><w:trackRevisions/></w:settings>',
323+
)
324+
325+
assert check_docx_review_markup(path) == []
326+
327+
262328
# ---------------------------------------------------------------------------
263329
# Regressions
264330
# ---------------------------------------------------------------------------
@@ -553,7 +619,7 @@ def test_cli_checks_docx_by_default_without_failing():
553619
assert "WA541" in output, "image-alt-missing, demoted to a warning"
554620

555621

556-
def test_cli_can_skip_docx_checks():
622+
def test_cli_can_skip_all_docx_checks():
557623
with TemporaryDirectory() as tmp:
558624
path = Path(tmp) / "inaccessible.docx"
559625
_write_docx(path, _base_files(_inaccessible_document_xml()))
@@ -562,7 +628,57 @@ def test_cli_can_skip_docx_checks():
562628
["--no-docx-accessibility", "--no-url-check", str(path)]
563629
)
564630

565-
assert exit_code == 1, "nothing left to check"
631+
assert exit_code == 1, "nothing is enabled to check the DOCX"
632+
633+
634+
def test_cli_review_markup_fails_independently_of_accessibility_severity():
635+
with TemporaryDirectory() as tmp:
636+
path = _build(
637+
tmp,
638+
"revisions",
639+
'<w:p><w:ins w:id="2"><w:r><w:t>New</w:t></w:r></w:ins></w:p>',
640+
)
641+
642+
exit_code, output = _run_cli(["--no-url-check", str(path)])
643+
644+
assert exit_code == 1
645+
assert "EG130" in output
646+
647+
648+
def test_cli_can_disable_docx_review_markup_rule():
649+
with TemporaryDirectory() as tmp:
650+
path = _build(
651+
tmp,
652+
"revisions",
653+
'<w:p><w:ins w:id="2"><w:r><w:t>New</w:t></w:r></w:ins></w:p>',
654+
)
655+
656+
exit_code, output = _run_cli(
657+
[
658+
"--no-docx-review-markup",
659+
"--no-url-check",
660+
str(path),
661+
]
662+
)
663+
664+
assert exit_code == 0, "warning-level accessibility findings do not fail"
665+
assert "EG130" not in output
666+
667+
668+
def test_cli_can_suppress_docx_review_markup_by_code():
669+
with TemporaryDirectory() as tmp:
670+
path = _build(
671+
tmp,
672+
"revisions",
673+
'<w:p><w:ins w:id="2"><w:r><w:t>New</w:t></w:r></w:ins></w:p>',
674+
)
675+
676+
exit_code, output = _run_cli(
677+
["--suppress", "EG130", "--no-url-check", str(path)]
678+
)
679+
680+
assert exit_code == 0
681+
assert "EG130" not in output
566682

567683

568684
def test_cli_error_severity_fails_the_command():

0 commit comments

Comments
 (0)