Skip to content

Commit e67e8fa

Browse files
stevewalloneclaude
andauthored
fix: render report summary charts after the table-of-contents rebuild (#15195)
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 <noreply@anthropic.com>
1 parent 325390d commit e67e8fa

15 files changed

Lines changed: 248 additions & 28 deletions

dojo/templates/dojo/endpoint_pdf_report.html

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ <h6>Notes</h6>
285285
<script src="{{ host }}{% static "chartjs-adapter-moment/dist/chartjs-adapter-moment.js" %}"></script>
286286
<script src="{{ host }}{% static "dojo/js/metrics.js" %}"></script>
287287
<script type="text/javascript">
288-
$(function () {
288+
function renderReportCharts() {
289289
var data = {};
290290
var data_1 = [];
291291
var ticks = [];
@@ -346,8 +346,9 @@ <h6>Notes</h6>
346346
[4, info],
347347
];
348348
open_findings(d1, d2, d3, d4, d5, ticks);
349-
});
349+
}
350350

351+
{% if include_table_of_contents%}
351352
window.onload = function () {
352353
var toc = "";
353354
var level = 3;
@@ -417,7 +418,18 @@ <h6>Notes</h6>
417418
}
418419

419420
document.getElementById("toc").innerHTML += toc;
421+
422+
// Render the summary charts AFTER the table-of-contents builder has
423+
// rewritten #contents.innerHTML. Rendering earlier (on
424+
// DOMContentLoaded) means this reassignment re-parses the subtree and
425+
// discards the already-painted Chart.js canvases, leaving blank charts.
426+
renderReportCharts();
420427
};
428+
{% else %}
429+
$(function () {
430+
renderReportCharts();
431+
});
432+
{% endif %}
421433

422434
</script>
423435
{% endblock %}

dojo/templates/dojo/engagement_pdf_report.html

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,7 @@ <h6>Notes</h6>
420420
<script src="{{ host }}{% static "chartjs-adapter-moment/dist/chartjs-adapter-moment.js" %}"></script>
421421
<script src="{{ host }}{% static "dojo/js/metrics.js" %}"></script>
422422
<script type="text/javascript">
423-
$(function () {
423+
function renderReportCharts() {
424424
var data = {};
425425
var data_1 = [];
426426
var ticks = [];
@@ -497,7 +497,7 @@ <h6>Notes</h6>
497497
opened_per_month_2(critical, high, medium, low);
498498
{% endif %}
499499

500-
});
500+
}
501501

502502
{% if include_table_of_contents%}
503503
window.onload = function () {
@@ -569,7 +569,17 @@ <h6>Notes</h6>
569569
}
570570

571571
document.getElementById("toc").innerHTML += toc;
572+
573+
// Render the summary charts AFTER the table-of-contents builder has
574+
// rewritten #contents.innerHTML. Rendering earlier (on
575+
// DOMContentLoaded) means this reassignment re-parses the subtree and
576+
// discards the already-painted Chart.js canvases, leaving blank charts.
577+
renderReportCharts();
572578
};
579+
{% else %}
580+
$(function () {
581+
renderReportCharts();
582+
});
573583
{% endif %}
574584
</script>
575585
{% endblock %}

dojo/templates/dojo/finding_pdf_report.html

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ <h6>Notes</h6>
270270
<script src="{{ host }}{% static "chartjs-adapter-moment/dist/chartjs-adapter-moment.js" %}"></script>
271271
<script src="{{ host }}{% static "dojo/js/metrics.js" %}"></script>
272272
<script type="text/javascript">
273-
$(function () {
273+
function renderReportCharts() {
274274
var data = {};
275275
var data_1 = [];
276276
var ticks = [];
@@ -331,8 +331,9 @@ <h6>Notes</h6>
331331
[4, info],
332332
];
333333
open_findings(d1, d2, d3, d4, d5, ticks);
334-
});
334+
}
335335

336+
{% if include_table_of_contents%}
336337
window.onload = function () {
337338
var toc = "";
338339
var level = 3;
@@ -406,7 +407,18 @@ <h6>Notes</h6>
406407
if (document.getElementById("toc") != null) {
407408
document.getElementById("toc").innerHTML += toc;
408409
}
410+
411+
// Render the summary charts AFTER the table-of-contents builder has
412+
// rewritten #contents.innerHTML. Rendering earlier (on
413+
// DOMContentLoaded) means this reassignment re-parses the subtree and
414+
// discards the already-painted Chart.js canvases, leaving blank charts.
415+
renderReportCharts();
409416
};
417+
{% else %}
418+
$(function () {
419+
renderReportCharts();
420+
});
421+
{% endif %}
410422

411423
</script>
412424
{% endblock %}

dojo/templates/dojo/product_endpoint_pdf_report.html

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ <h6>Notes</h6>
338338
<script src="{% static "dojo/js/metrics.js" %}"></script>
339339
{% endblock metrics %}
340340
<script type="text/javascript">
341-
$(function () {
341+
function renderReportCharts() {
342342
var critical = 0;
343343
var high = 0;
344344
var medium = 0;
@@ -471,8 +471,9 @@ <h6>Notes</h6>
471471
i++;
472472
});
473473
finding_age(data_1, ticks);
474-
});
474+
}
475475

476+
{% if include_table_of_contents%}
476477
window.onload = function () {
477478
var toc = "";
478479
var level = 3;
@@ -542,6 +543,17 @@ <h6>Notes</h6>
542543
}
543544

544545
document.getElementById("toc").innerHTML += toc;
546+
547+
// Render the summary charts AFTER the table-of-contents builder has
548+
// rewritten #contents.innerHTML. Rendering earlier (on
549+
// DOMContentLoaded) means this reassignment re-parses the subtree and
550+
// discards the already-painted Chart.js canvases, leaving blank charts.
551+
renderReportCharts();
545552
};
553+
{% else %}
554+
$(function () {
555+
renderReportCharts();
556+
});
557+
{% endif %}
546558
</script>
547559
{% endblock %}

dojo/templates/dojo/product_pdf_report.html

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ <h6>Notes</h6>
391391
<script src="{{ host }}{% static "chartjs-adapter-moment/dist/chartjs-adapter-moment.js" %}"></script>
392392
<script src="{{ host }}{% static "dojo/js/metrics.js" %}"></script>
393393
<script type="text/javascript">
394-
$(function () {
394+
function renderReportCharts() {
395395
var critical = 0;
396396
var high = 0;
397397
var medium = 0;
@@ -488,8 +488,9 @@ <h6>Notes</h6>
488488
i++;
489489
});
490490
finding_age(data_1, ticks);
491-
});
491+
}
492492

493+
{% if include_table_of_contents%}
493494
window.onload = function () {
494495
var toc = "";
495496
var level = 3;
@@ -559,6 +560,17 @@ <h6>Notes</h6>
559560
}
560561

561562
document.getElementById("toc").innerHTML += toc;
563+
564+
// Render the summary charts AFTER the table-of-contents builder has
565+
// rewritten #contents.innerHTML. Rendering earlier (on
566+
// DOMContentLoaded) means this reassignment re-parses the subtree and
567+
// discards the already-painted Chart.js canvases, leaving blank charts.
568+
renderReportCharts();
562569
};
570+
{% else %}
571+
$(function () {
572+
renderReportCharts();
573+
});
574+
{% endif %}
563575
</script>
564576
{% endblock %}

dojo/templates/dojo/product_type_pdf_report.html

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ <h6>Notes</h6>
320320
<script src="{{ host }}{% static "chartjs-adapter-moment/dist/chartjs-adapter-moment.js" %}"></script>
321321
<script src="{{ host }}{% static "dojo/js/metrics.js" %}"></script>
322322
<script type="text/javascript">
323-
$(function () {
323+
function renderReportCharts() {
324324
var critical = 0;
325325
var high = 0;
326326
var medium = 0;
@@ -375,7 +375,7 @@ <h6>Notes</h6>
375375
{% endfor %}
376376
opened_per_month_2(critical, high, medium, low);
377377
{% endif %}
378-
});
378+
}
379379

380380
{% if include_table_of_contents%}
381381
window.onload = function () {
@@ -447,7 +447,17 @@ <h6>Notes</h6>
447447
}
448448

449449
document.getElementById("toc").innerHTML += toc;
450+
451+
// Render the summary charts AFTER the table-of-contents builder has
452+
// rewritten #contents.innerHTML. Rendering earlier (on
453+
// DOMContentLoaded) means this reassignment re-parses the subtree and
454+
// discards the already-painted Chart.js canvases, leaving blank charts.
455+
renderReportCharts();
450456
};
457+
{% else %}
458+
$(function () {
459+
renderReportCharts();
460+
});
451461
{% endif %}
452462
</script>
453463
{% endblock %}

dojo/templates/dojo/test_pdf_report.html

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@ <h6>Notes</h6>
434434
<script src="{{ host }}{% static "dojo/js/metrics.js" %}"></script>
435435
<script type="text/javascript">
436436
{% if include_executive_summary %}
437-
$(function () {
437+
function renderReportCharts() {
438438
var critical = 0;
439439
var high = 0;
440440
var medium = 0;
@@ -474,7 +474,6 @@ <h6>Notes</h6>
474474
[4, info],
475475
];
476476
open_findings(d1, d2, d3, d4, d5, ticks);
477-
});
478477

479478
var data = {};
480479
var data_1 = []
@@ -510,8 +509,10 @@ <h6>Notes</h6>
510509
{% if endpoint_opened_per_month %}
511510
opened_per_month_2(critical, high, medium, low);
512511
{% endif %}
512+
}
513513
{% endif %}
514514

515+
{% if include_table_of_contents%}
515516
window.onload = function () {
516517
var toc = "";
517518
var level = 3;
@@ -582,6 +583,21 @@ <h6>Notes</h6>
582583
}
583584

584585
document.getElementById("toc").innerHTML += toc;
586+
587+
{% if include_executive_summary %}
588+
// Render the summary charts AFTER the table-of-contents builder has
589+
// rewritten #contents.innerHTML. Rendering earlier (on
590+
// DOMContentLoaded) means this reassignment re-parses the subtree and
591+
// discards the already-painted Chart.js canvases, leaving blank charts.
592+
renderReportCharts();
593+
{% endif %}
585594
};
595+
{% else %}
596+
{% if include_executive_summary %}
597+
$(function () {
598+
renderReportCharts();
599+
});
600+
{% endif %}
601+
{% endif %}
586602
</script>
587603
{% endblock %}

dojo/templates_classic/dojo/endpoint_pdf_report.html

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ <h6>Notes</h6>
287287
<script src="{{ host }}{% static "flot/jquery.flot.stack.js" %}"></script>
288288
<script src="{{ host }}{% static "dojo/js/classic/metrics.js" %}"></script>
289289
<script type="text/javascript">
290-
$(function () {
290+
function renderReportCharts() {
291291
var data = {};
292292
var data_1 = [];
293293
var ticks = [];
@@ -348,8 +348,9 @@ <h6>Notes</h6>
348348
[4, info],
349349
];
350350
open_findings(d1, d2, d3, d4, d5, ticks);
351-
});
351+
}
352352

353+
{% if include_table_of_contents%}
353354
window.onload = function () {
354355
var toc = "";
355356
var level = 3;
@@ -419,7 +420,18 @@ <h6>Notes</h6>
419420
}
420421

421422
document.getElementById("toc").innerHTML += toc;
423+
424+
// Render the summary charts AFTER the table-of-contents builder has
425+
// rewritten #contents.innerHTML. Rendering earlier (on
426+
// DOMContentLoaded) means this reassignment re-parses the subtree and
427+
// discards the already-painted chart canvases, leaving blank charts.
428+
renderReportCharts();
422429
};
430+
{% else %}
431+
$(function () {
432+
renderReportCharts();
433+
});
434+
{% endif %}
423435

424436
</script>
425437
{% endblock %}

dojo/templates_classic/dojo/engagement_pdf_report.html

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,7 @@ <h6>Notes</h6>
422422
<script src="{{ host }}{% static "flot/jquery.flot.stack.js" %}"></script>
423423
<script src="{{ host }}{% static "dojo/js/classic/metrics.js" %}"></script>
424424
<script type="text/javascript">
425-
$(function () {
425+
function renderReportCharts() {
426426
var data = {};
427427
var data_1 = [];
428428
var ticks = [];
@@ -499,7 +499,7 @@ <h6>Notes</h6>
499499
opened_per_month_2(critical, high, medium, low);
500500
{% endif %}
501501

502-
});
502+
}
503503

504504
{% if include_table_of_contents%}
505505
window.onload = function () {
@@ -571,7 +571,17 @@ <h6>Notes</h6>
571571
}
572572

573573
document.getElementById("toc").innerHTML += toc;
574+
575+
// Render the summary charts AFTER the table-of-contents builder has
576+
// rewritten #contents.innerHTML. Rendering earlier (on
577+
// DOMContentLoaded) means this reassignment re-parses the subtree and
578+
// discards the already-painted chart canvases, leaving blank charts.
579+
renderReportCharts();
574580
};
581+
{% else %}
582+
$(function () {
583+
renderReportCharts();
584+
});
575585
{% endif %}
576586
</script>
577587
{% endblock %}

dojo/templates_classic/dojo/finding_pdf_report.html

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ <h6>Notes</h6>
266266
<script src="{{ host }}{% static "flot/jquery.flot.stack.js" %}"></script>
267267
<script src="{{ host }}{% static "dojo/js/classic/metrics.js" %}"></script>
268268
<script type="text/javascript">
269-
$(function () {
269+
function renderReportCharts() {
270270
var data = {};
271271
var data_1 = [];
272272
var ticks = [];
@@ -327,8 +327,9 @@ <h6>Notes</h6>
327327
[4, info],
328328
];
329329
open_findings(d1, d2, d3, d4, d5, ticks);
330-
});
330+
}
331331

332+
{% if include_table_of_contents%}
332333
window.onload = function () {
333334
var toc = "";
334335
var level = 3;
@@ -402,7 +403,18 @@ <h6>Notes</h6>
402403
if (document.getElementById("toc") != null) {
403404
document.getElementById("toc").innerHTML += toc;
404405
}
406+
407+
// Render the summary charts AFTER the table-of-contents builder has
408+
// rewritten #contents.innerHTML. Rendering earlier (on
409+
// DOMContentLoaded) means this reassignment re-parses the subtree and
410+
// discards the already-painted chart canvases, leaving blank charts.
411+
renderReportCharts();
405412
};
413+
{% else %}
414+
$(function () {
415+
renderReportCharts();
416+
});
417+
{% endif %}
406418

407419
</script>
408420
{% endblock %}

0 commit comments

Comments
 (0)