Some HTML-formatted text is completely removed from JSDoc Intellisense

[thw0rted]

• VSCode Version: 1.51.1
• OS Version: Win10

Steps to Reproduce:

1. Annotate something (in TS or JS) with a JSDoc comment, using <pre> or <code> tags for formatting
2. The content of the tag is erased in the Intellisense description of the thing

Does this issue occur when all extensions are disabled?: Yes

---

See this Playground example, where A basic interface with <code>x</code> and <pre>y</pre> renders as A basic interface with and ..

I'm aware that per #31161 the official position is that HTML markup is "not supported", so by all means strip out tags, but completely removing text content is still a bug. We can't control how third-party code writes their documentation, so the language service needs to handle it in a sane way.

[trivikr]

The <pre> and <code> blocks are allowed under CommonMark spec 0.29 4.5 Fenced code blocks.

[thw0rted]

Per the other issue I linked in the OP, they "support Markdown without HTML tag support". That's... a position to take, I guess -- if I had to write an IDE I'd certainly want to farm out Markdown parsing to one of the squillion existing libraries rather than write a partial implementation myself -- but at least they're clear about it. My concern is not with getting the tags supported, it's that stripping the tags should not also remove the content inside the tags.

[trivikr]

Copying description from related issue which was closed as duplicate.

• VSCode Version: 1.52.1
• OS Version: Darwin x64 19.6.0

Steps to Reproduce:

1. Open the example code in VSCode.
2. Hover over Foo. The Tooltip reads "".
3. Hover over Bar. The Tooltip reads "A basic interface with bar."

The doc comment for Foo is compliant under CommonMark spec 0.29 4.6 HTML Blocks kind 6

• Start condition: line begins the string < followed by p, followed by the string >
• End condition: line is followed by a blank line

Example Code

/**
 * <p>A basic interface with foo.</p>
 */
interface Foo {
  foo: number;
}

let f: Foo; // Tooltip reads ""

/**
 * A basic interface with bar.
 */
interface Bar {
  bar: number;
}

let b: Bar; // Tooltip reads "A basic interface with bar."

Screenshots

Does this issue occur when all extensions are disabled?: Yes

[trivikr]

Some findings which could be helpful:

• The markdownRenderer uses marked from vs/base/common/marked/marked:

  vscode/src/vs/base/browser/markdownRenderer.ts

  Line 11 in 6260e65

  import * as marked from 'vs/base/common/marked/marked';

• The renderer is markdown parser from https://github.com/markedjs/marked

  vscode/src/vs/base/common/marked/marked.js

  Lines 1 to 5 in 6260e65

  	/**
  	* marked - a markdown parser
  	* Copyright (c) 2011-2020, Christopher Jeffrey. (MIT Licensed)
  	* https://github.com/markedjs/marked
  	*/

The Marked Demo returns data as per CommonMark spec 0.29 4.6 HTML Blocks kind 6 for provided example.

Screenshot

The example is also given in README under CLI usage https://github.com/markedjs/marked#usage
The issue seems to be in VSCode repo.

[thw0rted]

Do you think the problem is here? I just searched for "strip" on a hunch, and found that any time marked hits an HTML tag in that case, it's completely ignored.

[mjbvz]

As discussed in #31161, we strip html while rendering JSDocs. Here's the main place where this happens:

vscode/src/vs/base/browser/markdownRenderer.ts

Line 220 in 6260e65

markedOptions.sanitizer = (html: string): string => {

We need more community feedback before enabling a subset of safe html tags in our markdown strings

[thw0rted]

My suggestion earlier was that instead of whitelisting tags to let through, you could remove all markup and include only the text node(s) as bare strings. Of course for elements that change plain text layout (block tags like headings, paragraph, lists, etc) the result would look pretty ugly, but for inline tags it would be an improvement over current behavior without possibility of security ramifications. A more thoughtful whitelisting would be more useful, but also more work.

[trivikr]

@thw0rted please use inclusive word like "allowlist"

[mjbvz]

Duplicate of #40607

[thw0rted]

@mjbvz I don't actually think this is a duplicate of the linked issue. That issue wants (certain) HTML tags to be rendered into styled markup. It's a pretty big bite to take, has a lot of security implications etc, and it looks to me like the issue hasn't seen movement in 3+ years as a result.

That's not what I'm asking for. Today, right now, the text content of unsupported tags disappears completely, resulting in a useless mess for a lot of third party libraries that I do not control. It's a bug with a simple, uncontroversial fix: stop eating the text. I'm worried that closing this as a duplicate of a difficult, controversial issue will result in users being stuck with the current, wrong behavior until someone has the time and energy to make a bunch of security and design decisions.

[thw0rted]

Re-reading #40607, maybe @trivikr's issue is actually the same? There might actually be multiple separate but related disappearing-text issues. I didn't initially understand the older issue just from reading through it, and I think it might only be the code and pre tags that have my specific problem. I've tried a bunch of different tags in different places, and here's what I figured out:

• code and pre contents always disappear, at any position in the docs. This is my problem.
• p contents only disappear (per @trivikr's issue) when they begin at the start of a line. The string /** Some text <p>other text</p> */ renders as Some text other text but /** <p>Some text</p> */ gives no tooltip. Experimentally, any block-level tag has this bug, and any inline tag (except code!) does not.

Re-reading #40607, the OP wants the second issue resolved -- and also wants <br/> to translate to a forced newline (akin to ending Markdown with a double space), I guess?.

So, there are really 3 related issues. code and pre are being mishandled regardless of position. All block-level tags are being mishandled if they begin a new line. And always-empty tags (br and hr) that should at least force a newline, don't. If it makes sense to only fix one of those when all 3 can be fixed, I guess it makes sense to merge them, but I was hoping that it might be simpler to fix the code/pre issue.