From 6ff0eb6d497fd5b3f711da0d95febd7e0380402a Mon Sep 17 00:00:00 2001 From: Steve Wall Date: Tue, 16 Jun 2026 14:48:53 -0600 Subject: [PATCH] fix: render report summary charts after the table-of-contents rebuild In every *_pdf_report.html template the Executive-Summary charts render from a jQuery ready handler, while the table-of-contents builder runs on window.onload and reassigns #contents.innerHTML. The chart containers live inside #contents, so whenever the charts paint before the TOC rebuild, the innerHTML round-trip re-parses the subtree and silently discards the painted canvases (Chart.js instances in the new UI, Flot canvases in the classic UI), leaving blank chart boxes with no console error. The two orderings race each other: on a warm local cache the async ready callback can land after onload and the charts survive, but with realistic network latency the charts paint first and are destroyed. The symptom was reported as far back as #2294 and #3253 (2020, Flot era) and never fixed - both were auto-closed by the stale bot. Wrap the chart rendering in renderReportCharts() in all seven report templates (engagement, product, product type, test, endpoint, finding, product endpoint) in both template trees (dojo/templates and dojo/templates_classic) and invoke it after the TOC builder has finished (or on a ready handler when the TOC is disabled), so the charts are always created into the final DOM. In five of the seven templates (product, test, endpoint, finding, product endpoint) the window.onload TOC builder was also missing the {% if include_table_of_contents %} guard the other templates have, so it ran even with the table of contents disabled. Because the #contents wrapper is itself rendered only when the TOC is enabled, the ungated handler dereferences a null #contents on its first line and throws an uncaught TypeError before it can rewrite anything (the finding template null-checks #contents and stays silent) - so the charts survive in that configuration, but every such report logs a console error. Those five now carry the same guard as their siblings, so the handler is not emitted when the TOC is off. In the test template the chart code was split between a ready handler and top-level script code; both pieces are consolidated into renderReportCharts() preserving their order, and its call sites stay inside {% if include_executive_summary %} because the function definition is only rendered with the executive summary. Add a regression assertion to the Selenium report tests that the summary charts end up painted when executive summary and table of contents are enabled together (factored into a helper and applied to the product type, product, engagement, test, and product-endpoint report tests). The check is chart-library agnostic (painted canvas pixels), so it holds on both the Chart.js and classic Flot report pages. Co-Authored-By: Claude Opus 4.8 Co-Authored-By: Claude Fable 5 --- dojo/templates/dojo/endpoint_pdf_report.html | 16 +++++- .../templates/dojo/engagement_pdf_report.html | 14 ++++- dojo/templates/dojo/finding_pdf_report.html | 16 +++++- .../dojo/product_endpoint_pdf_report.html | 16 +++++- dojo/templates/dojo/product_pdf_report.html | 16 +++++- .../dojo/product_type_pdf_report.html | 14 ++++- dojo/templates/dojo/test_pdf_report.html | 20 ++++++- .../dojo/endpoint_pdf_report.html | 16 +++++- .../dojo/engagement_pdf_report.html | 14 ++++- .../dojo/finding_pdf_report.html | 16 +++++- .../dojo/product_endpoint_pdf_report.html | 16 +++++- .../dojo/product_pdf_report.html | 16 +++++- .../dojo/product_type_pdf_report.html | 14 ++++- .../dojo/test_pdf_report.html | 20 ++++++- tests/report_builder_test.py | 52 +++++++++++++++++++ 15 files changed, 248 insertions(+), 28 deletions(-) diff --git a/dojo/templates/dojo/endpoint_pdf_report.html b/dojo/templates/dojo/endpoint_pdf_report.html index 96d76d0b71f..84d152f97e5 100644 --- a/dojo/templates/dojo/endpoint_pdf_report.html +++ b/dojo/templates/dojo/endpoint_pdf_report.html @@ -285,7 +285,7 @@
Notes
{% endblock %} diff --git a/dojo/templates/dojo/engagement_pdf_report.html b/dojo/templates/dojo/engagement_pdf_report.html index 8b0dd906af2..8ae8c2d29a5 100644 --- a/dojo/templates/dojo/engagement_pdf_report.html +++ b/dojo/templates/dojo/engagement_pdf_report.html @@ -420,7 +420,7 @@
Notes
{% endblock %} diff --git a/dojo/templates/dojo/finding_pdf_report.html b/dojo/templates/dojo/finding_pdf_report.html index 5b1011fd178..c5667c1c37e 100644 --- a/dojo/templates/dojo/finding_pdf_report.html +++ b/dojo/templates/dojo/finding_pdf_report.html @@ -270,7 +270,7 @@
Notes
{% endblock %} diff --git a/dojo/templates/dojo/product_endpoint_pdf_report.html b/dojo/templates/dojo/product_endpoint_pdf_report.html index b86e2860267..77348e6eb3d 100644 --- a/dojo/templates/dojo/product_endpoint_pdf_report.html +++ b/dojo/templates/dojo/product_endpoint_pdf_report.html @@ -338,7 +338,7 @@
Notes
{% endblock metrics %} {% endblock %} diff --git a/dojo/templates/dojo/product_pdf_report.html b/dojo/templates/dojo/product_pdf_report.html index a25e0084192..9501b62afbd 100644 --- a/dojo/templates/dojo/product_pdf_report.html +++ b/dojo/templates/dojo/product_pdf_report.html @@ -391,7 +391,7 @@
Notes
{% endblock %} diff --git a/dojo/templates/dojo/product_type_pdf_report.html b/dojo/templates/dojo/product_type_pdf_report.html index 11eeb095e48..9848f8bc498 100644 --- a/dojo/templates/dojo/product_type_pdf_report.html +++ b/dojo/templates/dojo/product_type_pdf_report.html @@ -320,7 +320,7 @@
Notes
{% endblock %} diff --git a/dojo/templates/dojo/test_pdf_report.html b/dojo/templates/dojo/test_pdf_report.html index c86684da0c6..c0001a3c150 100644 --- a/dojo/templates/dojo/test_pdf_report.html +++ b/dojo/templates/dojo/test_pdf_report.html @@ -434,7 +434,7 @@
Notes
{% endblock %} diff --git a/dojo/templates_classic/dojo/endpoint_pdf_report.html b/dojo/templates_classic/dojo/endpoint_pdf_report.html index a1d6843c144..37076ef3e9c 100644 --- a/dojo/templates_classic/dojo/endpoint_pdf_report.html +++ b/dojo/templates_classic/dojo/endpoint_pdf_report.html @@ -287,7 +287,7 @@
Notes
{% endblock %} diff --git a/dojo/templates_classic/dojo/engagement_pdf_report.html b/dojo/templates_classic/dojo/engagement_pdf_report.html index 7b90548ae1b..b10b3293ecd 100644 --- a/dojo/templates_classic/dojo/engagement_pdf_report.html +++ b/dojo/templates_classic/dojo/engagement_pdf_report.html @@ -422,7 +422,7 @@
Notes
{% endblock %} diff --git a/dojo/templates_classic/dojo/finding_pdf_report.html b/dojo/templates_classic/dojo/finding_pdf_report.html index 86820527db2..d44a3b1092c 100644 --- a/dojo/templates_classic/dojo/finding_pdf_report.html +++ b/dojo/templates_classic/dojo/finding_pdf_report.html @@ -266,7 +266,7 @@
Notes
{% endblock %} diff --git a/dojo/templates_classic/dojo/product_endpoint_pdf_report.html b/dojo/templates_classic/dojo/product_endpoint_pdf_report.html index b5c67cf9b1c..63d91e77624 100644 --- a/dojo/templates_classic/dojo/product_endpoint_pdf_report.html +++ b/dojo/templates_classic/dojo/product_endpoint_pdf_report.html @@ -344,7 +344,7 @@
Notes
{% endblock metrics %} {% endblock %} diff --git a/dojo/templates_classic/dojo/product_pdf_report.html b/dojo/templates_classic/dojo/product_pdf_report.html index 347fa6f810e..3580801dfbe 100644 --- a/dojo/templates_classic/dojo/product_pdf_report.html +++ b/dojo/templates_classic/dojo/product_pdf_report.html @@ -397,7 +397,7 @@
Notes
{% endif %} {% endblock %} diff --git a/dojo/templates_classic/dojo/product_type_pdf_report.html b/dojo/templates_classic/dojo/product_type_pdf_report.html index 36a2f82a1b7..3c5234e6997 100644 --- a/dojo/templates_classic/dojo/product_type_pdf_report.html +++ b/dojo/templates_classic/dojo/product_type_pdf_report.html @@ -322,7 +322,7 @@
Notes
{% endblock %} diff --git a/dojo/templates_classic/dojo/test_pdf_report.html b/dojo/templates_classic/dojo/test_pdf_report.html index 7dd1e0f102c..0989b8796c0 100644 --- a/dojo/templates_classic/dojo/test_pdf_report.html +++ b/dojo/templates_classic/dojo/test_pdf_report.html @@ -436,7 +436,7 @@
Notes
{% endblock %} diff --git a/tests/report_builder_test.py b/tests/report_builder_test.py index 7f11eb93fec..0ce5bb08110 100644 --- a/tests/report_builder_test.py +++ b/tests/report_builder_test.py @@ -3,6 +3,7 @@ from base_test_class import BaseTestCase from product_test import ProductTest +from selenium.common.exceptions import TimeoutException from selenium.webdriver.common.action_chains import ActionChains from selenium.webdriver.common.by import By from selenium.webdriver.support import expected_conditions @@ -30,6 +31,40 @@ def enter_values(self, driver): if "wysiwyg-content" in class_names: widget.find_element(By.CLASS_NAME, "editor").send_keys("wysiwyg") + # Regression guard for the report-chart / table-of-contents race: the + # executive-summary charts are rendered from a ready handler, while the + # table-of-contents builder runs on window.onload and reassigns + # #contents.innerHTML. That reassignment re-parses the subtree and + # destroys any chart canvases painted before it (Chart.js in the new UI, + # Flot in the classic UI). With executive summary AND table of contents + # both enabled, every chart must end up painted once the page settles. + def assert_report_charts_painted(self, chart_ids): + driver = self.driver + charts_painted = ( + "return arguments[0].map(function (id) {" + " var el = document.getElementById(id);" + " if (!el) { return false; }" + " var canvases = el.querySelectorAll('canvas');" + " for (var i = 0; i < canvases.length; i++) {" + " if (!canvases[i].width || !canvases[i].height) { continue; }" + " var ctx = canvases[i].getContext('2d');" + " var data = ctx.getImageData(0, 0, canvases[i].width, canvases[i].height).data;" + " for (var j = 3; j < data.length; j += 64) {" + " if (data[j] > 0) { return true; }" + " }" + " }" + " return false;" + "});" + ) + try: + WebDriverWait(driver, 20).until(lambda d: all(d.execute_script(charts_painted, chart_ids))) + except TimeoutException: + self.fail( + "Executive-summary charts were not painted after page load " + f"({chart_ids} painted: {driver.execute_script(charts_painted, chart_ids)}); " + "the table-of-contents builder likely destroyed them.", + ) + def generate_HTML_report(self): driver = self.driver driver.get(self.base_url + "reports/builder") @@ -59,6 +94,10 @@ def test_product_type_report(self): driver.find_element(By.NAME, "_generate").click() + # opened_per_month_2 is only rendered when the product type has + # endpoint-per-month data, so only the unconditional chart is asserted. + self.assert_report_charts_painted(["open_findings"]) + def test_product_report(self): driver = self.driver self.goto_product_overview(driver) @@ -77,6 +116,8 @@ def test_product_report(self): driver.find_element(By.NAME, "_generate").click() + self.assert_report_charts_painted(["open_findings", "finding_age"]) + def test_engagement_report(self): driver = self.driver self.goto_product_overview(driver) @@ -97,6 +138,8 @@ def test_engagement_report(self): driver.find_element(By.NAME, "_generate").click() + self.assert_report_charts_painted(["open_findings", "finding_age"]) + def test_test_report(self): driver = self.driver self.goto_product_overview(driver) @@ -118,6 +161,8 @@ def test_test_report(self): driver.find_element(By.NAME, "_generate").click() + self.assert_report_charts_painted(["open_findings", "finding_age"]) + def test_product_endpoint_report(self): driver = self.driver self.goto_product_overview(driver) @@ -144,7 +189,14 @@ def test_product_endpoint_report(self): driver.find_element(By.NAME, "_generate").click() + self.assert_report_charts_painted(["accepted_findings", "open_findings", "closed_findings", "finding_age"]) + def test_product_list_report(self): + # Unlike the report tests above, this does not call + # assert_report_charts_painted(): it generates the Findings Report, + # whose view currently errors before the report (and its charts) + # render, so there are no chart canvases to assert on. This test only + # exercises the generate action until that view is fixed. driver = self.driver self.goto_product_overview(driver) driver.find_element(By.ID, "dropdownMenu1").click()