From cb916c80bce8f0b4ea578bf7f3b187715999e1bd Mon Sep 17 00:00:00 2001 From: Suhaib Mujahid Date: Tue, 25 Aug 2026 12:35:07 -0400 Subject: [PATCH 1/3] 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; From 38ea5c2faf67fe2d908ecf0d385613860f08523f Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Tue, 22 Sep 2026 21:19:51 -0400 Subject: [PATCH 2/3] Bug 2074690 - BMO REST API cannot set any DATE-type custom field --- Bugzilla/WebService/Bug.pm | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm index 6a095fef2b..2e5f6fc511 100644 --- a/Bugzilla/WebService/Bug.pm +++ b/Bugzilla/WebService/Bug.pm @@ -56,11 +56,11 @@ sub DATE_FIELDS { update => [] }; - # Add date related custom fields + # Add datetime custom fields. Date-only fields are left out so they are + # passed through as YYYY-MM-DD, since converting them would append a time + # component that _check_date_field rejects. foreach my $field (Bugzilla->active_custom_fields({skip_extensions => 1})) { - next - unless ($field->type == FIELD_TYPE_DATETIME - || $field->type == FIELD_TYPE_DATE); + next unless $field->type == FIELD_TYPE_DATETIME; push(@{$fields->{create}}, $field->name); push(@{$fields->{update}}, $field->name); } From 0054326dace19d217e010916b0a39e65168e267d Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Wed, 23 Sep 2026 14:49:09 -0400 Subject: [PATCH 3/3] Copilot review fixes --- qa/config/generate_test_data.pl | 20 +++++++ qa/t/rest_bug_date_fields.t | 101 ++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+) create mode 100644 qa/t/rest_bug_date_fields.t diff --git a/qa/config/generate_test_data.pl b/qa/config/generate_test_data.pl index ec65ad3474..16367f9bdf 100644 --- a/qa/config/generate_test_data.pl +++ b/qa/config/generate_test_data.pl @@ -814,6 +814,26 @@ BEGIN obsolete => 0, values => [qw(one two three)], }, + { + name => 'cf_qa_date', + description => 'QA Date', + type => FIELD_TYPE_DATE, + sortkey => 300, + mailhead => 0, + enter_bug => 1, + custom => 1, + obsolete => 0, + }, + { + name => 'cf_qa_datetime', + description => 'QA DateTime', + type => FIELD_TYPE_DATETIME, + sortkey => 400, + mailhead => 0, + enter_bug => 1, + custom => 1, + obsolete => 0, + }, ); print "creating custom fields...\n"; diff --git a/qa/t/rest_bug_date_fields.t b/qa/t/rest_bug_date_fields.t new file mode 100644 index 0000000000..5507e5950e --- /dev/null +++ b/qa/t/rest_bug_date_fields.t @@ -0,0 +1,101 @@ +#!/usr/bin/env perl +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +##################################################### +# Test for REST Bug.create() and Bug.update() with # +# DATE and DATETIME custom fields # +# POST /rest/bug # +# PUT /rest/bug/ # +##################################################### + +# FIELD_TYPE_DATE custom fields must be passed through to the Bug object as +# YYYY-MM-DD. FIELD_TYPE_DATETIME custom fields must still be converted from +# ISO 8601 by the REST server before reaching the Bug object. See bug 2074690. + +use 5.10.1; +use strict; +use warnings; +use lib qw(lib ../../lib ../../local/lib/perl5); + +use Bugzilla; +use Bugzilla::Util qw(datetime_from); +use QA::Util qw(get_config); +use QA::Tests qw(create_bug_fields); +use QA::REST::Util qw(api_headers); + +use Test::Mojo; +use Test::More; + +use constant DATE_FIELD => 'cf_qa_date'; +use constant DATETIME_FIELD => 'cf_qa_datetime'; + +my $config = get_config(); +my $api_key = $config->{editbugs_user_api_key}; +my $url = Bugzilla->localconfig->urlbase; + +my $t = Test::Mojo->new(); + +# The REST API always returns dates as ISO 8601 in UTC with a trailing 'Z'. +# A date-only value is stored without a time zone, so compute the expected +# output the same way the server does rather than assuming it runs in UTC. +sub expected_iso8601 { + my ($value) = @_; + return datetime_from($value, 'UTC')->iso8601() . 'Z'; +} + +sub check_bug_dates { + my ($bug_id, $date, $datetime, $desc) = @_; + my $fields = join(',', DATE_FIELD, DATETIME_FIELD); + $t->get_ok( + $url . "rest/bug/$bug_id?include_fields=$fields" => api_headers($api_key)) + ->status_is(200) + ->json_is('/bugs/0/' . DATE_FIELD, expected_iso8601($date), + "$desc: date field has the right value") + ->json_is('/bugs/0/' . DATETIME_FIELD, $datetime, + "$desc: datetime field has the right value"); +} + +############################### +# Create with both field types # +############################### + +my $new_bug = create_bug_fields($config); +$new_bug->{+DATE_FIELD} = '2026-01-15'; +$new_bug->{+DATETIME_FIELD} = '2026-01-15T12:15:00Z'; + +$t->post_ok($url . 'rest/bug' => api_headers($api_key) => json => $new_bug) + ->status_is(200)->json_has('/id'); +my $bug_id = $t->tx->res->json->{id}; + +check_bug_dates($bug_id, '2026-01-15', '2026-01-15T12:15:00Z', 'After create'); + +############################### +# Update with both field types # +############################### + +$t->put_ok($url + . "rest/bug/$bug_id" => api_headers($api_key) => json => + {DATE_FIELD, '2026-02-20', DATETIME_FIELD, '2026-02-20T08:45:00Z'}) + ->status_is(200); + +check_bug_dates($bug_id, '2026-02-20', '2026-02-20T08:45:00Z', 'After update'); + +############################################# +# Date fields still reject a time component # +############################################# + +$t->put_ok($url + . "rest/bug/$bug_id" => api_headers($api_key) => json => + {DATE_FIELD, '2026-03-01 10:00:00'})->status_is(400) + ->json_is('/code' => 56) + ->json_like('/message' => qr/is not a legal date/); + +check_bug_dates($bug_id, '2026-02-20', '2026-02-20T08:45:00Z', + 'After rejected update'); + +done_testing();