[timdegroot1996]

Fixes phase 2 and 3 (B1-B28) of the JS codebase review: correctness bugs found by reading the code, all re-checked against main at 2.5.0 first.

Run identity

• B4 "run_name" labels broke the timelines, compare and log links. Runs sharing a run name collapsed into one timeline column, compare selected the first run with that name, "All Suites Combined" summed different runs, and log links opened the first match. Graphs now key on run_start and map it to the label only for display (get_run_label_by_run_start in graph_data/helpers.js). The compare selects use run_start as option value, and the bar graphs carry an aligned runStarts array for tooltips and log clicks.
• B25: filtering matches rows to runs on the stored run_start (now kept on every source), so hiding milliseconds can't merge two runs. The log lookup tries an exact match first. The graph keys still use the shown run_start.
• B5: compare radar values are aligned to the suite labels; a suite a run doesn't have is a gap.

Listeners and leaks

• B1: setup_data_and_graphs runs on every page switch. The listeners on template elements (rerun view selects, section filters modal, document, dashboard nav, information popups) are now added once.
• B2: one log click listener per canvas. B3: one ResizeObserver per graph.

Filters and data

• B7: profiles compare custom filters and their modes, and ignore option order and newly appeared unchecked options. B8: no implicit global.
• B14/B15: metadata is parsed with a regex (no JSON.parse crash) and matched per item, so Env: prod no longer matches Env: production. The test section matches whole suite names (is_test_in_suite) and parsed tags.
• B17: the overview duration average excludes the card's own run by position. B18: a run with only skipped tests counts as skipped everywhere (get_run_status, moved to common.js).
• B20: "Only changes" / "No changes" count statuses, so rerun and note borders don't count as a change. B21: the message config keeps the alias/run name tooltips.
• B22: the suite folder reset happens before the suite select is built. B24: an unknown stored graph type falls back to the graph's default.
• B6: "Only last run" no longer crashes on empty data. B16: stats show 0 instead of NaN. B11: bar duration graphs show the newest 30 entries, like the combined view.

UI

• B9: axis label clicks find the tick by its pixel position (also on the timeline axis). B10: server log paths are URL-encoded, with the #id anchor kept outside the encoding.
• B12: confirm_action resolves false on Esc/backdrop and reuses the modal instance. B13: an old alert timer no longer closes a newer alert.
• B23: the uploaded logo is scaled to 256 px; a full or blocked storage shows a message instead of stopping the dashboard.
• B26: clipboard copy is awaited and guarded. B27: date strings use the ISO T form (not verified in Safari; the installed WebKit build doesn't match Playwright). B28: typo and a missing line break.

Choices made

• B18: skip-only runs are skipped, not passed (the rule the date histogram and run cards already used).
• B22: the folder reset stays (every rebuild already did it), the select now follows it.
• B11: this changes the bar duration output for graphs with more than 30 entries; no robot reference needed an update.

Tests

• JS unit tests: 523 passed (42 new). Old tests that asserted label arrays from the timeline data are updated to the run_start contract.
• Robot (Docker): 144 tests, including a new 03_dashboard test for B1. 143 passed on the full run. The one failure was Validate Compare Base View: B5 changes the radar data. The snapshot was regenerated in Docker, and then the suite passed. The diff puts each run's suite durations on its own suites (before, the WebshopAPI values sat on the first WebshopUI suite names) and stores them as numbers.
• Generated the example dashboard from source and clicked through every page and graph type in headless Chrome, in run_start, alias and run_name label modes: no JS errors. In run_name mode the test statistics timeline keeps one column per run.

Dead code (D1-D8) and performance (P1-P10)

Added as two more commits on top of the bug fixes, so they build on them without conflicts.

Dead code: the built-in run/suite/test/keyword stat widgets never render, but their create/update functions computed stats on every pass. Those functions are removed (D1), and a later commit also removes their 34 graph metadata entries and the matching information keys. Also removed: unused variables and writes (D2, D3, D8), an identical-branch ternary (D4), a selector that matched nothing (D5), a donut update right after recreation (D6) and an unused alias (D7).

Performance, measured on a 504-run / 41.7k-test dashboard built from the fixtures:

Path	Before	After
Filter modal closed without a change (P1)	507 ms rebuild	no rebuild
Settings modal closed without a change (P1)	721 ms rebuild	no rebuild
Overview grouping (P4)	57 ms	43 ms
Filter pass (P5, P6, P10)	49 ms	39 ms
Test statistics timeline (P3)	56 ms	48 ms

• Load time, full graph updates and page switches didn't change measurably: they are dominated by Chart.js drawing.
• P2: Chart.js graphs in hidden or collapsed sections wait until their canvas gets a size, so they aren't computed or drawn while hidden. On expand they are rebuilt with the current data. Visible offscreen graphs are still drawn right away, because the robot tests read them.
• P5: the run sort stays after the amount filter, because that filter depends on the stored order.
• P7: stat widgets compute each scope once per pass. P8: batched settings writes. P9: the overview nav only writes the DOM on a change.

Tests: JS 527 passed (4 more for these items). Robot (Docker) 144/144 after each of the two commits.

Security (S0-S5)

Robot Framework stores names, tags, metadata and messages as-is (output.xml only XML-escapes them), and Robot's own log.html escapes them at render time. The dashboard put them into innerHTML unescaped. A probe with a payload test file ran script from the suite/run name, run tag and project version (Overview) and from the test name, plain message and *HTML* message (Tables). In server mode anyone who can upload an output could run script for every viewer, including /admin.

• S0: one shared escape_html (also escapes ') in the new leaf module js/escape.js, used for every fix below. It replaces escape_html_for_merge from variables/globals.js.
• S1: DataTables cells get an escaping render function on the Tables page and in the admin run and log tables. A test message that starts with *HTML* is still rendered as HTML, without the marker, like in the Robot log.
• S2: the overview log link no longer builds an inline onclick from the log path. The path is in an escaped data-log-path attribute with one delegated click listener.
• S3: escaped or textContent for filter modal options (metadata, run names, run tags, versions, custom filters), overview project names, the settings keyword library list, profile names, server error text in alerts, stat/link widget titles and colours, and custom section titles.
• S4: link widgets allow only http:, https: and relative URLs (is_safe_url), both when adding a widget and when opening one loaded from settings or a JSON config.
• S5: the version filter selector uses CSS.escape.

Tests: new robot suite 11_security.robot runs a payload test file, builds a dashboard with a payload run tag and project version, opens every page and the filter modal, and checks that only the *HTML* message ran its payload. JS unit tests for escape_html and is_safe_url. JS 533 passed, robot (Docker) 145/145.

Code structure (C1-C8)

• C1: one parser per format: parse_custom_filters and get_tz_suffix live in common.js (the timezone regex was copied 6 times), and the custom graph engine builds on them and on parse_metadata_items.
• C2: clear_project_filter and update_filters_button_indicator move to filter/controls.js. controls.js used them without importing them, which hid a filter_modal → controls → filter_modal cycle. donut.js now imports previousFolder.
• C3: section_filters.js is table-driven (SECTION_GRAPH_UPDATES, setup_switch): 386 → 257 lines, one list per section instead of 5+ copies.
• C4: shared timeline run axis and tooltip (apply_timeline_run_axis, apply_timeline_defaults) and truncate_message. The compare tests timeline now also uses timelineScale, so its run labels are centred like on the other timelines.
• C5: aggregate_by_folder and assign_fail_colors replace the duplicated folder donut code. Numeric suite names keep their order.
• C6: overview run cards sort on data-run-number/passed/failed/skipped instead of parsing their text.
• C7: the run tag filter and profiles read the checkbox value instead of the id. Saved profiles store the same raw tag, so no migration. The ids stay runTagCheckBox<tag>: the robot keywords use them, and since S3 they are escaped.
• C8: for...of over arrays, add_alert instead of alert(), fetch instead of XHR, the skip/folder toggles read event.target.checked, a one-time graphs-finalized listener replaces a 500 ms timeout, and fast page switches drop stale setup_data_and_graphs runs.

Found on the way: send_request on the admin page assigned an undeclared response, the log file upload showed the output upload spinner, and three alert messages (dashboard refresh error, JSON config error, settings load error) were not escaped. A JSON parse error can quote its input. Also fixed the server auth note in CLAUDE.md and the server-api skill: the mutating endpoints and /refresh-dashboard need credentials too.

Tests: JS 541 passed (new tests for get_tz_suffix, truncate_message, aggregate_by_folder, assign_fail_colors). Robot (Docker) 145/145. Example dashboard clicked through in headless Chrome (pages, fast page switches, every section filter and switch, overview sort, filter modal): no errors.

Large timelines (performance) and comment cleanup

Measured on a 110-run database (1158 distinct tests, 26k test rows) with "All" selected in the test section:

Path	Before	After
Select all suites in the test section (animations on)	8.4-10.2 s	1.2-1.3 s
Test statistics timeline update	5.1 s	0.34 s
Suite statistics, "All Suites Separate" (3402 bars, no animation)	91 ms	34 ms

• Timelines: convert_timeline_data made one dataset per item and bar style (860 datasets for 8557 bars). Chart.js looks up the stack of every bar across all datasets, so the cost grew with bars × datasets. It now makes one dataset per bar style (status colour, rerun or note border): 5 datasets. Every point keeps its row in y, so tooltips, log clicks and notes are unchanged. The most time consuming tooltip reads raw.y instead of the dataset label, and "Only changes" / "No changes" work on the points. This covers every timeline graph; test statistics and compare tests gain the most.
• Bar rounding: barConfig.borderRadius is a function, so Chart.js resolved the options of every bar separately. chart_factory.js resolves it once per build (a rounding change rebuilds the graphs anyway). This helps every bar and timeline graph.
• Animation stays a user setting. The test duration line graph still draws one line per test (656 with all suites); that is inherent to the graph.
• Comments: comments added in this PR that only restated the code or described the old code are removed or trimmed to the current reason.

Tests: JS 542 passed (a new test for the rounding resolution, the convert_timeline_data tests updated to the new grouping). Robot (Docker) 145/145. Two test statistics checks read the timeline rows instead of the dataset labels, and the rerun border keyword counts bars instead of datasets. Four snapshots were regenerated (dashboardTestSection, dashboardSuiteSection, dashboardKeywordSection, compareSection). Every changed graph has the same labels and the same points as before, only in fewer datasets.

🤖 Generated with Claude Code