fix: render select tooltip as markdown instead of raw HTML - #497
Closed
dungdong-aws wants to merge 1 commit into
Closed
dungdong-aws wants to merge 1 commit into
dungdong-aws wants to merge 1 commit into
Conversation
The select tooltip was authored as raw HTML:
`<strong>${label}</strong><br>${description}`
That string is passed to CardBody, which renders its body through the
markdown parser. The parser's `html` renderer deliberately escapes raw
HTML as XSS hardening (src/helper/marked.ts), so the tags reached the
user as literal text instead of a bold label and a description.
Author the tooltip as markdown instead. The parser is already configured
with `includeLineBreaks`, so a newline becomes a line break.
Also adds a `Tooltip content` suite to select.spec.ts, which previously
had no tooltip coverage at all. The tests call the real configureMarked()
so the HTML-escaping renderer is active -- without it a test would pass
against the buggy code too.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Hovering the model selector in Amazon Q chat showed literal markup instead of
formatted text — e.g.
<strong>claude-opus-4.6</strong><br>— rather than a boldmodel name with its description underneath.
This affects every IDE host (VS Code, JetBrains, Eclipse, Visual Studio),
because they all load the same bundled chat UI. It is not host-side plugin code.
Root cause
Two parts of mynah-ui disagreed about the tooltip's content format:
The tooltip was authored as raw HTML in
src/components/form-items/select.ts:and passed to
CardBody({ body: content }).CardBodyrenders its body through the markdown parser(
src/components/card/card-body.ts,parseMarkdown(...)).That parser deliberately escapes raw HTML as XSS hardening —
src/helper/marked.ts:html: ({ text }) => escapeHTML(text).So
<strong>became<strong>and was displayed verbatim. The existingmarked.spec.tsalready encodes this distinction: markdown**bold**renders as<strong>, while a raw<strong>input is escaped.Fix
Author the tooltip as markdown. The parser is already configured with
includeLineBreaks: true, so\nbecomes a line break:Tests
select.spec.tspreviously had no tooltip coverage at all, which is why thisregression went unnoticed. This PR adds a
Tooltip contentsuite:<strong>element existsand that no literal
<strong>/<br>appears in the texttooltipprop when the option has no description, andrenders no bold element in that path
Two details make these tests meaningful rather than vacuous:
configureMarked(), so the HTML-escaping renderer isactive. Without it the tests would pass against the buggy code too.
Overlay(whose real responsibility is positioning, which is noiseunder jsdom) to capture the content that would have been displayed.
Relationship to #489
#489 proposes the same one-line change. This PR is equivalent on
select.ts(byte-identical code line) and covers the same three test cases,with one extra assertion in the fallback case.
The difference: #489 also modifies
ui-tests/__test__/flows/quick-action-commands-header.ts— aBoolean()coercion in a Playwright helper for the quick-picks header, unrelated to the
tooltip (its own commit is labelled
chore:). This PR leaves that out to keepthe change scoped to the bug.
Happy to close this in favour of #489 if the maintainers prefer — the goal is the
fix landing, not this particular PR.
Verification
my environment, so CI is the first execution of these tests. Will follow up
with results and before/after screenshots.
Follow-up considered and left out
Callers currently send
description: model.description ?? ''. An empty string isnot
null, so the tooltip still fires with an empty second line. Guarding it:That is a behavioural change rather than a rendering fix, so it is
deliberately out of scope here (#489 omits it too). Happy to add it if
maintainers want it in the same change.
Internal tracking: P447078389