docs: upgrade vendored DataTables 1.11.5 -> 2.3.8 - #2173
Conversation
Swap the vendored single-component DataTables downloader build from 1.11.5
to 2.3.8 (docs/_static/js/datatables.js, docs/_static/css/datatables.css).
The only init site, docs/_static/js/tables.js, uses the option-less
jQuery-style $('#id').DataTable() form, which is unchanged in 2.x, so
neither it nor docs/_templates/layout.html needed edits. jQuery 3.6.0 is
still injected ahead of the bundle by sphinxcontrib-jquery and satisfies
2.x.
Remove docs/_static/css/DataTables-1.11.5/ (10 sort-icon images). 2.x draws
sort indicators in pure CSS: the new datatables.css contains no url()
references at all, and the replaced 1.11.5 stylesheet was the only thing in
the repo that referenced those files.
Migrate custom selectors for the 2.x generated-class renames, each verified
against the downloaded bundle rather than assumed:
- .dataTables_info -> .dt-info (tables.css, text.css)
- .paginate_button -> .dt-paging-button (tables.css, text.css)
- #coverage a.paginate_button{,.current,.next,.previous}
-> #coverage button.dt-paging-button{...}
2.x renders paging controls as <button> (was <a> in 1.x), so the old
rules failed on both element type and class. The .current/.next/
.previous modifiers are still emitted and are retained.
The #table-cfgcmd_wrapper / #table-opcmd_wrapper rules are deliberately
untouched: only the wrapper *class* changed (dataTables_wrapper ->
dt-container); the element id is still <tableId>_wrapper.
Drop the CodeQL exclusion (.github/codeql/codeql-config.yml, plus the
optional codeql-cfg-path input in .github/workflows/codeql.yml). That
config existed solely to paths-ignore the 1.11.5 bundle, which tripped 7
alerts (5x js/incomplete-multi-character-sanitization, 2x
js/incomplete-sanitization). Upstream 2.x hardened the implicated helpers -
_stripHtml now runs a do/while fixpoint over the <script> strip, and
_escapeHtml uses all-global regexes - so those patterns are not expected to
recur. Removing the exclusion restores default CodeQL coverage of the file;
this PR's own CodeQL run is the empirical test.
Provenance (DataTables downloader builder, styling "DataTables default",
component set dt only - the 2.x equivalent of the previous #dt/dt-1.11.5):
https://cdn.datatables.net/v/dt/dt-2.3.8/datatables.js
sha256 184fb4bd0b9a81a955acd608ba94d0643c74271e78c0fae30f1f40b824f88b1d
https://cdn.datatables.net/v/dt/dt-2.3.8/datatables.css
sha256 e37677437e0fbe4a463aafc83bc4ea8d60986b72050ba9f47f740b41148184ae
Verified with a local make html: build succeeded, and coverage.html renders
both tables (table-cfgcmd 9095 rows, table-opcmd 3036 rows) with the 2.3.8
assets injected in the correct order after jQuery.
🤖 Generated by [robots](https://vyos.io)
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (7)
🧰 Additional context used🔍 Remote MCP vyos.devAdditional review context
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe pull request removes the custom CodeQL workflow configuration input and updates DataTables CSS to version 2.3.8, including theme variables, responsive and RTL layouts, dark mode, pagination, typography, and renamed control selectors. ChangesCodeQL workflow configuration
DataTables styling modernization
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches✨ Simplify code
Comment |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
docs/_static/css/datatables.css (1)
703-747: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExclude vendored DataTables CSS from Stylelint instead of patching it.
The 18
declaration-block-no-shorthand-property-overrideserrors are upstream's deliberate gradient fallback pattern (background-colorthen vendor-prefixedbackground). Editing them here would be lost on the next bundle regeneration; add the vendored asset to.stylelintignore(or a targetedoverridesentry) so lint stays green without diverging from upstream.🧹 Suggested `.stylelintignore` entry
+docs/_static/js/datatables.js +docs/_static/css/datatables.css🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/_static/css/datatables.css` around lines 703 - 747, Add the vendored DataTables CSS asset represented by the selectors div.dt-container and dt-paging-button to .stylelintignore, or configure an equally targeted Stylelint override, so its upstream gradient fallback declarations are excluded from declaration-block-no-shorthand-property-overrides without modifying the CSS.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/_static/css/tables.css`:
- Around line 55-58: Update the hover rule for the `#coverage` paging-button
selectors so background: none no longer overrides the intended hover
background-color or DataTables 2.x hover styling. Reorder or remove the
conflicting shorthand while preserving the existing hover fill and selector
coverage.
In `@docs/_static/js/datatables.js`:
- Around line 5228-5237: Update the focus handler for header cells to assign
scrollLeft to scrollFoot when footer exists; keep the initial scrollBody
synchronization unchanged.
- Around line 13382-13391: Update the cell.addClass expression in the isOrdering
branch to parenthesize each conditional ordering-class result before
concatenating them, ensuring asc and desc classes are independently evaluated
and unsorted or mixed-direction colspan headers receive the correct classes.
- Around line 4929-4932: Update the footer handling near settings.tfoot to use
the v2 footer reference settings.nTFoot, and update the related scrolling-table
selectors near the affected scroll logic from div.dataTables_scroll* to the
dt-scroll* selectors used by the bundle. Preserve empty-footer removal and
caption attachment behavior with the v2 selectors.
---
Nitpick comments:
In `@docs/_static/css/datatables.css`:
- Around line 703-747: Add the vendored DataTables CSS asset represented by the
selectors div.dt-container and dt-paging-button to .stylelintignore, or
configure an equally targeted Stylelint override, so its upstream gradient
fallback declarations are excluded from
declaration-block-no-shorthand-property-overrides without modifying the CSS.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 8520f0e1-eaec-4827-a760-f2daa3f6939f
⛔ Files ignored due to path filters (5)
docs/_static/css/DataTables-1.11.5/images/sort_asc.pngis excluded by!**/*.pngdocs/_static/css/DataTables-1.11.5/images/sort_asc_disabled.pngis excluded by!**/*.pngdocs/_static/css/DataTables-1.11.5/images/sort_both.pngis excluded by!**/*.pngdocs/_static/css/DataTables-1.11.5/images/sort_desc.pngis excluded by!**/*.pngdocs/_static/css/DataTables-1.11.5/images/sort_desc_disabled.pngis excluded by!**/*.png
📒 Files selected for processing (11)
.github/codeql/codeql-config.yml.github/workflows/codeql.ymldocs/_static/css/DataTables-1.11.5/images/sort_asc.webpdocs/_static/css/DataTables-1.11.5/images/sort_asc_disabled.webpdocs/_static/css/DataTables-1.11.5/images/sort_both.webpdocs/_static/css/DataTables-1.11.5/images/sort_desc.webpdocs/_static/css/DataTables-1.11.5/images/sort_desc_disabled.webpdocs/_static/css/datatables.cssdocs/_static/css/tables.cssdocs/_static/css/text.cssdocs/_static/js/datatables.js
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
ansible/ansible(manual)
💤 Files with no reviewable changes (2)
- .github/codeql/codeql-config.yml
- .github/workflows/codeql.yml
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Mergify Merge Protections
- GitHub Check: Summary
🧰 Additional context used
🪛 ast-grep (0.44.1)
docs/_static/js/datatables.js
[warning] 1122-1122: Detects non-literal values in regular expressions
Context: new RegExp( '(\' + [ '/', '.', '*', '+', '?', '|', '(', ')', '[', ']', '{', '}', '\', '$', '^', '-' ].join('|\') + ')', 'g' )
Note: [CWE-1333] Inefficient Regular Expression Complexity (ReDoS via non-literal RegExp).
(detect-non-literal-regexp)
[warning] 1334-1338: Avoid hand-rolled HTML escaping (replacing characters with HTML entities); use a vetted encoder/sanitizer such as DOMPurify or sanitize-html.
Context: d
.replace(/&/g, '&')
.replace(/</g, '<')
.replace(/>/g, '>')
.replace(/"/g, '"')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(manual-sanitization)
[warning] 1334-1337: Avoid hand-rolled HTML escaping (replacing characters with HTML entities); use a vetted encoder/sanitizer such as DOMPurify or sanitize-html.
Context: d
.replace(/&/g, '&')
.replace(/</g, '<')
.replace(/>/g, '>')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(manual-sanitization)
[warning] 1334-1336: Avoid hand-rolled HTML escaping (replacing characters with HTML entities); use a vetted encoder/sanitizer such as DOMPurify or sanitize-html.
Context: d
.replace(/&/g, '&')
.replace(/</g, '<')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(manual-sanitization)
[warning] 1334-1335: Avoid hand-rolled HTML escaping (replacing characters with HTML entities); use a vetted encoder/sanitizer such as DOMPurify or sanitize-html.
Context: d
.replace(/&/g, '&')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(manual-sanitization)
[error] 1503-1505: React's useState should not be directly called
Context: setTimeout( function () {
fn.apply(that, args);
}, timeout || 250 )
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[warning] 2831-2831: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: td.innerHTML = val
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
[warning] 4080-4080: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: cell.innerHTML = columnDef.sTitle
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
[warning] 4092-4092: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: cell.innerHTML = columnDef.footer
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
[warning] 4673-4676: Detects non-literal values in regular expressions
Context: new RegExp(
'^'+_fnEscapeRegex(search)+'$',
options.caseInsensitive ? 'i' : ''
)
Note: [CWE-1333] Inefficient Regular Expression Complexity (ReDoS via non-literal RegExp).
(detect-non-literal-regexp)
[warning] 4739-4739: Detects non-literal values in regular expressions
Context: new RegExp( search, options.caseInsensitive ? 'i' : '' )
Note: [CWE-1333] Inefficient Regular Expression Complexity (ReDoS via non-literal RegExp).
(detect-non-literal-regexp)
[error] 4834-4834: React's useState should not be directly called
Context: setTimeout( function(){ _fnInitialise( settings ); }, 200 )
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 5095-5099: React's useState should not be directly called
Context: setTimeout(function () {
run();
_fnProcessingDisplay(settings, false);
}, 0)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[warning] 12982-12982: Avoid using the initial state variable in setState
Context: setProp('className', val)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[warning] 12985-12985: Avoid using the initial state variable in setState
Context: setDetect(val)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[warning] 12988-12988: Avoid using the initial state variable in setState
Context: setOrder(val)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[warning] 12991-12991: Avoid using the initial state variable in setState
Context: setProp('render', val)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[warning] 12994-12994: Avoid using the initial state variable in setState
Context: setProp('search', val)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[error] 12982-12982: React's useState should not be directly called
Context: setProp('className', val)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 12985-12985: React's useState should not be directly called
Context: setDetect(val)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 12988-12988: React's useState should not be directly called
Context: setOrder(val)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 12991-12991: React's useState should not be directly called
Context: setProp('render', val)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 12994-12994: React's useState should not be directly called
Context: setProp('search', val)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 12998-12998: React's useState should not be directly called
Context: setProp('className', val.className)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 13002-13002: React's useState should not be directly called
Context: setDetect(val.detect)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 13006-13006: React's useState should not be directly called
Context: setOrder(val.order)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 13010-13010: React's useState should not be directly called
Context: setProp('render', val.render)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 13014-13014: React's useState should not be directly called
Context: setProp('search', val.search)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[error] 13711-13713: React's useState should not be directly called
Context: setTimeout( function () {
searchFn.call(jqFilter[0], e);
}, 10)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
🪛 Stylelint (17.14.0)
docs/_static/css/tables.css
[error] 219-219: Expected no quotes around "Roboto" (font-family-name-quotes)
(font-family-name-quotes)
docs/_static/css/datatables.css
[error] 712-712: Overridden property "background-color" by shorthand "background" (declaration-block-no-shorthand-property-overrides)
(declaration-block-no-shorthand-property-overrides)
[error] 713-713: Overridden property "background-color" by shorthand "background" (declaration-block-no-shorthand-property-overrides)
(declaration-block-no-shorthand-property-overrides)
[error] 714-714: Overridden property "background-color" by shorthand "background" (declaration-block-no-shorthand-property-overrides)
(declaration-block-no-shorthand-property-overrides)
[error] 715-715: Overridden property "background-color" by shorthand "background" (declaration-block-no-shorthand-property-overrides)
(declaration-block-no-shorthand-property-overrides)
[error] 716-716: Overridden property "background-color" by shorthand "background" (declaration-block-no-shorthand-property-overrides)
(declaration-block-no-shorthand-property-overrides)
[error] 717-717: Overridden property "background-color" by shorthand "background" (declaration-block-no-shorthand-property-overrides)
(declaration-block-no-shorthand-property-overrides)
[error] 730-730: Overridden property "background-color" by shorthand "background" (declaration-block-no-shorthand-property-overrides)
(declaration-block-no-shorthand-property-overrides)
[error] 731-731: Overridden property "background-color" by shorthand "background" (declaration-block-no-shorthand-property-overrides)
(declaration-block-no-shorthand-property-overrides)
[error] 732-732: Overridden property "background-color" by shorthand "background" (declaration-block-no-shorthand-property-overrides)
(declaration-block-no-shorthand-property-overrides)
[error] 733-733: Overridden property "background-color" by shorthand "background" (declaration-block-no-shorthand-property-overrides)
(declaration-block-no-shorthand-property-overrides)
[error] 734-734: Overridden property "background-color" by shorthand "background" (declaration-block-no-shorthand-property-overrides)
(declaration-block-no-shorthand-property-overrides)
[error] 735-735: Overridden property "background-color" by shorthand "background" (declaration-block-no-shorthand-property-overrides)
(declaration-block-no-shorthand-property-overrides)
[error] 740-740: Overridden property "background-color" by shorthand "background" (declaration-block-no-shorthand-property-overrides)
(declaration-block-no-shorthand-property-overrides)
[error] 741-741: Overridden property "background-color" by shorthand "background" (declaration-block-no-shorthand-property-overrides)
(declaration-block-no-shorthand-property-overrides)
[error] 742-742: Overridden property "background-color" by shorthand "background" (declaration-block-no-shorthand-property-overrides)
(declaration-block-no-shorthand-property-overrides)
[error] 743-743: Overridden property "background-color" by shorthand "background" (declaration-block-no-shorthand-property-overrides)
(declaration-block-no-shorthand-property-overrides)
[error] 744-744: Overridden property "background-color" by shorthand "background" (declaration-block-no-shorthand-property-overrides)
(declaration-block-no-shorthand-property-overrides)
[error] 745-745: Overridden property "background-color" by shorthand "background" (declaration-block-no-shorthand-property-overrides)
(declaration-block-no-shorthand-property-overrides)
🔍 Remote MCP vyos.dev
Additional review context
- T6315 — “Add Codeql reusable action workflow” is Resolved and requested the reusable CodeQL workflow in
vyos-github-actions; this supports the PR’s migration to the shared workflow. - T6491 — “codeql workflow update py version” is Resolved and specifically requested Python 3.11 for the CodeQL workflow. The PR should ensure the shared workflow still satisfies this requirement.
- T8144 — “Update workflows for pull_request_target policy updates” is Resolved and explicitly included
.github/workflows/codeql.ymlin its workflow-check list. - T9082 — “CodeQL security scanning rollout — vyos org fleet” is Resolved. Its verification recorded
vyos-documentationwith both workflow and analyses present, and the latest CodeQL run succeeded on 2026-07-19 11:57 UTC. This provides evidence that removing the local CodeQL configuration input did not prevent scanning.
🔇 Additional comments (7)
docs/_static/css/datatables.css (5)
7-362: LGTM!Also applies to: 375-400, 427-434
451-694: LGTM!
748-783: LGTM!
785-842: LGTM!
844-866: LGTM!docs/_static/css/tables.css (1)
41-53: LGTM!Also applies to: 217-229
docs/_static/css/text.css (1)
21-22: 📐 Maintainability & Code QualityNo change needed
docs/_static/css/text.css:21-22uses DataTables 2.x selectors; the remainingdataTables_*/sorting_*hits are confined todocs/_static/css/datatables.cssanddocs/_static/js/datatables.jsin the vendored bundle.> Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
🧹 Nitpick comments (1)
docs/_static/css/datatables.css (1)
703-747: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExclude vendored DataTables CSS from Stylelint instead of patching it.
The 18
declaration-block-no-shorthand-property-overrideserrors are upstream's deliberate gradient fallback pattern (background-colorthen vendor-prefixedbackground). Editing them here would be lost on the next bundle regeneration; add the vendored asset to.stylelintignore(or a targetedoverridesentry) so lint stays green without diverging from upstream.🧹 Suggested `.stylelintignore` entry
+docs/_static/js/datatables.js +docs/_static/css/datatables.css🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/_static/css/datatables.css` around lines 703 - 747, Add the vendored DataTables CSS asset represented by the selectors div.dt-container and dt-paging-button to .stylelintignore, or configure an equally targeted Stylelint override, so its upstream gradient fallback declarations are excluded from declaration-block-no-shorthand-property-overrides without modifying the CSS.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/_static/css/tables.css`:
- Around line 55-58: Update the hover rule for the `#coverage` paging-button
selectors so background: none no longer overrides the intended hover
background-color or DataTables 2.x hover styling. Reorder or remove the
conflicting shorthand while preserving the existing hover fill and selector
coverage.
In `@docs/_static/js/datatables.js`:
- Around line 5228-5237: Update the focus handler for header cells to assign
scrollLeft to scrollFoot when footer exists; keep the initial scrollBody
synchronization unchanged.
- Around line 13382-13391: Update the cell.addClass expression in the isOrdering
branch to parenthesize each conditional ordering-class result before
concatenating them, ensuring asc and desc classes are independently evaluated
and unsorted or mixed-direction colspan headers receive the correct classes.
- Around line 4929-4932: Update the footer handling near settings.tfoot to use
the v2 footer reference settings.nTFoot, and update the related scrolling-table
selectors near the affected scroll logic from div.dataTables_scroll* to the
dt-scroll* selectors used by the bundle. Preserve empty-footer removal and
caption attachment behavior with the v2 selectors.
---
Nitpick comments:
In `@docs/_static/css/datatables.css`:
- Around line 703-747: Add the vendored DataTables CSS asset represented by the
selectors div.dt-container and dt-paging-button to .stylelintignore, or
configure an equally targeted Stylelint override, so its upstream gradient
fallback declarations are excluded from
declaration-block-no-shorthand-property-overrides without modifying the CSS.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 8520f0e1-eaec-4827-a760-f2daa3f6939f
⛔ Files ignored due to path filters (5)
docs/_static/css/DataTables-1.11.5/images/sort_asc.pngis excluded by!**/*.pngdocs/_static/css/DataTables-1.11.5/images/sort_asc_disabled.pngis excluded by!**/*.pngdocs/_static/css/DataTables-1.11.5/images/sort_both.pngis excluded by!**/*.pngdocs/_static/css/DataTables-1.11.5/images/sort_desc.pngis excluded by!**/*.pngdocs/_static/css/DataTables-1.11.5/images/sort_desc_disabled.pngis excluded by!**/*.png
📒 Files selected for processing (11)
.github/codeql/codeql-config.yml.github/workflows/codeql.ymldocs/_static/css/DataTables-1.11.5/images/sort_asc.webpdocs/_static/css/DataTables-1.11.5/images/sort_asc_disabled.webpdocs/_static/css/DataTables-1.11.5/images/sort_both.webpdocs/_static/css/DataTables-1.11.5/images/sort_desc.webpdocs/_static/css/DataTables-1.11.5/images/sort_desc_disabled.webpdocs/_static/css/datatables.cssdocs/_static/css/tables.cssdocs/_static/css/text.cssdocs/_static/js/datatables.js
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
ansible/ansible(manual)
💤 Files with no reviewable changes (2)
- .github/codeql/codeql-config.yml
- .github/workflows/codeql.yml
📜 Review details
🔇 Additional comments (7)
docs/_static/css/datatables.css (5)
7-362: LGTM!Also applies to: 375-400, 427-434
451-694: LGTM!
748-783: LGTM!
785-842: LGTM!
844-866: LGTM!docs/_static/css/tables.css (1)
41-53: LGTM!Also applies to: 217-229
docs/_static/css/text.css (1)
21-22: 📐 Maintainability & Code QualityNo change needed
docs/_static/css/text.css:21-22uses DataTables 2.x selectors; the remainingdataTables_*/sorting_*hits are confined todocs/_static/css/datatables.cssanddocs/_static/js/datatables.jsin the vendored bundle.> Likely an incorrect or invalid review comment.
🛑 Comments failed to post (3)
docs/_static/js/datatables.js (3)
4929-4932: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail asset="$(mktemp)" curl -fsSL https://cdn.datatables.net/2.3.8/js/dataTables.js > "$asset" sha256sum docs/_static/js/datatables.js "$asset" grep -nE 'settings\.(nTFoot|tfoot)|dataTables_scroll|dt-scroll' \ docs/_static/js/datatables.js "$asset"Repository: vyos/vyos-documentation
Length of output: 208
🏁 Script executed:
#!/bin/bash set -euo pipefail # Show the exact nearby code with line numbers sed -n '4918,4938p' docs/_static/js/datatables.js printf '\n---\n' sed -n '7690,7715p' docs/_static/js/datatables.js printf '\n---\n' # Locate the relevant identifiers in the file grep -nE 'settings\.(nTFoot|tfoot)|dataTables_scroll|dt-scroll' docs/_static/js/datatables.js || trueRepository: vyos/vyos-documentation
Length of output: 2065
🏁 Script executed (no clone):
#!/bin/bash set -euo pipefail # Fetch the upstream DataTables 2.3.8 bundle and compare the specific snippets. asset="$(mktemp)" python3 - <<'PY' > /dev/null import urllib.request, sys url='https://cdn.datatables.net/2.3.8/js/dataTables.js' data=urllib.request.urlopen(url, timeout=30).read() open(sys.argv[1], 'wb').write(data) PY "$asset" python3 - <<'PY' "$asset" docs/_static/js/datatables.js import sys, hashlib, re, pathlib up = pathlib.Path(sys.argv[1]).read_bytes() loc = pathlib.Path(sys.argv[2]).read_bytes() print("upstream_sha256", hashlib.sha256(up).hexdigest()) print("local_sha256 ", hashlib.sha256(loc).hexdigest()) for label, blob in [("upstream", up), ("local", loc)]: text = blob.decode('utf-8', errors='replace') print("\n##", label) for pat in [r'settings\.(nTFoot|tfoot)', r'dataTables_scroll', r'dt-scroll']: print(pat, len(re.findall(pat, text))) PYLength of output: 203
🏁 Script executed:
#!/bin/bash set -euo pipefail python3 - <<'PY' from pathlib import Path p = Path('docs/_static/js/datatables.js') text = p.read_text(encoding='utf-8', errors='replace') for needle in ['settings.tfoot', 'settings.nTFoot', 'dataTables_scroll', 'dt-scroll']: print(needle, text.count(needle)) PYRepository: vyos/vyos-documentation
Length of output: 228
🌐 Web query:
"DataTables 2.3.8 settings.tfoot dt-scroll dataTables_scroll caption"💡 Result:
In DataTables 2.3.8, handling of the tfoot element, scrolling (dt-scroll), and table captions involves specific internal behaviors designed to maintain table structure and accessibility [1][2]. TFOOT and Accessibility In DataTables 2.3.8, a specific update was included to improve accessibility: if an empty tfoot element is present, DataTables will now remove it after initialization [3][2]. This change addresses requirements where empty footer tags could trigger accessibility warnings [4]. If you are working with internal settings, note that references to the footer element in settings are typically stored in the nTFoot property (e.g., settings.nTFoot) [4]. Table Captions with Scrolling DataTables supports the HTML caption element [5][6]. When scrolling is enabled (using scrollX or scrollY), DataTables must internally split the table into separate header, body, and footer sections to manage the scrollable viewport [1]. Because of this, the caption cannot simply reside in the main table element; it must be attached to either the header or the footer section to ensure it remains positioned correctly relative to the scrolling content [1]. If a table does not have a footer, DataTables may create one to ensure the caption is attached to a stable section rather than the body, which would otherwise cause the caption to appear at the bottom of the scrollable area [1]. Scrolling Context When scrollY or scrollX are enabled, DataTables applies specific classes, such as dt-scroll, and generates a wrapper structure to control the display [7][8][9]. The CSS caption-side property controls whether the caption appears above or below the table [5][6]. If you need to manipulate the caption programmatically, you can use the caption API method, which allows you to get or set the text and specify the position (top or bottom) [6]. Top Results: [1] https://next.datatables.net/blog/2014/table-captions [5] https://datatables.net/ref/core/option/caption [3] DataTables/Dist-DataTables@2.3.7...2.3.8 [4] https://datatables.net/forums/discussion/81708/empty-footer-is-there-a-way-to-tell-data-tables-not-to-include [2] https://cdn.datatables.net/2.3.8/
Citations:
- 1: https://next.datatables.net/blog/2014/table-captions
- 2: https://cdn.datatables.net/2.3.8/
- 3: DataTables/Dist-DataTables@2.3.7...2.3.8
- 4: https://datatables.net/forums/discussion/81708/empty-footer-is-there-a-way-to-tell-data-tables-not-to-include
- 5: https://datatables.net/ref/core/option/caption
- 6: https://datatables.net/ref/core/api/caption()
- 7: https://datatables.net/reference/option/scrollY
- 8: https://datatables.net/examples/core/basic_init/scroll_y.html
- 9: https://datatables.net/ref/core/option/scrollX
🏁 Script executed (no clone):
#!/bin/bash set -euo pipefail python3 - <<'PY' import hashlib, pathlib, urllib.request local = pathlib.Path('docs/_static/js/datatables.js').read_bytes() upstream = urllib.request.urlopen('https://cdn.datatables.net/2.3.8/js/dataTables.js', timeout=30).read() print("local_sha256 ", hashlib.sha256(local).hexdigest()) print("upstream_sha256", hashlib.sha256(upstream).hexdigest()) print("same_bytes ", local == upstream) for label, blob in [("local", local), ("upstream", upstream)]: text = blob.decode('utf-8', errors='replace') print(f"\n## {label}") for needle in ['settings.tfoot', 'settings.nTFoot', 'dataTables_scroll', 'dt-scroll']: print(needle, text.count(needle)) PYLength of output: 633
🏁 Script executed:
#!/bin/bash set -euo pipefail # Locate the relevant file and inspect nearby context plus identifier usage. git ls-files | rg '^docs/_static/js/datatables\.js$|^docs/_static/js/' printf '\n---\n' sed -n '4918,4938p' docs/_static/js/datatables.js printf '\n---\n' sed -n '7694,7708p' docs/_static/js/datatables.js printf '\n---\n' grep -nE 'settings\.(tfoot|nTFoot)|dataTables_scroll|dt-scroll' docs/_static/js/datatables.js || true printf '\n---\n' # Compare against the upstream 2.3.8 bundle using only read-only checks. python3 - <<'PY' import hashlib, urllib.request, pathlib, sys local_path = pathlib.Path('docs/_static/js/datatables.js') if not local_path.exists(): print("local_missing") sys.exit(0) local = local_path.read_bytes() upstream = urllib.request.urlopen('https://cdn.datatables.net/2.3.8/js/dataTables.js', timeout=30).read() print("same_bytes", local == upstream) print("local_sha256", hashlib.sha256(local).hexdigest()) print("upstream_sha256", hashlib.sha256(upstream).hexdigest()) for label, blob in [("local", local), ("upstream", upstream)]: text = blob.decode('utf-8', errors='replace') print(f"\n## {label}") for needle in ['settings.tfoot', 'settings.nTFoot', 'dataTables_scroll', 'dt-scroll']: print(needle, text.count(needle)) PYRepository: vyos/vyos-documentation
Length of output: 4373
Use the v2 DataTables footer and scroll selectors
docs/_static/js/datatables.js:4929, 7701-7705still referencessettings.tfootanddiv.dataTables_scroll*. The bundle usessettings.nTFootanddt-scroll*, so empty footers won’t be removed and captions won’t attach to scrolling tables.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/_static/js/datatables.js` around lines 4929 - 4932, Update the footer handling near settings.tfoot to use the v2 footer reference settings.nTFoot, and update the related scrolling-table selectors near the affected scroll logic from div.dataTables_scroll* to the dt-scroll* selectors used by the bundle. Preserve empty-footer removal and caption attachment behavior with the v2 selectors.
5228-5237: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Show the relevant region with line numbers sed -n '5195,5265p' docs/_static/js/datatables.js # Find references to the footer scroll element and related focus handlers rg -n "scrollFoot|scrollBody\\.scrollLeft|header cells|footer" docs/_static/js/datatables.jsRepository: vyos/vyos-documentation
Length of output: 5893
🏁 Script executed:
#!/bin/bash set -euo pipefail # Map the file structure around the relevant symbols first ast-grep outline docs/_static/js/datatables.js --view expanded || true # Then inspect the focus handler and footer scroll wiring sed -n '5210,5255p' docs/_static/js/datatables.js # Show all occurrences of the footer scroll variable in this file rg -n "\bscrollFoot\b|\bscrollBody\.scrollLeft\b|\bscrollHead\.scrollLeft\b" docs/_static/js/datatables.jsRepository: vyos/vyos-documentation
Length of output: 1586
Scroll the footer on header focus.
docs/_static/js/datatables.js:5232-5235assignsscrollBody.scrollLeftin both branches, so a focused header never updatesscrollFootwhen a scrolling footer exists. Set thefooterbranch toscrollFoot.scrollLeft = scrollLeft.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/_static/js/datatables.js` around lines 5228 - 5237, Update the focus handler for header cells to assign scrollLeft to scrollFoot when footer exists; keep the initial scrollBody synchronization unchanged.
13382-13391: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Show the relevant region with line numbers sed -n '13370,13400p' docs/_static/js/datatables.js # Probe how the expression parses/behaves for representative inputs node <<'JS' function expr(asc, desc) { const orderClasses = { isAsc: 'ASC', isDesc: 'DESC' }; const orderDirs = { includes(x) { return x === 'asc' ? asc : desc; } }; return orderDirs.includes('asc') ? orderClasses.isAsc : '' + orderDirs.includes('desc') ? orderClasses.isDesc : ''; } for (const [asc, desc] of [ [false, false], [true, false], [false, true], [true, true], ]) { console.log({ asc, desc, result: expr(asc, desc) }); } JSRepository: vyos/vyos-documentation
Length of output: 1435
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the exact tokenization by evaluating both the original and parenthesized forms node <<'JS' function original(asc, desc) { const orderClasses = { isAsc: 'ASC', isDesc: 'DESC' }; const orderDirs = { includes: (x) => (x === 'asc' ? asc : desc) }; return orderDirs.includes('asc') ? orderClasses.isAsc : '' + orderDirs.includes('desc') ? orderClasses.isDesc : ''; } function proposed(asc, desc) { const orderClasses = { isAsc: 'ASC', isDesc: 'DESC' }; const orderDirs = { includes: (x) => (x === 'asc' ? asc : desc) }; return (orderDirs.includes('asc') ? orderClasses.isAsc : '') + ' ' + (orderDirs.includes('desc') ? orderClasses.isDesc : ''); } for (const asc of [false, true]) { for (const desc of [false, true]) { console.log({ asc, desc, original: original(asc, desc), proposed: proposed(asc, desc) }); } } JSRepository: vyos/vyos-documentation
Length of output: 413
Parenthesize the ordering class concatenation.
docs/_static/js/datatables.js:13388-13391— without parentheses, the expression is parsed asasc ? isAsc : ('' + desc) ? isDesc : '', so thedescbranch wins wheneverascis absent. Unsorted and mixed-direction colspan headers can get the wrong sort class.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/_static/js/datatables.js` around lines 13382 - 13391, Update the cell.addClass expression in the isOrdering branch to parenthesize each conditional ordering-class result before concatenating them, ensuring asc and desc classes are independently evaluated and unsorted or mixed-direction colspan headers receive the correct classes.
The hover rule carried 'background-color: #E1E4E5 !important' followed by 'background: none'. The important longhand outranks the later normal-importance shorthand, so the gray hover fill did apply — but the pair is confusing and CodeRabbit read it as a dead declaration. Collapse to a single 'background: #E1E4E5 !important', which produces the identical effective state (gray fill, upstream 2.x hover gradient suppressed) in one declaration. 🤖 Generated by [robots](https://vyos.io)
|
@coderabbitai — dispositions for the actionable items that failed to post as inline threads (reviews 4799356438 / 4799359423), plus the nitpick:
🤖 Addressed by Claude Code |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== Asset hashes =='
sha256sum docs/_static/js/datatables.js docs/_static/css/datatables.css
printf '%s\n' '== DataTables initialization and options =='
rg -n -i -C 3 \
'(DataTable\s*\(|\.dataTable\s*\(|scrollX\s*:|scrollY\s*:|scrollCollapse\s*:|tfoot|<tfoot|colspan)' \
docs \
-g '!docs/_static/js/datatables.js' \
-g '!docs/_static/css/datatables.css' || true
printf '%s\n' '== Stylelint configuration or workflow references =='
fd -HI '^\.stylelint(rc.*|ignore)$' . || true
rg -n -i -C 2 'stylelint' . \
-g '!docs/_static/js/datatables.js' \
-g '!docs/_static/css/datatables.css' || true
printf '%s\n' '== DataTables provenance/hash references =='
rg -n -i -C 2 '(datatables.{0,60}(2\.3\.8|sha-?256|cdn|download)|sha-?256.{0,100}datatables)' \
. \
-g '!docs/_static/js/datatables.js' \
-g '!docs/_static/css/datatables.css' || trueLength of output: 1308 🏁 Script executed: #!/bin/bash
set -u
printf '%s\n' '== Asset hashes =='
for file in docs/_static/js/datatables.js docs/_static/css/datatables.css; do
if command -v openssl >/dev/null 2>&1; then
openssl dgst -sha256 "$file"
elif command -v shasum >/dev/null 2>&1; then
shasum -a 256 "$file"
else
printf 'No supported SHA-256 utility available for %s\n' "$file"
fi
done
printf '%s\n' '== DataTables initialization and options =='
rg -n -i -C 3 \
'(DataTable\s*\(|\.dataTable\s*\(|scrollX\s*:|scrollY\s*:|scrollCollapse\s*:|<tfoot|colspan)' \
docs \
-g '!docs/_static/js/datatables.js' \
-g '!docs/_static/css/datatables.css' || true
printf '%s\n' '== Stylelint configuration or workflow references =='
fd -HI '^\.stylelint(rc.*|ignore)$' . 2>/dev/null || true
rg -n -i -C 2 'stylelint' . \
-g '!docs/_static/js/datatables.js' \
-g '!docs/_static/css/datatables.css' || true
printf '%s\n' '== DataTables provenance/hash references in tracked files =='
rg -n -i -C 2 '(datatables.{0,60}(2\.3\.8|sha-?256|cdn|download)|sha-?256.{0,100}datatables)' \
. \
-g '!docs/_static/js/datatables.js' \
-g '!docs/_static/css/datatables.css' || trueLength of output: 1875
I’m withdrawing the vendored DataTables and sandbox-only Stylelint findings. The asset hashes could not be independently recomputed in this sandbox because no SHA-256 utility is available. ✏️ Learnings added
|
DataTables 1.x nested the search <input> inside its <label>; 2.x renders them as siblings inside div.dt-search, so the narrow-viewport descendant rule 'label input' no longer matched and the 10px top margin was silently lost below 576px. Target '.dt-search input' instead. Flagged independently by the implementation pass and both adversarial reviewers (Codex + agy). 🤖 Generated by [robots](https://vyos.io)
Adversarial review —
|
1 participant
Change Summary
Upgrades the vendored DataTables bundle loaded on every docs page from 1.11.5 (March 2022) to 2.3.8 (April 2026), and restores CodeQL coverage of the file by removing the
paths-ignoreexclusion introduced in vyos-documentation#2171.What changed
docs/_static/js/datatables.js+docs/_static/css/datatables.cssswapped for the 2.3.8 downloader-builder build — same single-component set (#dt/dt-2.3.8, the 2.x equivalent of the previous#dt/dt-1.11.5), same filenames, sodocs/_templates/layout.htmlis untouched. Neither file had ever been locally modified (git log --follow: one commit each, dfc9ceb).docs/_static/js/tables.js(option-less$('#table-cfgcmd').DataTable()/$('#table-opcmd').DataTable()), is fully compatible with 2.x jQuery-style init — unchanged. jQuery 3.6.0 is still injected ahead of the bundle bysphinxcontrib-jquery(sphinx-rtd-theme dep) and satisfies 2.x.docs/_static/css/DataTables-1.11.5/(10 sort-icon images). DataTables 2.x draws sort indicators in pure CSS — the new stylesheet contains zerourl()references, and the replaced 1.11.5 stylesheet was the only thing in the repo referencing those files..dataTables_info→.dt-info(tables.css, text.css),.paginate_button→.dt-paging-button(tables.css, text.css),#coverage a.paginate_button…→#coverage button.dt-paging-button…(2.x renders paging controls as<button>, not<a>). The#table-*_wrapperid-based rules are deliberately untouched — only the wrapper class changed in 2.x, the id is still<tableId>_wrapper. While migrating the paging-button rule, a pre-existing invalidbackground-color: none;declaration (always ignored by browsers; the adjacentbackground: none;is the effective reset) was dropped from it (Phase-0 CodeRabbit finding).CodeQL exclusion removed
The 1.11.5 bundle tripped 7 code-scanning alerts (5×
js/incomplete-multi-character-sanitization, 2×js/incomplete-sanitization), which vyos-documentation#2171 suppressed via.github/codeql/codeql-config.ymlpaths-ignorerather than hand-patching vendored code — accepting that the exclusion also suppressed all future queries on the file. Upstream 2.x hardened exactly the implicated helpers:_stripHtmlnow runs a do/while fixpoint over the<scriptstrip (with an explicit CodeQL-workaround comment in upstream source), and_escapeHtmluses all-global regexes. This PR therefore deletes the config file and the optionalcodeql-cfg-pathinput from.github/workflows/codeql.yml, restoring default CodeQL coverage of the file.This PR's own CodeQL run is the empirical test: the workflow's
pull_requesttrigger paths cover the js/css swap, and the run analyzes the PR merge ref with the exclusion gone. If alerts recur on the new bundle, the exclusion comes back in a follow-up commit with a note; expectation is zero.Provenance (DataTables downloader builder, styling "DataTables default", component set
dtonly):datatables.js184fb4bd0b9a81a955acd608ba94d0643c74271e78c0fae30f1f40b824f88b1ddatatables.csse37677437e0fbe4a463aafc83bc4ea8d60986b72050ba9f47f740b41148184aeIn-tree files are byte-identical to the CDN artifacts (re-download + sha256 compare).
Verification
make html(Sphinx 7.2.6, Python 3.12 venv fromrequirements.txt): build succeeded;coverage.htmlrenders both tables (table-cfgcmd9,095 rows,table-opcmd3,036 rows) with the 2.3.8 assets injected in correct order after jQuery.dt-ordering-*classes applied, rows reorder); search filter works (9,094 → 568 entries for "interfaces ethernet"); paging works; zero console errors. Migrated custom styles confirmed binding to the 2.x-generated elements via computed styles (info color/font, paging-button font/radius, current-page highlight).Related Task(s)
Related: IS-631
(IS-631 is the CodeQL-remediation change ticket that introduced the exclusion this PR removes.)
Related PR(s)
paths-ignoreexclusion removed hereBackport
circinusandsagittacarry the byte-identical 1.11.5 bundle (same git blob) and are backport candidates. Note for backporting: the.github/codeql/*changes in this PR are rolling-only (neither the exclusion config nor thecodeql-cfg-pathinput exists on the LTS branches), so a Mergify backport will conflict on those hunks and needs them dropped during resolution.Checklist:
🤖 Generated by robots