fix: address self-review findings
This commit is contained in:
+42
-6
@@ -92,6 +92,7 @@ class AccessibleSvgParser(HTMLParser):
|
|||||||
self.svg_stack = []
|
self.svg_stack = []
|
||||||
self.svgs = []
|
self.svgs = []
|
||||||
self.open_text = []
|
self.open_text = []
|
||||||
|
self.id_locations = {}
|
||||||
|
|
||||||
def source_offset(self):
|
def source_offset(self):
|
||||||
line, column = self.getpos()
|
line, column = self.getpos()
|
||||||
@@ -102,13 +103,15 @@ class AccessibleSvgParser(HTMLParser):
|
|||||||
attr_map = {name.casefold(): value for name, value in attrs}
|
attr_map = {name.casefold(): value for name, value in attrs}
|
||||||
offset = self.source_offset()
|
offset = self.source_offset()
|
||||||
line, _column = self.getpos()
|
line, _column = self.getpos()
|
||||||
|
element_id = attr_map.get("id")
|
||||||
|
if element_id:
|
||||||
|
self.id_locations.setdefault(element_id, []).append((line, offset))
|
||||||
|
|
||||||
if self.svg_stack:
|
if self.svg_stack:
|
||||||
parent = self.svg_stack[-1]
|
parent = self.svg_stack[-1]
|
||||||
if len(self.element_stack) == parent.content_depth:
|
if len(self.element_stack) == parent.content_depth:
|
||||||
if parent.first_child is None:
|
if parent.first_child is None:
|
||||||
parent.first_child = tag
|
parent.first_child = tag
|
||||||
element_id = attr_map.get("id")
|
|
||||||
if element_id:
|
if element_id:
|
||||||
for svg in self.svg_stack:
|
for svg in self.svg_stack:
|
||||||
svg.ids.add(element_id)
|
svg.ids.add(element_id)
|
||||||
@@ -174,6 +177,19 @@ def lint_accessible_svgs(text):
|
|||||||
def add(line, offset, message):
|
def add(line, offset, message):
|
||||||
findings.append((line, offset, "a11y", message))
|
findings.append((line, offset, "a11y", message))
|
||||||
|
|
||||||
|
naming_ids = {
|
||||||
|
element.element_id
|
||||||
|
for svg in parser.svgs
|
||||||
|
if (svg.attrs.get("aria-hidden") or "").casefold() != "true"
|
||||||
|
for element in svg.titles + svg.descriptions
|
||||||
|
if element.element_id
|
||||||
|
}
|
||||||
|
for element_id in sorted(naming_ids):
|
||||||
|
locations = parser.id_locations[element_id]
|
||||||
|
if len(locations) > 1:
|
||||||
|
line, offset = locations[1]
|
||||||
|
add(line, offset, f'duplicate accessible-name id="{element_id}" is not allowed')
|
||||||
|
|
||||||
for svg in parser.svgs:
|
for svg in parser.svgs:
|
||||||
aria_hidden = (svg.attrs.get("aria-hidden") or "").casefold()
|
aria_hidden = (svg.attrs.get("aria-hidden") or "").casefold()
|
||||||
if aria_hidden == "true":
|
if aria_hidden == "true":
|
||||||
@@ -183,6 +199,25 @@ def lint_accessible_svgs(text):
|
|||||||
add(svg.line, svg.offset, 'diagram <svg> must carry role="img"')
|
add(svg.line, svg.offset, 'diagram <svg> must carry role="img"')
|
||||||
|
|
||||||
labelled_by = (svg.attrs.get("aria-labelledby") or "").split()
|
labelled_by = (svg.attrs.get("aria-labelledby") or "").split()
|
||||||
|
accessible_name_ids = labelled_by + [
|
||||||
|
element.element_id
|
||||||
|
for element in svg.titles + svg.descriptions
|
||||||
|
if element.element_id
|
||||||
|
]
|
||||||
|
placeholder_ids = list(
|
||||||
|
dict.fromkeys(
|
||||||
|
element_id
|
||||||
|
for element_id in accessible_name_ids
|
||||||
|
if "[" in element_id or "]" in element_id
|
||||||
|
)
|
||||||
|
)
|
||||||
|
if placeholder_ids:
|
||||||
|
add(
|
||||||
|
svg.line,
|
||||||
|
svg.offset,
|
||||||
|
"accessible-name IDs contain unresolved placeholder(s): "
|
||||||
|
+ ", ".join(placeholder_ids),
|
||||||
|
)
|
||||||
if not labelled_by:
|
if not labelled_by:
|
||||||
add(
|
add(
|
||||||
svg.line,
|
svg.line,
|
||||||
@@ -421,18 +456,15 @@ def main():
|
|||||||
colors, rgb_triplets = allowed_colors()
|
colors, rgb_triplets = allowed_colors()
|
||||||
|
|
||||||
skipped = 0
|
skipped = 0
|
||||||
|
baseline = set()
|
||||||
if args.all:
|
if args.all:
|
||||||
paths = sorted(ASSET_DIR.glob("example-*.html"))
|
paths = sorted(ASSET_DIR.glob("example-*.html"))
|
||||||
if args.baseline:
|
if args.baseline:
|
||||||
baseline = load_baseline()
|
baseline = load_baseline()
|
||||||
selected = []
|
|
||||||
for path in paths:
|
for path in paths:
|
||||||
relative = display_path(path)
|
relative = display_path(path)
|
||||||
if path.name in baseline or relative in baseline:
|
if path.name in baseline or relative in baseline:
|
||||||
skipped += 1
|
skipped += 1
|
||||||
else:
|
|
||||||
selected.append(path)
|
|
||||||
paths = selected
|
|
||||||
else:
|
else:
|
||||||
paths = args.files
|
paths = args.files
|
||||||
|
|
||||||
@@ -440,7 +472,11 @@ def main():
|
|||||||
for path in paths:
|
for path in paths:
|
||||||
try:
|
try:
|
||||||
text = path.read_text(encoding="utf-8")
|
text = path.read_text(encoding="utf-8")
|
||||||
findings = lint_text(text, colors, rgb_triplets)
|
relative = display_path(path)
|
||||||
|
if args.baseline and (path.name in baseline or relative in baseline):
|
||||||
|
findings = lint_accessible_svgs(text)
|
||||||
|
else:
|
||||||
|
findings = lint_text(text, colors, rgb_triplets)
|
||||||
except OSError as error:
|
except OSError as error:
|
||||||
findings = [(0, 0, "read-error", str(error))]
|
findings = [(0, 0, "read-error", str(error))]
|
||||||
|
|
||||||
|
|||||||
@@ -3,6 +3,7 @@
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import shutil
|
||||||
import subprocess
|
import subprocess
|
||||||
import sys
|
import sys
|
||||||
import tempfile
|
import tempfile
|
||||||
@@ -116,6 +117,21 @@ def main() -> int:
|
|||||||
'bare id="title" and id="desc" are not allowed',
|
'bare id="title" and id="desc" are not allowed',
|
||||||
directory,
|
directory,
|
||||||
)
|
)
|
||||||
|
require_failure(
|
||||||
|
"placeholder-ids",
|
||||||
|
VALID_SVG.replace("fixture-title", "[diagram-slug]-title").replace(
|
||||||
|
"fixture-desc", "[diagram-slug]-desc"
|
||||||
|
),
|
||||||
|
"accessible-name IDs contain unresolved placeholder(s): "
|
||||||
|
"[diagram-slug]-title, [diagram-slug]-desc",
|
||||||
|
directory,
|
||||||
|
)
|
||||||
|
require_failure(
|
||||||
|
"duplicate-naming-ids",
|
||||||
|
VALID_SVG + VALID_SVG,
|
||||||
|
'duplicate accessible-name id="fixture-desc" is not allowed',
|
||||||
|
directory,
|
||||||
|
)
|
||||||
require_pass(
|
require_pass(
|
||||||
"decorative",
|
"decorative",
|
||||||
'<svg xmlns="http://www.w3.org/2000/svg" aria-hidden="true">'
|
'<svg xmlns="http://www.w3.org/2000/svg" aria-hidden="true">'
|
||||||
@@ -123,6 +139,50 @@ def main() -> int:
|
|||||||
directory,
|
directory,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
project = directory / "baseline-project"
|
||||||
|
(project / "scripts").mkdir(parents=True)
|
||||||
|
(project / "skills/diagram-design/assets").mkdir(parents=True)
|
||||||
|
(project / "skills/diagram-design/references").mkdir(parents=True)
|
||||||
|
shutil.copy2(LINTER, project / "scripts/lint-skin.py")
|
||||||
|
shutil.copy2(
|
||||||
|
ROOT / "skills/diagram-design/references/style-guide.md",
|
||||||
|
project / "skills/diagram-design/references/style-guide.md",
|
||||||
|
)
|
||||||
|
(project / "scripts/lint-skin-baseline.txt").write_text(
|
||||||
|
"example-baseline.html\n", encoding="utf-8"
|
||||||
|
)
|
||||||
|
(project / "skills/diagram-design/assets/example-baseline.html").write_text(
|
||||||
|
'<svg style="color: #123456"></svg>\n', encoding="utf-8"
|
||||||
|
)
|
||||||
|
baseline_result = subprocess.run(
|
||||||
|
[
|
||||||
|
sys.executable,
|
||||||
|
str(project / "scripts/lint-skin.py"),
|
||||||
|
"--all",
|
||||||
|
"--baseline",
|
||||||
|
],
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
check=False,
|
||||||
|
)
|
||||||
|
if baseline_result.returncode != 1:
|
||||||
|
raise AssertionError(
|
||||||
|
"baseline-a11y: expected exit 1, got "
|
||||||
|
f"{baseline_result.returncode}\n"
|
||||||
|
f"{baseline_result.stdout}{baseline_result.stderr}"
|
||||||
|
)
|
||||||
|
if "a11y: diagram <svg> must carry role=\"img\"" not in baseline_result.stdout:
|
||||||
|
raise AssertionError(
|
||||||
|
"baseline-a11y: missing expected accessibility finding\n"
|
||||||
|
f"{baseline_result.stdout}"
|
||||||
|
)
|
||||||
|
if "color:" in baseline_result.stdout:
|
||||||
|
raise AssertionError(
|
||||||
|
"baseline-a11y: legacy visual finding was not exempted\n"
|
||||||
|
f"{baseline_result.stdout}"
|
||||||
|
)
|
||||||
|
print("OK: baseline file receives a11y checks but keeps visual exemptions")
|
||||||
|
|
||||||
print("All accessible-SVG lint cases passed.")
|
print("All accessible-SVG lint cases passed.")
|
||||||
return 0
|
return 0
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user