From e1eae610c08d45fab509fd0d3c2e475cf0327bd3 Mon Sep 17 00:00:00 2001 From: Suhaib Mujahid Date: Tue, 25 Aug 2026 12:35:07 -0400 Subject: [PATCH] Bug 2060932 - Support GitHub-style collapsible sections in comments Comments render markdown with cmark's safe option and every `<` escaped beforehand, so raw HTML never reaches the parser. Instead of weakening that, convert the four disclosure tags back to real elements after rendering: mark the escaped tags in text nodes (skipping pre/code so the syntax can still be documented), then re-parse so the block level elements are lifted out of the paragraph markdown wrapped them in. Only those four exact tags are recognized and they never carry attributes, so no other markup can be smuggled in. The marker characters are stripped from the input so they cannot be forged, and unbalanced tags cannot leak an unclosed element into the page. --- Bugzilla/Markdown.pm | 65 +++++++++++++++++++++++++++++++++++++-- skins/standard/global.css | 12 ++++++++ t/markdown.t | 63 +++++++++++++++++++++++++++++++++++++ 3 files changed, 137 insertions(+), 3 deletions(-) diff --git a/Bugzilla/Markdown.pm b/Bugzilla/Markdown.pm index 2a77b4932c..efc67daad9 100644 --- a/Bugzilla/Markdown.pm +++ b/Bugzilla/Markdown.pm @@ -36,6 +36,30 @@ sub _build_markdown_parser { } my $MARKDOWN_OFF = quotemeta '#[markdown(off)]'; + +# The only raw HTML allowed in comments: GitHub-style collapsible sections. +# Markdown rendering escapes all tags, so the escaped text is swapped back to +# real elements afterwards. Only these exact tags are recognized and they never +# carry attributes, so no other markup can be smuggled in. +my %DISCLOSURE_MARKER = ( + '
' => "\x{E000}", + '
' => "\x{E001}", + '' => "\x{E002}", + '' => "\x{E003}", +); + +# Markdown wraps the tags in a paragraph. Closing and reopening it lets the +# HTML parser lift the block level disclosure elements out of the paragraph; +# the empty paragraphs left behind are dropped afterwards. +my %DISCLOSURE_HTML = ( + "\x{E000}" => '

', + "\x{E001}" => '

', + "\x{E002}" => '

', + "\x{E003}" => '

', +); + +my $DISCLOSURE_RE = qr{}i; + sub render_html { my ($self, $markdown, $bug, $comment, $user) = @_; my $parser = $self->markdown_parser; @@ -60,9 +84,12 @@ sub render_html { return $html; } + my $has_disclosure = $markdown =~ $DISCLOSURE_RE; + # Replace < with \x{FFFD} (special unicode replacement character), - # and remove \x{FFFD} later. - $markdown =~ tr/\x{FFFD}//d; + # and remove \x{FFFD} later. The private use characters reserved for the + # disclosure markers are dropped too, so they can't be forged in a comment. + $markdown =~ tr/\x{FFFD}\x{E000}-\x{E003}//d; $markdown =~ s{<(?!https?://)}{\x{FFFD}}gs; my @valid_text_parent_tags = ('h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'p', 'li', 'td'); @@ -91,8 +118,40 @@ sub render_html { }); return $node; }); - return $dom->to_string; + return $has_disclosure ? _expand_disclosure_tags($dom) : $dom->to_string; +} + +# Turn the escaped

/ text left by the markdown renderer back +# into real elements. Text inside code blocks is skipped so the syntax can +# still be documented in a comment. +sub _expand_disclosure_tags { + my ($dom) = @_; + + my $found = 0; + $dom->descendant_nodes->each(sub { + my ($node) = @_; + return unless $node->type eq 'text'; + return if $node->ancestors('pre, code')->size; + my $text = $node->content; + return unless $text =~ s/($DISCLOSURE_RE)/$DISCLOSURE_MARKER{lc $1}/g; + $found = 1; + $node->content($text); + }); + + my $html = $dom->to_string; + return $html unless $found; + + $html =~ s/([\x{E000}-\x{E003}])/$DISCLOSURE_HTML{$1}/g; + + # Drop the line breaks and empty paragraphs the rewrite leaves behind. + $html =~ s{\s*\s*(?=

)}{}g; + $html =~ s{(?<=

)\s*\s*}{}g; + + my $expanded = Mojo::DOM->new($html); + $expanded->find('p') + ->grep(sub { !$_->children->size && $_->all_text !~ /\S/ })->map('remove'); + return $expanded->to_string; } sub _is_external_link { diff --git a/skins/standard/global.css b/skins/standard/global.css index ce8d31c36e..564a110412 100644 --- a/skins/standard/global.css +++ b/skins/standard/global.css @@ -2724,6 +2724,18 @@ div.bz_comment_text pre { margin: 0; } +.markdown-body details { + margin-bottom: 10px; +} + +.markdown-body details > *:last-child { + margin-bottom: 0; +} + +.markdown-body summary { + cursor: pointer; +} + .markdown-body ul, .markdown-body ol { padding-left: 0; diff --git a/t/markdown.t b/t/markdown.t index f8b541d5b2..30d10c6496 100644 --- a/t/markdown.t +++ b/t/markdown.t @@ -98,4 +98,67 @@ is($ahref->attr('href'), 'https://searchfox.org/mozilla-central/rev/76fe4bb38534 is($parser->render_html(''), "

<foo>

\n", "literal tags work"); +# Bug 2060932: collapsible sections via
/. +is( + $parser->render_html('
Text to click' + . 'Text hidden by default
'), + '
Text to click' + . "

Text hidden by default

\n", + 'Disclosure tags on a single line' +); + +my $details_block = <<'MARKDOWN'; +
+Click **me** + +Hidden content + +
+MARKDOWN + +is( + $parser->render_html($details_block), + "
Click me\n" + . "

Hidden content

\n
\n", + 'Disclosure tags as their own blocks, with markdown in the summary' +); + +is( + $parser->render_html("
Uphidden
"), + "
Up

hidden

\n", + 'Disclosure tags are case insensitive' +); + +is( + $parser->render_html("```\n
xy
\n```"), + "
<details><summary>x</summary>"
+    . "y</details>\n
\n", + 'Disclosure tags in a code block stay literal' +); + +is( + $parser->render_html('Use `
` to fold.'), + "

Use <details> to fold.

\n", + 'Disclosure tags in a code span stay literal' +); + +like( + $parser->render_html('
nope'), + qr{<details open onclick="x">nope}, + 'Only the bare disclosure tags are recognized' +); + +is( + $parser->render_html("\x{E000}\x{E002}nope\x{E003}\x{E001}"), + "

nope

\n", + 'The internal disclosure markers cannot be forged in a comment' +); + +# An unbalanced tag must not leak an unclosed element into the page. +like( + $parser->render_html("
\noops\n\nrest\n"), + qr{
\z}, + 'An unclosed disclosure section is closed for us' +); + done_testing;