From a49ad2068136e4dfa2e7de4974699ff83939e063 Mon Sep 17 00:00:00 2001
From: Nikolay Vitkov <[email protected]>
Date: Wed, 23 Sep 2026 15:22:31 +0000
Subject: [PATCH] Migrate IssueView to MarkdownPlaceholderLitRenderer

- Render issue descriptions in IssueView via <devtools-markdown-view>
  using tokenizeWithPlaceholders and MarkdownPlaceholderLitRenderer.
- Remove string-level substitutePlaceholders from
  MarkdownIssueDescription.ts.
- Update IssueView and MarkdownView unit tests.

Bug: 513686739
Change-Id: I488154a683f31acf3b7b3f36f590d2d851952fc3
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/8445257
Reviewed-by: Alex Rudenko <[email protected]>
Auto-Submit: Nikolay Vitkov <[email protected]>
Commit-Queue: Alex Rudenko <[email protected]>
---
diff --git a/front_end/core/text_utils/Markdown.test.ts b/front_end/core/text_utils/Markdown.test.ts
new file mode 100644
index 0000000..41d8048
--- /dev/null
+++ b/front_end/core/text_utils/Markdown.test.ts
@@ -0,0 +1,50 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import {assert} from 'chai';
+
+import * as TextUtils from './text_utils.js';
+
+describe('Markdown', () => {
+  it('tokenizes {PLACEHOLDER_*} into placeholder AST tokens without substituting strings', () => {
+    const tokens = TextUtils.Markdown.tokenizeWithPlaceholders(
+        'Check {PLACEHOLDER_Link} here.',
+        new Map([['PLACEHOLDER_Link', '[Spoofed](https://example.com)']]),
+    );
+
+    const paragraph = tokens.find(t => t.type === 'paragraph');
+    assert.exists(paragraph);
+    const inlineTokens = 'tokens' in paragraph && paragraph.tokens ? paragraph.tokens : [];
+    const placeholderToken = inlineTokens.find(
+        (t): t is TextUtils.Markdown.PlaceholderToken => t.type === 'placeholder',
+    );
+    assert.exists(placeholderToken);
+    assert.strictEqual(placeholderToken.key, 'PLACEHOLDER_Link');
+    assert.strictEqual(placeholderToken.raw, '{PLACEHOLDER_Link}');
+    assert.isFalse(inlineTokens.some(t => t.type === 'link'));
+  });
+
+  it('validates invalid, missing, and unused placeholders when substitutions are provided', () => {
+    assert.throws(() => {
+      TextUtils.Markdown.tokenizeWithPlaceholders(
+          'Missing {PLACEHOLDER_PH1}',
+          new Map(),
+      );
+    }, /No replacement provided for placeholder 'PLACEHOLDER_PH1'/);
+
+    assert.throws(() => {
+      TextUtils.Markdown.tokenizeWithPlaceholders(
+          'Invalid key',
+          new Map([['invalid_ph', 'foo']]),
+      );
+    }, /Invalid placeholder 'invalid_ph' provided/);
+
+    assert.throws(() => {
+      TextUtils.Markdown.tokenizeWithPlaceholders(
+          'No placeholders used here',
+          new Map([['PLACEHOLDER_Unused', 'foo']]),
+      );
+    }, /Unused replacements provided: PLACEHOLDER_Unused/);
+  });
+});
diff --git a/front_end/core/text_utils/Markdown.ts b/front_end/core/text_utils/Markdown.ts
new file mode 100644
index 0000000..458c61ff
--- /dev/null
+++ b/front_end/core/text_utils/Markdown.ts
@@ -0,0 +1,73 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import * as Marked from '../../third_party/marked/marked.js';
+
+export interface PlaceholderToken extends Marked.Marked.Tokens.Generic {
+  type: 'placeholder';
+  raw: string;
+  key: string;
+}
+
+export const VALID_PLACEHOLDER_MATCH_PATTERN: RegExp = /\{(PLACEHOLDER_[a-zA-Z][a-zA-Z0-9_]*)\}/g;
+const VALID_PLACEHOLDER_NAME_PATTERN = /^PLACEHOLDER_[a-zA-Z][a-zA-Z0-9_]*$/;
+
+const placeholderExtension: Marked.Marked.TokenizerExtension = {
+  name: 'placeholder',
+  level: 'inline',
+  start(src: string): number |
+      undefined {
+        return src.match(/\{PLACEHOLDER_[a-zA-Z][a-zA-Z0-9_]*\}/)?.index;
+      },
+  tokenizer(src: string): PlaceholderToken |
+      undefined {
+        const match = /^\{(PLACEHOLDER_[a-zA-Z][a-zA-Z0-9_]*)\}/.exec(src);
+        if (match) {
+          return {
+            type: 'placeholder',
+            raw: match[0],
+            key: match[1],
+          };
+        }
+        return undefined;
+      },
+};
+
+export function validateSubstitutions(rawMarkdown: string, substitutions?: Map<string, string>): void {
+  if (!substitutions) {
+    return;
+  }
+  const unusedPlaceholders = new Set(substitutions.keys());
+  for (const key of unusedPlaceholders) {
+    if (!VALID_PLACEHOLDER_NAME_PATTERN.test(key)) {
+      throw new Error(`Invalid placeholder '${key}' provided in the substitutions map.`);
+    }
+  }
+  for (const [, placeholder] of rawMarkdown.matchAll(VALID_PLACEHOLDER_MATCH_PATTERN)) {
+    if (!substitutions.has(placeholder)) {
+      throw new Error(`No replacement provided for placeholder '${placeholder}'.`);
+    }
+    unusedPlaceholders.delete(placeholder);
+  }
+  if (unusedPlaceholders.size > 0) {
+    throw new Error(`Unused replacements provided: ${[...unusedPlaceholders]}`);
+  }
+}
+
+/**
+ * Tokenizes markdown into a Marked AST while preserving `{PLACEHOLDER_*}` tokens
+ * as `PlaceholderToken` AST nodes instead of substituting strings before lexing.
+ */
+export function tokenizeWithPlaceholders(markdown: string|Marked.Marked.Token[],
+                                         substitutions?: Map<string, string>): Marked.Marked.Token[] {
+  const rawMarkdown = typeof markdown === 'string' ? markdown : markdown.map(token => token.raw).join('');
+  validateSubstitutions(rawMarkdown, substitutions);
+  if (typeof markdown !== 'string' && !rawMarkdown.includes('{PLACEHOLDER_')) {
+    return markdown;
+  }
+  const markedInstance = new Marked.Marked.Marked({
+    extensions: [placeholderExtension],
+  });
+  return markedInstance.lexer(rawMarkdown);
+}
diff --git a/front_end/core/text_utils/text_utils.ts b/front_end/core/text_utils/text_utils.ts
index 2060b3b..a7bc7cb 100644
--- a/front_end/core/text_utils/text_utils.ts
+++ b/front_end/core/text_utils/text_utils.ts
@@ -5,6 +5,7 @@
 import * as CodeMirrorUtils from './CodeMirrorUtils.js';
 import * as ContentData from './ContentData.js';
 import * as ContentProvider from './ContentProvider.js';
+import * as Markdown from './Markdown.js';
 import * as StaticContentProvider from './StaticContentProvider.js';
 import * as StreamingContentData from './StreamingContentData.js';
 import * as Text from './Text.js';
@@ -17,6 +18,7 @@
   CodeMirrorUtils,
   ContentData,
   ContentProvider,
+  Markdown,
   StaticContentProvider,
   StreamingContentData,
   Text,
diff --git a/front_end/models/issues_manager/MarkdownIssueDescription.test.ts b/front_end/models/issues_manager/MarkdownIssueDescription.test.ts
index 0c25055..160db36 100644
--- a/front_end/models/issues_manager/MarkdownIssueDescription.test.ts
+++ b/front_end/models/issues_manager/MarkdownIssueDescription.test.ts
@@ -32,7 +32,6 @@
         () => IssuesManager.MarkdownIssueDescription.createIssueDescriptionFromRawMarkdown(
             invalidIssueDescription, emptyMarkdownDescription));
   });
-
   it('overrides the Markdown heading title with description.title while still validating that a heading exists', () => {
     const descriptionWithTitle = {
       file: '<unused>',
@@ -49,63 +48,40 @@
     assert.throws(() => IssuesManager.MarkdownIssueDescription.createIssueDescriptionFromRawMarkdown(
                       invalidIssueDescription, descriptionWithTitle));
   });
-});
 
-describe('substitutePlaceholders', () => {
-  it('returns the input as-is, with no placeholders present in the input', () => {
-    const str = 'Example string with no placeholders';
+  it('tokenizes placeholders into AST nodes without substituting untrusted strings before Markdown lexing', () => {
+    const rawMarkdown = '# Issue Title\n\nBody with {PLACEHOLDER_Untrusted}.';
+    const description = IssuesManager.MarkdownIssueDescription.createIssueDescriptionFromRawMarkdown(rawMarkdown, {
+      file: '<unused>',
+      links: [],
+      substitutions: new Map([
+        ['PLACEHOLDER_Untrusted', '[Spoofed Link](https://example.com)'],
+      ]),
+    });
 
-    assert.strictEqual(IssuesManager.MarkdownIssueDescription.substitutePlaceholders(str), str);
+    const paragraph = description.markdown.find(t => t.type === 'paragraph');
+    assert.exists(paragraph);
+    const tokens = 'tokens' in paragraph && paragraph.tokens ? paragraph.tokens : [];
+    assert.isTrue(tokens.some(t => t.type === 'placeholder'));
+    assert.isFalse(tokens.some(t => t.type === 'link'));
   });
 
-  it('subsitutes a single placeholder', () => {
-    const str = 'Example string with a single {PLACEHOLDER_placeholder}';
+  it('validates placeholder names and required replacements during description creation', () => {
+    assert.throws(() => {
+      IssuesManager.MarkdownIssueDescription.createIssueDescriptionFromRawMarkdown(
+          '# Title\n\nMissing {PLACEHOLDER_Missing}', {
+            file: '<unused>',
+            links: [],
+            substitutions: new Map(),
+          });
+    }, /No replacement provided for placeholder 'PLACEHOLDER_Missing'/);
 
-    const actual = IssuesManager.MarkdownIssueDescription.substitutePlaceholders(
-        str, new Map<string, string>([['PLACEHOLDER_placeholder', 'fooholder']]));
-    assert.strictEqual(actual, 'Example string with a single fooholder');
-  });
-
-  it('substitutes multiple placeholders', () => {
-    const str = 'Example string with two placeholders, \'{PLACEHOLDER_ph1}\' and \'{PLACEHOLDER_ph2}\'.';
-
-    const actual = IssuesManager.MarkdownIssueDescription.substitutePlaceholders(
-        str, new Map<string, string>([['PLACEHOLDER_ph1', 'foo'], ['PLACEHOLDER_ph2', 'bar']]));
-    assert.strictEqual(actual, 'Example string with two placeholders, \'foo\' and \'bar\'.');
-  });
-
-  it('throws an error for placeholders that don\'t have a replacement in the map', () => {
-    const str = 'Example string where a replacement for {PLACEHOLDER_placeholder} is not provided.';
-
-    assert.throws(() => IssuesManager.MarkdownIssueDescription.substitutePlaceholders(str));
-  });
-
-  it('ignores placeholder syntax where the placeholder doesn\'t have the PLACEHOLDER prefix', () => {
-    const str = 'Example string with a {placeholder} that must be ignored.';
-
-    assert.strictEqual(IssuesManager.MarkdownIssueDescription.substitutePlaceholders(str), str);
-  });
-
-  it('throws an error for unused replacements', () => {
-    const str = 'Example string with no placeholder';
-
-    assert.throws(
-        () =>
-            IssuesManager.MarkdownIssueDescription.substitutePlaceholders(str, new Map([['PLACEHOLDER_FOO', 'bar']])));
-  });
-
-  it('allows the same placeholder to be used multiple times', () => {
-    const str = 'Example string with the same placeholder used twice: {PLACEHOLDER_PH1} {PLACEHOLDER_PH1}';
-
-    const actual =
-        IssuesManager.MarkdownIssueDescription.substitutePlaceholders(str, new Map([['PLACEHOLDER_PH1', 'foo']]));
-    assert.strictEqual(actual, 'Example string with the same placeholder used twice: foo foo');
-  });
-
-  it('throws an error for invalid placeholder syntax provided in the substitutions map', () => {
-    const str = 'Example string with no placeholder';
-
-    assert.throws(
-        () => IssuesManager.MarkdownIssueDescription.substitutePlaceholders(str, new Map([['invalid_ph', 'foo']])));
+    assert.throws(() => {
+      IssuesManager.MarkdownIssueDescription.createIssueDescriptionFromRawMarkdown('# Title\n\nBody', {
+        file: '<unused>',
+        links: [],
+        substitutions: new Map([['PLACEHOLDER_Unused', 'val']]),
+      });
+    }, /Unused replacements provided: PLACEHOLDER_Unused/);
   });
 });
diff --git a/front_end/models/issues_manager/MarkdownIssueDescription.ts b/front_end/models/issues_manager/MarkdownIssueDescription.ts
index 0068d57..62554cd 100644
--- a/front_end/models/issues_manager/MarkdownIssueDescription.ts
+++ b/front_end/models/issues_manager/MarkdownIssueDescription.ts
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import * as Platform from '../../core/platform/platform.js';
+import * as TextUtils from '../../core/text_utils/text_utils.js';
 import * as Marked from '../../third_party/marked/marked.js';
 
 /**
@@ -75,9 +76,8 @@
 
 export async function createIssueDescriptionFromMarkdown(description: MarkdownIssueDescription):
     Promise<IssueDescription> {
-  const rawMarkdown = await getMarkdownFileContent(description.file);
-  const rawMarkdownWithPlaceholdersReplaced = substitutePlaceholders(rawMarkdown, description.substitutions);
-  return createIssueDescriptionFromRawMarkdown(rawMarkdownWithPlaceholdersReplaced, description);
+  const markdown = await getMarkdownFileContent(description.file);
+  return createIssueDescriptionFromRawMarkdown(markdown, description);
 }
 
 /**
@@ -85,7 +85,7 @@
  */
 export function createIssueDescriptionFromRawMarkdown(
     markdown: string, description: MarkdownIssueDescription): IssueDescription {
-  const markdownAst = Marked.Marked.lexer(markdown);
+  const markdownAst = TextUtils.Markdown.tokenizeWithPlaceholders(markdown, description.substitutions);
   const markdownTitle = findTitleFromMarkdownAst(markdownAst);
   if (!markdownTitle) {
     throw new Error('Markdown issue descriptions must start with a heading');
@@ -99,49 +99,6 @@
   };
 }
 
-const validPlaceholderMatchPattern = /\{(PLACEHOLDER_[a-zA-Z][a-zA-Z0-9]*)\}/g;
-const validPlaceholderNamePattern = /PLACEHOLDER_[a-zA-Z][a-zA-Z0-9]*/;
-
-/**
- * Replaces placeholders in markdown text with a string provided by the
- * `substitutions` map. To keep mental overhead to a minimum, the same
- * syntax is used as for l10n placeholders. Please note that the
- * placeholders require a mandatory 'PLACEHOLDER_' prefix.
- *
- * Example:
- *   const str = "This is markdown with `code` and two placeholders, namely {PLACEHOLDER_PH1} and {PLACEHOLDER_PH2}".
- *   const result = substitutePlaceholders(str, new Map([['PLACEHOLDER_PH1', 'foo'], ['PLACEHOLDER_PH2', 'bar']]));
- *
- * Exported only for unit testing.
- */
-export function substitutePlaceholders(markdown: string, substitutions?: Map<string, string>): string {
-  const unusedPlaceholders = new Set(substitutions ? substitutions.keys() : []);
-  validatePlaceholders(unusedPlaceholders);
-
-  const result = markdown.replace(validPlaceholderMatchPattern, (_, placeholder) => {
-    const replacement = substitutions ? substitutions.get(placeholder) : undefined;
-    if (replacement === undefined) {
-      throw new Error(`No replacement provided for placeholder '${placeholder}'.`);
-    }
-    unusedPlaceholders.delete(placeholder);
-    return replacement;
-  });
-
-  if (unusedPlaceholders.size > 0) {
-    throw new Error(`Unused replacements provided: ${[...unusedPlaceholders]}`);
-  }
-
-  return result;
-}
-
-/** Ensure that all provided placeholders match the naming pattern. **/
-function validatePlaceholders(placeholders: Set<string>): void {
-  const invalidPlaceholders = [...placeholders].filter(placeholder => !validPlaceholderNamePattern.test(placeholder));
-  if (invalidPlaceholders.length > 0) {
-    throw new Error(`Invalid placeholders provided in the substitutions map: ${invalidPlaceholders}`);
-  }
-}
-
 export function findTitleFromMarkdownAst(markdownAst: Marked.Marked.Token[]): string|null {
   if (markdownAst.length === 0 || markdownAst[0].type !== 'heading' || markdownAst[0].depth !== 1) {
     return null;
diff --git a/front_end/panels/issues/IssueView.test.ts b/front_end/panels/issues/IssueView.test.ts
index 913abe2..5037f30 100644
--- a/front_end/panels/issues/IssueView.test.ts
+++ b/front_end/panels/issues/IssueView.test.ts
@@ -87,4 +87,64 @@
     assert.strictEqual(row.getAttribute('data-backend-node-id'), '42');
     assert.strictEqual(row.getAttribute('data-target-id'), target.id());
   });
+
+  it('renders issue title and substitutes placeholders in description body using MarkdownPlaceholderLitRenderer',
+     () => {
+       const aggregationKey = 'key' as unknown as IssuesManager.IssueAggregator.AggregationKey;
+       const issue = StubIssue.createFromRequestIds(['id1']);
+       const aggregatedIssue = new IssuesManager.IssueAggregator.AggregatedIssue('code', aggregationKey);
+       aggregatedIssue.addInstance(issue);
+       const description = IssuesManager.MarkdownIssueDescription.createIssueDescriptionFromRawMarkdown(
+           '# Fallback heading\n\nCheck {PLACEHOLDER_message} for details.', {
+             file: '<unused>',
+             title: 'Deprecated feature used',
+             links: [],
+             substitutions: new Map([
+               ['PLACEHOLDER_message', 'body details'],
+             ]),
+           });
+       const view = new Issues.IssueView.IssueView(aggregatedIssue, description);
+       const treeOutline = new UI.TreeOutline.TreeOutline();
+       treeOutline.appendChild(view);
+
+       assert.strictEqual(view.getIssueTitle(), 'Deprecated feature used');
+       assert.strictEqual(view.listItemElement.querySelector('.title')?.textContent, 'Deprecated feature used');
+       const markdownView = view.childrenListElement.querySelector('devtools-markdown-view');
+       assert.exists(markdownView);
+       assert.strictEqual(markdownView.shadowRoot?.querySelector('.markdown-placeholder')?.textContent, 'body details');
+       view.clear();
+     });
+
+  it('safely escapes HTML, Markdown links, and Unicode BiDi overrides in MarkdownView body without double-escaping special characters',
+     () => {
... (file diff truncated)