From f119ff63069b8028943cd2532efc90da90952c97 Mon Sep 17 00:00:00 2001 From: Ali Bahaloo Date: Sat, 25 Jul 2026 10:42:09 -0700 Subject: [PATCH 1/2] fix(report): make filter buttons legible and open advisories in a new tab MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects found during live testing of the 2.3.0 HTML report. 1. Filter button labels were invisible until clicked. Root cause was a mistake in the stylesheet, not the browser: the report declared `:root { color-scheme: light dark; }` while the page hardcodes light body colours and implements no dark theme. That declaration tells the browser the page supports dark mode, so it styled form controls for dark mode — white text — against the `#fff` background set here. `.controls button` set a background but no `color`, and `font: inherit` does not carry colour, so nothing overrode the user agent. Only the pressed button was visible, because it explicitly sets `color: #fff` on a dark background. Reproduced with prefers-color-scheme: dark, where the unpressed buttons computed to rgb(255,255,255) on rgb(255,255,255). Fixed at both levels: `color-scheme: light` so the report stops advertising support it does not have, and an explicit `color` on the buttons so they are correct regardless of user agent or scheme. The explicit colour is the real fix; it does not depend on colour-scheme behaviour. Verified under dark mode: unpressed buttons now compute to 17.2:1 contrast and the pressed button to 8.7:1, both above WCAG AAA. Also adds hover and focus-visible states — a button with no hover feedback reads as broken, and the focus ring restores keyboard affordance that came from the user-agent style being overridden. 2. Advisory links replaced the report in the same tab. `target="_blank"` was missing. A reader working through a filtered list lost their filter and scroll position on every advisory they opened. `rel="noreferrer noopener"` was already present and is the required companion, so it now denies the opened page a handle back to the report. Both are covered by regression tests that were confirmed to fail against the pre-fix code (5 of 8 fail when the defects are reintroduced), rather than only passing after the fix: the CSS tests parse the `.controls button` declaration block and assert an explicit colour distinct from its background, and the link tests assert every anchor carries both target and rel. Tests: 236 -> 244. Co-Authored-By: Claude Opus 5 --- cisco_advisory_impact_agent/html_report.py | 23 ++++++-- tests/test_html_report.py | 68 ++++++++++++++++++++++ 2 files changed, 87 insertions(+), 4 deletions(-) diff --git a/cisco_advisory_impact_agent/html_report.py b/cisco_advisory_impact_agent/html_report.py index 8b98418..833800b 100644 --- a/cisco_advisory_impact_agent/html_report.py +++ b/cisco_advisory_impact_agent/html_report.py @@ -120,8 +120,12 @@ def _advisories_html(advisories): for adv in ordered: advisory_id = esc(adv.get("advisoryId")) url = esc(adv.get("publicationUrl") or "") - link = f'{advisory_id}' if url \ - else advisory_id + # Opens in a new tab so the reader keeps their place in the report — they are + # working through a list, and navigating away loses their filter and scroll + # position. `rel` is required alongside `target` to deny the opened page a + # reference back to this document. + link = (f'' + f'{advisory_id}') if url else advisory_id sir = esc(adv.get("sir") or "Unrated") fix = first_fix_of(adv) fix_html = (f'fixed in {esc(fix)}' if fix @@ -197,7 +201,11 @@ def reason_text(reason): # Document # --------------------------------------------------------------------------- # _CSS = """ -:root { color-scheme: light dark; } +/* This report is light-themed throughout (see the hardcoded body colours below), so it + must not advertise dark-mode support. Declaring `light dark` made the browser style + form controls for dark mode — giving the filter buttons white text on the white + background set here, invisible until pressed. */ +:root { color-scheme: light; } * { box-sizing: border-box; } body { margin: 0; padding: 2rem 1.5rem; font: 15px/1.5 -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, Helvetica, Arial, sans-serif; color: #1b1b1b; @@ -232,10 +240,17 @@ def reason_text(reason): .nofix { font-style: italic; color: #8a5a00; } .controls { margin: .75rem 0 1rem; display: flex; gap: .5rem; flex-wrap: wrap; align-items: center; } +/* `color` is set explicitly rather than left to the user agent: `font: inherit` does not + carry colour, so an inherited default is whatever the browser decides and can end up + matching the background. */ .controls button { font: inherit; font-size: .85rem; padding: .3rem .7rem; cursor: pointer; - border: 1px solid #c3c7cc; border-radius: 4px; background: #fff; } + border: 1px solid #c3c7cc; border-radius: 4px; + background: #fff; color: #1b1b1b; } +.controls button:hover { background: #eef1f4; border-color: #9aa1a9; } +.controls button:focus-visible { outline: 2px solid #1f4e78; outline-offset: 1px; } .controls button[aria-pressed="true"] { background: #1f4e78; color: #fff; border-color: #1f4e78; } +.controls button[aria-pressed="true"]:hover { background: #1a4265; } .controls .spacer { flex: 1 1 auto; } .summary { background: #fff; border: 1px solid #e2e4e7; border-radius: 6px; padding: 1rem 1.1rem; margin: 1rem 0; } diff --git a/tests/test_html_report.py b/tests/test_html_report.py index 06f54f4..6dfcaa2 100644 --- a/tests/test_html_report.py +++ b/tests/test_html_report.py @@ -201,6 +201,74 @@ def test_single_advisory_is_not_pluralised(self): self.assertIn("1 advisory<", text) +class ControlLegibilityRegressionTests(unittest.TestCase): + """The filter buttons were once white-on-white until pressed. Guard both causes.""" + + def _rule(self, selector): + """The declaration block for an exact selector in the inlined stylesheet.""" + match = re.search(re.escape(selector) + r"\s*\{([^}]*)\}", hr._CSS) + self.assertIsNotNone(match, f"no CSS rule found for {selector!r}") + return match.group(1) + + def test_buttons_set_an_explicit_text_colour(self): + # `font: inherit` does not carry colour, so leaving it to the user agent is what + # allowed the text to match the background. + block = self._rule(".controls button") + self.assertRegex(block, r"(^|;|\s)color\s*:", + "'.controls button' must set an explicit color") + + def test_button_colour_differs_from_its_background(self): + block = self._rule(".controls button") + colours = dict(re.findall(r"(background|color)\s*:\s*([^;]+)", block)) + self.assertIn("color", colours) + self.assertIn("background", colours) + self.assertNotEqual(colours["color"].strip().lower(), + colours["background"].strip().lower()) + + def test_report_does_not_advertise_dark_mode_it_does_not_implement(self): + # The page hardcodes light body colours; claiming `light dark` made the browser + # style form controls for dark mode against a light background. + self.assertNotIn("color-scheme: light dark", hr._CSS) + self.assertIn("color-scheme: light", hr._CSS) + + def test_pressed_and_unpressed_states_are_visually_distinct(self): + unpressed = self._rule(".controls button") + pressed = self._rule('.controls button[aria-pressed="true"]') + self.assertNotEqual( + re.search(r"background\s*:\s*([^;]+)", unpressed).group(1).strip(), + re.search(r"background\s*:\s*([^;]+)", pressed).group(1).strip()) + + +class AdvisoryLinkTargetTests(unittest.TestCase): + """Advisory links must open in a new tab and not leak a handle back to the report.""" + + def test_advisory_link_opens_in_a_new_tab(self): + text = hr.render([_device(advisories=[fx.advisory()])]) + self.assertIn('target="_blank"', text) + + def test_every_advisory_link_has_a_target(self): + advisories = [fx.advisory(advisory_id=f"cisco-sa-{i:02d}") for i in range(5)] + text = hr.render([_device(advisories=advisories)]) + anchors = re.findall(r"]*>", text) + self.assertEqual(len(anchors), 5) + for anchor in anchors: + self.assertIn('target="_blank"', anchor) + + def test_new_tab_links_carry_noopener_and_noreferrer(self): + # Required alongside target="_blank": denies the opened page window.opener. + text = hr.render([_device(advisories=[fx.advisory()])]) + for anchor in re.findall(r"]*>", text): + self.assertIn("noopener", anchor) + self.assertIn("noreferrer", anchor) + + def test_an_advisory_without_a_url_renders_no_anchor(self): + adv = dict(fx.advisory()) + adv["publicationUrl"] = "" + text = hr.render([_device(advisories=[adv])]) + self.assertEqual(re.findall(r"]*>", text), []) + self.assertIn(adv["advisoryId"], text) + + class InteractivityTests(unittest.TestCase): def test_filter_controls_exist_for_each_verdict(self): devices = [ From 5170b4b1c7540af223b1e7d20c59b2a67e3ad2e8 Mon Sep 17 00:00:00 2001 From: Ali Bahaloo Date: Sat, 25 Jul 2026 10:45:35 -0700 Subject: [PATCH 2/2] chore(release): bump VERSION to 2.3.1 Patch bump for the HTML report fixes in f119ff6 (invisible filter button labels; advisory links opening in the same tab). Behaviour-only fixes to generated output, no API or interface change, so PATCH per semver. Per the release runbook the bump lands in this PR so `main` always reflects the latest released version; the tag is pushed after merge. Co-Authored-By: Claude Opus 5 --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 276cbf9..2bf1c1c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.3.0 +2.3.1