Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,14 @@ table header and merged-cell risks, explicitly low-contrast text, and
floating objects or text boxes that disturb reading order. These run by
default; use `--no-docx-accessibility` to skip them.

DOCX files are also checked for embedded comments and tracked-change markup.
These are errors by default because drafting material can leak into published
documents or change their output. Accept or reject all changes and remove all
comments before committing a template. Use `--no-docx-review-markup` to disable
only this rule while keeping the accessibility checks, or suppress `EG130` with
the standard `--suppress` option. The existing `--no-docx-accessibility` master
switch disables all DOCX checks, including this one.

**Every finding is capped at warning severity by default**, so turning these
checks on reports problems without failing the build. Most existing
templates have findings today, and the intent is for authors to work through
Expand Down
105 changes: 105 additions & 0 deletions src/dayamlchecker/docx_accessibility.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,51 @@
# a caption fragment) that having no heading styles is not a real finding.
_HEADING_REQUIRED_TEXT_LENGTH = 1500

_WORDPROCESSINGML_NAMESPACES = {
"http://schemas.openxmlformats.org/wordprocessingml/2006/main",
"http://purl.oclc.org/ooxml/wordprocessingml/main",
}
_COMMENT_MARKUP_NAMES = {
"comment",
"commentRangeStart",
"commentRangeEnd",
"commentReference",
}
_TRACKED_CHANGE_NAMES = {
"ins",
"del",
"delText",
"delInstrText",
"moveFrom",
"moveTo",
"moveFromRangeStart",
"moveFromRangeEnd",
"moveToRangeStart",
"moveToRangeEnd",
"customXmlInsRangeStart",
"customXmlInsRangeEnd",
"customXmlDelRangeStart",
"customXmlDelRangeEnd",
"customXmlMoveFromRangeStart",
"customXmlMoveFromRangeEnd",
"customXmlMoveToRangeStart",
"customXmlMoveToRangeEnd",
"conflictIns",
"conflictDel",
"numberingChange",
"cellIns",
"cellDel",
"cellMerge",
"pPrChange",
"rPrChange",
"sectPrChange",
"tblPrChange",
"tblPrExChange",
"tblGridChange",
"trPrChange",
"tcPrChange",
}

_SEVERITY_RANK = {Severity.INFO: 0, Severity.WARNING: 1, Severity.ERROR: 2}


Expand Down Expand Up @@ -161,6 +206,60 @@ def check_docx_accessibility(
]


def check_docx_review_markup(path: str | Path) -> list[Finding]:
"""Return an error when a DOCX still contains comments or revisions.

This is intentionally independent from the accessibility severity ceiling:
unresolved review markup can expose drafting material or produce unintended
output, so it is a default-on release-safety check.
"""
path = Path(path)
comment_parts: set[str] = set()
revision_parts: set[str] = set()

try:
with zipfile.ZipFile(path) as package:
for part_name in package.namelist():
if not part_name.startswith("word/") or not part_name.endswith(".xml"):
continue
try:
root = ET.fromstring(package.read(part_name))
except (ET.ParseError, KeyError):
continue
for element in root.iter():
if _namespace(element.tag) not in _WORDPROCESSINGML_NAMESPACES:
continue
local_name = _local_name(element.tag)
if local_name in _COMMENT_MARKUP_NAMES:
comment_parts.add(part_name)
if local_name in _TRACKED_CHANGE_NAMES:
revision_parts.add(part_name)
except (OSError, zipfile.BadZipFile):
# The accessibility package check owns the existing unreadable-DOCX
# diagnostic. Avoid reporting a misleading review-markup error too.
return []

markup = []
if comment_parts:
markup.append("embedded comments")
if revision_parts:
markup.append("tracked changes")
if not markup:
return []

parts = sorted(comment_parts | revision_parts)
return [
Finding(
message_id=MessageId.DOCX_REVIEW_MARKUP,
file_name=str(path),
context={
"markup": " and ".join(markup),
"parts": ", ".join(parts),
},
)
]


def collect_docx_files(
paths: list[Path], include_default_ignores: bool = True
) -> list[Path]:
Expand Down Expand Up @@ -1110,6 +1209,12 @@ def _local_name(tag: str) -> str:
return tag


def _namespace(tag: str) -> str:
if tag.startswith("{") and "}" in tag:
return tag[1:].split("}", 1)[0]
return ""


def _attr(element: Optional[ET.Element], local_name: str) -> str:
if element is None:
return ""
Expand Down
11 changes: 11 additions & 0 deletions src/dayamlchecker/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ class MessageId(StrEnum):
MULTIPLE_MANDATORY_BLOCKS = "multiple_mandatory_blocks"
MISSING_METADATA_FIELDS = "missing_metadata_fields"
ATTACHMENT_CONDITIONAL_VARIABLE = "attachment_conditional_variable"
DOCX_REVIEW_MARKUP = "docx_review_markup"

ACCESSIBILITY_COMBOBOX_NOT_ACCESSIBLE = "accessibility_combobox_not_accessible"
ACCESSIBILITY_NO_LABEL_MULTI_FIELD = "accessibility_no_label_multi_field"
Expand Down Expand Up @@ -965,6 +966,16 @@ class MessageDefinition:
"displayed content"
),
),
MessageId.DOCX_REVIEW_MARKUP: MessageDefinition(
code="EG130",
severity=Severity.ERROR,
finding_class=FindingClass.GENERAL,
summary="DOCX contains review markup",
template=(
"the document contains {markup}; accept or reject tracked changes and "
"remove comments before publishing (found in {parts})"
),
),
# DOCX template accessibility
MessageId.ACCESSIBILITY_DOCX_DOCX_UNREADABLE: MessageDefinition(
code="EA540",
Expand Down
12 changes: 12 additions & 0 deletions src/dayamlchecker/yaml_structure.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
from dayamlchecker.docx_accessibility import (
DocxAccessibilityOptions,
check_docx_accessibility,
check_docx_review_markup,
collect_docx_files,
)

Expand Down Expand Up @@ -2842,6 +2843,15 @@ def main(argv: Optional[list[str]] = None) -> int:
"error to fail on documents with accessibility errors"
),
)
parser.add_argument(
"--docx-review-markup",
action=argparse.BooleanOptionalAction,
default=True,
help=(
"Fail when DOCX templates contain comments or tracked changes "
"(default: on)"
),
)
parser.add_argument(
"--format",
choices=("text", "github"),
Expand Down Expand Up @@ -2907,6 +2917,8 @@ def main(argv: Optional[list[str]] = None) -> int:
docx_options = runtime_options.docx_accessibility_options()
for docx_file in docx_files:
all_findings.extend(check_docx_accessibility(docx_file, docx_options))
if args.docx_review_markup:
all_findings.extend(check_docx_review_markup(docx_file))

if args.url_check and yaml_files:
url_check_root = (
Expand Down
120 changes: 118 additions & 2 deletions tests/test_docx_accessibility.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from dayamlchecker.docx_accessibility import (
DocxAccessibilityOptions,
check_docx_accessibility,
check_docx_review_markup,
)
from dayamlchecker.messages import Severity
from dayamlchecker.yaml_structure import main
Expand Down Expand Up @@ -259,6 +260,71 @@ def test_unreadable_docx_reports_a_package_finding():
assert strict[0].severity == ERROR


# ---------------------------------------------------------------------------
# Comments and tracked changes
# ---------------------------------------------------------------------------


def test_docx_comments_are_errors_by_default():
with TemporaryDirectory() as tmp:
path = _build(
tmp,
"commented",
'<w:p><w:commentRangeStart w:id="0"/><w:r><w:t>Draft</w:t></w:r>'
'<w:commentRangeEnd w:id="0"/><w:r><w:commentReference w:id="0"/>'
"</w:p>",
)
with zipfile.ZipFile(path, "a") as package:
package.writestr(
"word/comments.xml",
'<?xml version="1.0" encoding="UTF-8"?>'
'<w:comments xmlns:w="http://schemas.openxmlformats.org/'
'wordprocessingml/2006/main"><w:comment w:id="0">'
"<w:p><w:r><w:t>Do not publish this note</w:t></w:r></w:p>"
"</w:comment></w:comments>",
)

findings = check_docx_review_markup(path)

assert len(findings) == 1
assert findings[0].code == "EG130"
assert findings[0].severity == ERROR
assert findings[0].finding_class == "general"
assert "embedded comments" in findings[0].message
assert "word/comments.xml" in findings[0].message


def test_docx_tracked_changes_are_errors_by_default():
with TemporaryDirectory() as tmp:
path = _build(
tmp,
"revisions",
'<w:p><w:del w:id="1"><w:r><w:delText>Old</w:delText></w:r></w:del>'
'<w:ins w:id="2"><w:r><w:t>New</w:t></w:r></w:ins></w:p>',
)

findings = check_docx_review_markup(path)

assert len(findings) == 1
assert findings[0].code == "EG130"
assert "tracked changes" in findings[0].message
assert "word/document.xml" in findings[0].message


def test_track_changes_setting_without_revision_markup_is_allowed():
with TemporaryDirectory() as tmp:
path = _build(tmp, "tracking-on", f"<w:p><w:r><w:t>{PROSE}</w:t></w:r></w:p>")
with zipfile.ZipFile(path, "a") as package:
package.writestr(
"word/settings.xml",
'<?xml version="1.0" encoding="UTF-8"?>'
'<w:settings xmlns:w="http://schemas.openxmlformats.org/'
'wordprocessingml/2006/main"><w:trackRevisions/></w:settings>',
)

assert check_docx_review_markup(path) == []


# ---------------------------------------------------------------------------
# Regressions
# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -553,7 +619,7 @@ def test_cli_checks_docx_by_default_without_failing():
assert "WA541" in output, "image-alt-missing, demoted to a warning"


def test_cli_can_skip_docx_checks():
def test_cli_can_skip_all_docx_checks():
with TemporaryDirectory() as tmp:
path = Path(tmp) / "inaccessible.docx"
_write_docx(path, _base_files(_inaccessible_document_xml()))
Expand All @@ -562,7 +628,57 @@ def test_cli_can_skip_docx_checks():
["--no-docx-accessibility", "--no-url-check", str(path)]
)

assert exit_code == 1, "nothing left to check"
assert exit_code == 1, "nothing is enabled to check the DOCX"


def test_cli_review_markup_fails_independently_of_accessibility_severity():
with TemporaryDirectory() as tmp:
path = _build(
tmp,
"revisions",
'<w:p><w:ins w:id="2"><w:r><w:t>New</w:t></w:r></w:ins></w:p>',
)

exit_code, output = _run_cli(["--no-url-check", str(path)])

assert exit_code == 1
assert "EG130" in output


def test_cli_can_disable_docx_review_markup_rule():
with TemporaryDirectory() as tmp:
path = _build(
tmp,
"revisions",
'<w:p><w:ins w:id="2"><w:r><w:t>New</w:t></w:r></w:ins></w:p>',
)

exit_code, output = _run_cli(
[
"--no-docx-review-markup",
"--no-url-check",
str(path),
]
)

assert exit_code == 0, "warning-level accessibility findings do not fail"
assert "EG130" not in output


def test_cli_can_suppress_docx_review_markup_by_code():
with TemporaryDirectory() as tmp:
path = _build(
tmp,
"revisions",
'<w:p><w:ins w:id="2"><w:r><w:t>New</w:t></w:r></w:ins></w:p>',
)

exit_code, output = _run_cli(
["--suppress", "EG130", "--no-url-check", str(path)]
)

assert exit_code == 0
assert "EG130" not in output


def test_cli_error_severity_fails_the_command():
Expand Down
Loading