Skip to content

fix(ui): show the line breaks of data-html popovers as text - #16395

Open
cristianchiriac wants to merge 1 commit into
DefectDojo:devfrom
cristianchiriac:fix-html-popover-text
Open

cristianchiriac wants to merge 1 commit into
DefectDojo:devfrom
cristianchiriac:fix-html-popover-text

Conversation

@cristianchiriac

Copy link
Copy Markdown
Contributor

Description

Fixes #16393.

Since the classic Bootstrap UI was removed (#15565), .has-popover elements are shown by two things: the CSS ::after tooltip in components/tailwind.css (content: attr(data-content)), and the $.fn.popover shim in dojo/static/dojo/js/index.js (pop.textContent = content, used by the hover handlers in view_test.html and findings_list_snippet.html). Both show data-content as plain text. The popovers that were written for Bootstrap with data-html="true" therefore show their markup. That's the screenshot in the issue: the endpoint list of a finding renders as ✓ host <br/> ✕ host <br/> ..., once in the dark CSS tooltip and once in the white shim box.

There are 10 such popovers in 4 templates: endpoints/locations in findings_list_snippet.html and view_test.html, finding group titles in view_test.html, CVSS vectors in view_finding.html, and the engagement and GitHub summaries in product.html.

This keeps the templates as they are:

  • index.js: on DOMContentLoaded, for .has-popover[data-html="true"], split data-content on <br>, drop <b>/</b>, collapse each line's whitespace and join the lines with \n. The content is never parsed as HTML. Anything else, such as markup inside an endpoint or a finding title, stays literal text, as it does in every other popover. This runs before the existing accessibility block, so the aria-label copied from data-content is the cleaned text too.
  • The popover shim uses white-space: pre for those elements (it stays nowrap for the others).
  • components/tailwind.css: a rule next to the existing ::after tooltip gives data-html="true" popovers white-space: pre-line. tailwind-out.css is rebuilt with npx @tailwindcss/cli -i tailwind.css -o ../dojo/static/dojo/css/tailwind-out.css --minify (4.3.3 from yarn.lock). On unchanged dev that command reproduces the committed file exactly, and the only change it makes is the new rule.

Test results

There are no JS tests for index.js, so I checked it in a browser (Chromium) with a page that loads jQuery, tailwind-out.css and index.js from dev and from this branch, with markup copied from the templates: an endpoint popover whose last endpoint is <img src=x onerror="document.title='XSS'"> (escaped in the attribute, as Django renders it), the engagement popover from product.html, and a plain file-path popover. I focused each icon to trigger :focus::after, then called .popover('show'):

  • dev: data-content, the ::after content and the shim text all contain the raw <br/>/<b> with the template indentation.
  • this branch: data-content is ✓ https://app.example.com/login\n✕ vdi-pw11-30\n✕ <img src=x onerror="document.title='XSS'">, the ::after computed white-space is pre-line, and the shim box has the same text with white-space: pre, no child elements, no image loaded and the document title unchanged. The engagement popover reads 3 Active, 1 Closed\nLast Assessed: Oct. 10, 2026, and the plain popover is unchanged (white-space: normal / nowrap).

Documentation

No user-facing documentation change.

Since the classic Bootstrap UI was removed, .has-popover elements are
shown by the CSS ::after tooltip and the popover shim in index.js, and
both display data-content as plain text. Popovers written for Bootstrap
with data-html="true" (the endpoint/location list of a finding, the
finding group titles, CVSS vectors, engagement and GitHub summaries)
showed their raw <br/> and <b> tags.

On page load, turn the <br> of these popovers into newlines, drop their
<b> tags, and show them with white-space: pre-line (tooltip) or pre
(shim). Everything else, such as markup in an endpoint or a title, stays
literal text.

Fixes DefectDojo#16393

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant