From 5305100f2a1fd0623391d5cf9b66755bfc1c6b79 Mon Sep 17 00:00:00 2001 From: Kohei Yoshino Date: Mon, 9 Nov 2026 00:17:54 -0500 Subject: [PATCH 1/6] Bug 2086124 - [HACKBOT] Structured metadata on comments --- Bugzilla/Comment.pm | 2 + Bugzilla/Hook.pm | 19 ++ .../bug_modal/activity_stream.html.tmpl | 2 +- .../default/bug_modal/comment_tags.html.tmpl | 6 +- extensions/BugModal/web/bug_modal.css | 3 +- extensions/Hackbot/Config.pm | 20 ++ extensions/Hackbot/Extension.pm | 69 ++++++ extensions/Hackbot/lib/Config.pm | 37 +++ extensions/Hackbot/lib/Metadata.pm | 55 +++++ extensions/Hackbot/t/bmo/comment-metadata.t | 227 ++++++++++++++++++ extensions/Hackbot/t/metadata.t | 82 +++++++ .../en/default/admin/params/hackbot.html.tmpl | 20 ++ .../default/hook/bug/comments-user.html.tmpl | 12 + ...ty_stream-before_comment_actions.html.tmpl | 23 ++ .../comment_tags-before_tags.html.tmpl | 13 + .../hook/bug_modal/header-end.html.tmpl | 9 + extensions/Hackbot/web/styles/hackbot.css | 54 +++++ extensions/Push/lib/Serialize.pm | 8 +- qa/config/generate_test_data.pl | 2 +- qa/config/selenium_test.conf | 4 + qa/t/1_test_hackbot_comments.t | 210 ++++++++++++++++ skins/standard/global.css | 1 + 22 files changed, 872 insertions(+), 6 deletions(-) create mode 100644 extensions/Hackbot/Config.pm create mode 100644 extensions/Hackbot/Extension.pm create mode 100644 extensions/Hackbot/lib/Config.pm create mode 100644 extensions/Hackbot/lib/Metadata.pm create mode 100644 extensions/Hackbot/t/bmo/comment-metadata.t create mode 100644 extensions/Hackbot/t/metadata.t create mode 100644 extensions/Hackbot/template/en/default/admin/params/hackbot.html.tmpl create mode 100644 extensions/Hackbot/template/en/default/hook/bug/comments-user.html.tmpl create mode 100644 extensions/Hackbot/template/en/default/hook/bug_modal/activity_stream-before_comment_actions.html.tmpl create mode 100644 extensions/Hackbot/template/en/default/hook/bug_modal/comment_tags-before_tags.html.tmpl create mode 100644 extensions/Hackbot/template/en/default/hook/bug_modal/header-end.html.tmpl create mode 100644 extensions/Hackbot/web/styles/hackbot.css create mode 100644 qa/t/1_test_hackbot_comments.t diff --git a/Bugzilla/Comment.pm b/Bugzilla/Comment.pm index 127c7b304a..e73a3c4867 100644 --- a/Bugzilla/Comment.pm +++ b/Bugzilla/Comment.pm @@ -406,6 +406,8 @@ sub body_full { } else { $body = $self->body; + Bugzilla::Hook::process('comment_body_full', + {comment => $self, body => \$body}); } if ($params->{wrap} and !$self->already_wrapped) { $body = wrap_comment($body); diff --git a/Bugzilla/Hook.pm b/Bugzilla/Hook.pm index e230dd92cd..b79a7fd819 100644 --- a/Bugzilla/Hook.pm +++ b/Bugzilla/Hook.pm @@ -735,6 +735,25 @@ spaces. =back +=head2 comment_body_full + +Called by L for a plain comment, before any +wrapping, so that an extension can change the text people read without +changing the stored text. Bugmail, webhook and Pulse payloads use the changed +text, while L and the C REST field keep +returning the stored text. + +Params: + +=over + +=item C - The L being read. + +=item C - A reference to the comment text, which you can modify. + +=back + + =head2 config_add_panels If you want to add new panels to the Parameters administrative interface, diff --git a/extensions/BugModal/template/en/default/bug_modal/activity_stream.html.tmpl b/extensions/BugModal/template/en/default/bug_modal/activity_stream.html.tmpl index c7413b062f..e11a106bb7 100644 --- a/extensions/BugModal/template/en/default/bug_modal/activity_stream.html.tmpl +++ b/extensions/BugModal/template/en/default/bug_modal/activity_stream.html.tmpl @@ -167,7 +167,7 @@ [% Hook.process('user', 'bug/comments.html.tmpl') %] [% END %] - + [% Hook.process('before_comment_actions', 'bug_modal/activity_stream.html.tmpl') %]
[% IF user.is_insider && bug.check_can_change_field('longdesc', 0, 1).allowed %] [% IF comment.is_private %] diff --git a/extensions/BugModal/template/en/default/bug_modal/comment_tags.html.tmpl b/extensions/BugModal/template/en/default/bug_modal/comment_tags.html.tmpl index 02e0ca1280..b733084455 100644 --- a/extensions/BugModal/template/en/default/bug_modal/comment_tags.html.tmpl +++ b/extensions/BugModal/template/en/default/bug_modal/comment_tags.html.tmpl @@ -6,8 +6,12 @@ # defined by the Mozilla Public License, v. 2.0. #%] +[% hidden_tags = [] %] +[% Hook.process('before_tags', 'bug_modal/comment_tags.html.tmpl') %] + [% FOREACH tag IN comment.tags ~%] - + [%~ '×' IF user.can_tag_comments %] [% IF comment.tag_url(tag) %] [role="group"] { justify-content: flex-end; flex-wrap: wrap; - gap: 4px 0; - width: 128px; + width: 64px; } .comment .comment-actions > [role="group"] .comment-private { diff --git a/extensions/Hackbot/Config.pm b/extensions/Hackbot/Config.pm new file mode 100644 index 0000000000..c05acc478f --- /dev/null +++ b/extensions/Hackbot/Config.pm @@ -0,0 +1,20 @@ +# 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. + +package Bugzilla::Extension::Hackbot; + +use 5.10.1; +use strict; +use warnings; + +use constant NAME => 'Hackbot'; + +use constant REQUIRED_MODULES => []; + +use constant OPTIONAL_MODULES => []; + +__PACKAGE__->NAME; diff --git a/extensions/Hackbot/Extension.pm b/extensions/Hackbot/Extension.pm new file mode 100644 index 0000000000..3db10a7784 --- /dev/null +++ b/extensions/Hackbot/Extension.pm @@ -0,0 +1,69 @@ +# 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. + +package Bugzilla::Extension::Hackbot; + +use 5.10.1; +use strict; +use warnings; + +use base qw(Bugzilla::Extension); + +use Bugzilla::Extension::Hackbot::Metadata qw(parse_front_matter); +use Bugzilla::User; + +our $VERSION = '1'; + +BEGIN { + no warnings 'redefine'; + *Bugzilla::Comment::hackbot = \&_comment_hackbot; +} + +# The ID of the account named by the hackbot_user param, or 0 when it is unset +# or names nobody. Bugzilla::User::login_to_id doesn't cache a miss, and this +# runs for every comment that is read. +sub _hackbot_user_id { + my $login = Bugzilla->params->{hackbot_user} or return 0; + my $cache = Bugzilla->request_cache->{hackbot_user_ids} ||= {}; + return $cache->{$login} //= Bugzilla::User::login_to_id($login) || 0; +} + +# Anyone can type a front matter block into a comment, so only the Hackbot +# account's comments are parsed. Returns the same list as parse_front_matter. +# A page reads each comment several times (header, badge, tags, text), so the +# result is kept on the comment for as long as its text is unchanged. +sub _parse_comment { + my ($comment) = @_; + my $user_id = _hackbot_user_id() or return; + return unless ($comment->{who} // 0) == $user_id; + + my $body = $comment->body; + my $parsed = $comment->{hackbot_front_matter}; + if (!$parsed || $parsed->[0] ne $body) { + $parsed = $comment->{hackbot_front_matter} = [$body, parse_front_matter($body)]; + } + return @$parsed[1 .. $#$parsed]; +} + +sub _comment_hackbot { + my ($comment) = @_; + my ($metadata) = _parse_comment($comment); + return $metadata; +} + +sub comment_body_full { + my ($self, $args) = @_; + my (undef, $body) = _parse_comment($args->{comment}); + ${$args->{body}} = $body if defined $body; +} + +sub config_add_panels { + my ($self, $args) = @_; + $args->{panel_modules}->{Hackbot} = 'Bugzilla::Extension::Hackbot::Config'; +} + +__PACKAGE__->NAME; diff --git a/extensions/Hackbot/lib/Config.pm b/extensions/Hackbot/lib/Config.pm new file mode 100644 index 0000000000..eea1c9076a --- /dev/null +++ b/extensions/Hackbot/lib/Config.pm @@ -0,0 +1,37 @@ +# 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. + +package Bugzilla::Extension::Hackbot::Config; + +use 5.10.1; +use strict; +use warnings; + +use Bugzilla::Config::Common; +use Bugzilla::User; + +our $sortkey = 1310; + +sub get_param_list { + my ($class) = @_; + + return ({ + name => 'hackbot_user', + type => 't', + default => '', + checker => \&check_hackbot_user, + }); +} + +# Empty is valid: it turns the extension off. +sub check_hackbot_user { + my ($login) = @_; + return '' if $login eq '' || Bugzilla::User::login_to_id($login); + return "There is no user named '$login'."; +} + +1; diff --git a/extensions/Hackbot/lib/Metadata.pm b/extensions/Hackbot/lib/Metadata.pm new file mode 100644 index 0000000000..c48fe7e17e --- /dev/null +++ b/extensions/Hackbot/lib/Metadata.pm @@ -0,0 +1,55 @@ +# 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. + +package Bugzilla::Extension::Hackbot::Metadata; + +use 5.10.1; +use strict; +use warnings; + +use base qw(Exporter); + +our @EXPORT_OK = qw(parse_front_matter); + +# The block hackbot-api puts at the top of a comment: +# +# --- +# hackbot: +# run: 550e8400-e29b-41d4-a716-446655440000 +# action: 3f2a9c1e7b4d4f0a8e6c2d1b5a7f9e30 +# agent: bug-fix +# url: https://hackbot.moz.tools/runs/550e8400-e29b-41d4-a716-446655440000 +# --- +# +# It is only ever generated, so this reads that one shape rather than YAML in +# general. Anything else is an ordinary comment that happens to start with a +# rule, and unknown keys are skipped so that fields can be added. +my $FRONT_MATTER = qr/\A---\nhackbot:\n((?: [a-z_]+: [^\n]*\n)+)---(?:\n+|\z)/; +my $IDENTIFIER = qr/\A[A-Za-z0-9._-]+\z/; + +# Returns the metadata hash and the comment text without the block, or an empty +# list when the text doesn't start with a valid block. The url key is left out +# unless it is an https URL, because it ends up in a link's href. +sub parse_front_matter { + my ($text) = @_; + + return unless defined $text && $text =~ $FRONT_MATTER; + my ($block, $body) = ($1, substr($text, $+[0])); + + my %fields = $block =~ /^ ([a-z_]+): ([^\n]*?)\s*$/mg; + + my %metadata; + foreach my $key (qw(run action agent)) { + return unless ($fields{$key} // '') =~ $IDENTIFIER; + $metadata{$key} = $fields{$key}; + } + $metadata{url} = $fields{url} if ($fields{url} // '') =~ m{\Ahttps://\S+\z}; + + return (\%metadata, $body); +} + +1; diff --git a/extensions/Hackbot/t/bmo/comment-metadata.t b/extensions/Hackbot/t/bmo/comment-metadata.t new file mode 100644 index 0000000000..edb7c77f67 --- /dev/null +++ b/extensions/Hackbot/t/bmo/comment-metadata.t @@ -0,0 +1,227 @@ +#!/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. + +# Bug 2086124: Hackbot comments carry a front matter block that links them to +# the run that wrote them. Comments are posted through the real REST API, +# in-process with Test::Mojo, then read back. The parser itself is covered +# without a DB in extensions/Hackbot/t/metadata.t. + +use 5.10.1; +use strict; +use warnings; +use lib qw(. lib local/lib/perl5); + +BEGIN { + $ENV{LOG4PERL_CONFIG_FILE} = 'log4perl-t.conf'; + $ENV{BUGZILLA_DISABLE_HOSTAGE} = 1; +} + +use Bugzilla; +BEGIN { Bugzilla->extensions } +use Bugzilla::App (); +use Bugzilla::Comment; +use Bugzilla::Config qw(SetParam write_params); +use Bugzilla::Constants qw(bz_locations); +use Bugzilla::Extension::Hackbot::Config; +use Bugzilla::Extension::Push::Serialize; +use Bugzilla::Test::Util qw(create_user issue_api_key); +use Bugzilla::User; +use Bugzilla::User::Setting qw(clear_settings_cache); + +use Email::MIME; +use Test::More; +use Test::Mojo; + +# The app reads params from the database (through memcached) on every +# request, so set them there, and put them back afterwards. Bugmail is sent +# while the comment is posted and written to a file, so it can be read here. +my %original_params = map { $_ => Bugzilla->params->{$_} // '' } + qw(hackbot_user use_mailer_queue mail_delivery_method); + +sub set_params { + my (%params) = @_; + SetParam($_, $params{$_}) foreach keys %params; + write_params(); + delete Bugzilla->request_cache->{hackbot_user_ids}; +} + +sub set_hackbot_user { set_params(hackbot_user => $_[0]) } + +END { set_params(%original_params) if %original_params } +set_params( + hackbot_user => '', + use_mailer_queue => 0, + mail_delivery_method => 'Test' +); + +my $admin = Bugzilla::User->check({id => 1}); +Bugzilla->set_user($admin); + +# Reused on a rerun. +my $hackbot = create_user('hackbot@hackbot-metadata.test', '*'); +my $other = create_user('other@hackbot-metadata.test', '*'); + +# Gets bugmail as text and HTML. Bugmail skips fake addresses (.bugs, .tld), +# so it has an ordinary one. Set up before the app runs any request: creating +# a user audits the change with remote_ip(), which wants a controller once a +# request has run. +my $watcher = create_user('watcher@hackbot-metadata.test', '*'); +$watcher->settings->{email_format}->set('html'); +clear_settings_cache($watcher->id); + +my %api_key = map { $_->login => issue_api_key($_->login)->api_key } $admin, + $hackbot, $other; + +# One Test::Mojo for the whole file: building the app twice makes CGI::Compile +# die with "Tried to load admin.cgi more than once". +my $t = Test::Mojo->new('Bugzilla::App'); + +sub api_headers { + my ($user) = @_; + return {'X-Bugzilla-API-Key' => $api_key{$user->login}}; +} + +$t->post_ok( + '/rest/bug' => api_headers($admin) => json => { + product => 'Firefox', + component => 'General', + version => 'Trunk', + type => 'defect', + severity => 'normal', + op_sys => 'Unspecified', + platform => 'Unspecified', + summary => 'Hackbot comment metadata', + description => 'A bug for Hackbot to comment on', + } +)->status_is(200, 'the bug is filed') + or diag $t->tx->res->body; +my $bug_id = $t->tx->res->json->{id}; + +# Post a comment as $user and return it, read back from the database. +sub post_comment { + my ($user, $text) = @_; + $t->post_ok("/rest/bug/$bug_id/comment" => api_headers($user) => json => + {comment => $text, is_markdown => 1})->status_is(201, 'the comment is added') + or diag $t->tx->res->body; + return Bugzilla::Comment->new($t->tx->res->json->{id}); +} + +my $url = 'https://hackbot.example/runs/abc-123'; +my $metadata + = {run => 'abc-123', action => 'f00', agent => 'bug-fix', url => $url}; +my $block = join("\n", + '---', 'hackbot:', ' run: abc-123', + ' action: f00', ' agent: bug-fix', " url: $url", + '---', '', ''); +my $text = 'The failure is here.'; + +# hackbot_user is empty: the feature is off. +my $comment = post_comment($hackbot, "$block$text"); +is($comment->body_full, "$block$text", 'off: the block stays in the text'); +is($comment->hackbot, undef, 'off: no metadata'); + +# hackbot_user is set. +set_hackbot_user($hackbot->login); + +$comment = post_comment($hackbot, "$block$text"); +is($comment->body_full, $text, 'body_full hides the block'); +is($comment->body_full({wrap => 1}), $text, '... when wrapped too'); +is($comment->body, "$block$text", 'body keeps the stored text'); +is_deeply($comment->hackbot, $metadata, 'hackbot returns the metadata'); + +$t->get_ok('/rest/bug/comment/' . $comment->id => api_headers($other)) + ->status_is(200) + ->json_is('/comments/' . $comment->id . '/text', + $text, 'REST text has no block') + ->json_is( + '/comments/' . $comment->id . '/raw_text', + "$block$text", + 'REST raw_text keeps the stored text' + ); + +# Webhook and Pulse payloads carry the text people read, as bugmail does. +my $push = Bugzilla::Extension::Push::Serialize->instance; +is($push->object_to_hash($comment, 1)->{body}, + $text, 'webhook and Pulse payloads have no block'); + +# Bugmail, in both formats, has the text and no block. +$t->put_ok("/rest/bug/$bug_id" => api_headers($admin) => json => + {cc => {add => [$watcher->login]}})->status_is(200, 'the watcher is CCd'); + +my $mail_file = bz_locations()->{datadir} . '/mailer.testfile'; +my $mail_start = -s $mail_file // 0; + +# Bugmail covers the changes since the last mail, to the second, so a comment +# posted in the same second as the CC change could be left out. +sleep 1; +my $mail_text = 'Mail check: the failure is here.'; +post_comment($hackbot, "$block$mail_text"); + +# The text/plain and text/html parts of the mail sent to the watcher, as +# [type, text] pairs. +my @parts; +if (open my $fh, '<', $mail_file) { + seek $fh, $mail_start, 0; + my $new_mail = do { local $/; <$fh> }; + foreach + my $raw ($new_mail =~ /^-{10} begin message\n(.*?)^-{10} end message/msg) + { + my $email = Email::MIME->new($raw); + next unless ($email->header('To') // '') eq $watcher->login; + $email->walk_parts(sub { + my ($part) = @_; + my ($type) = $part->content_type =~ m{^(text/(?:plain|html))}; + push @parts, [$type, $part->body_str] if $type; + }); + } +} +my @comment_parts = grep { $_->[1] =~ /\Q$mail_text\E/ } @parts; +is(scalar @comment_parts, 2, 'bugmail has the comment as text and HTML'); +foreach my $part (@comment_parts) { + unlike($part->[1], qr/hackbot:|run: abc-123/, + "$part->[0] bugmail has no block"); +} + +# The text is parsed again after an edit. +is($comment->hackbot->{run}, 'abc-123', 'metadata before the edit'); +(my $edited_block = $block) =~ s/abc-123/def-456/g; +$comment->update_text({ + text => "${edited_block}Edited.", + timestamp => Bugzilla->dbh->selectrow_array('SELECT NOW()'), + is_hidden => 0, +}); +is($comment->hackbot->{run}, 'def-456', 'an edited block is read again'); +is($comment->body_full, 'Edited.', '... and hidden'); +$comment->update_text({ + text => 'Edited, with no block.', + timestamp => Bugzilla->dbh->selectrow_array('SELECT NOW()'), + is_hidden => 0, +}); +is($comment->hackbot, undef, 'a removed block leaves no metadata'); +is($comment->body_full, 'Edited, with no block.', '... and the text is whole'); + +# Anyone can type the block. +my $forged = post_comment($other, "$block$text"); +is($forged->body_full, "$block$text", 'another user: the block stays as text'); +is($forged->hackbot, undef, 'another user: no metadata'); + +my $plain = post_comment($hackbot, 'A plain comment'); +is($plain->body_full, 'A plain comment', 'plain Hackbot comment is unchanged'); +is($plain->hackbot, undef, 'plain Hackbot comment has no metadata'); + +# The hackbot_user param. +my $check = \&Bugzilla::Extension::Hackbot::Config::check_hackbot_user; +is($check->(''), '', 'an empty login is valid'); +is($check->($hackbot->login), '', 'an existing login is valid'); +like( + $check->('nobody@hackbot-metadata.test'), + qr/no user named/, + 'an unknown login is rejected' +); + +done_testing; diff --git a/extensions/Hackbot/t/metadata.t b/extensions/Hackbot/t/metadata.t new file mode 100644 index 0000000000..62a642245a --- /dev/null +++ b/extensions/Hackbot/t/metadata.t @@ -0,0 +1,82 @@ +#!/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. + +use strict; +use warnings; +use lib qw( . lib local/lib/perl5 ); + +BEGIN { + $ENV{LOG4PERL_CONFIG_FILE} = 'log4perl-t.conf'; + $ENV{BUGZILLA_DISABLE_HOSTAGE} = 1; +} + +use Bugzilla::Test::MockDB; +use Bugzilla::Test::MockParams; + +use Test2::V0; + +use Bugzilla; +BEGIN { Bugzilla->extensions } + +my $parse = \&Bugzilla::Extension::Hackbot::Metadata::parse_front_matter; + +my $url = 'https://hackbot.moz.tools/runs/abc-123'; + +sub block { + my (%fields) = (run => 'abc-123', action => 'f00', agent => 'bug-fix', @_); + my $lines = join('', map { + defined $fields{$_} ? " $_: $fields{$_}\n" : '' + } qw(run action agent url extra)); + return "---\nhackbot:\n$lines---\n"; +} + +my ($metadata, $body) = $parse->(block(url => $url) . "\nThe failure is here.\n"); +is( + $metadata, + {run => 'abc-123', action => 'f00', agent => 'bug-fix', url => $url}, + 'fields are read' +); +is($body, "The failure is here.\n", 'block and blank line are removed'); + +($metadata, $body) = $parse->(block()); +is($body, '', 'a block with no text after it'); + +($metadata, $body) = $parse->(substr(block(), 0, -1)); +is($metadata->{run}, 'abc-123', 'stored text has its final newline trimmed'); + +($metadata) = $parse->(block(extra => 'value')); +ok(!exists $metadata->{extra}, 'unknown keys are skipped'); + +($metadata, $body) = $parse->(block() . "\nA\n\n---\n\nB"); +is($body, "A\n\n---\n\nB", 'a later rule is part of the text'); + +($metadata, $body) = $parse->(block() . block() . 'text'); +is($body, block() . 'text', 'only the first block counts'); + +($metadata) = $parse->(block(url => 'javascript:alert(1)')); +is($metadata->{run}, 'abc-123', 'a bad url does not reject the block'); +ok(!exists $metadata->{url}, 'only https URLs are kept'); + +($metadata) = $parse->(block(url => 'https://a.test/"x y')); +ok(!exists $metadata->{url}, 'a URL with spaces is dropped'); + +for my $key (qw(run action agent)) { + is([$parse->(block($key => undef))], [], "$key is required"); + is([$parse->(block($key => ''))], [], "$key must be an identifier"); +} + +is([$parse->(undef)], [], 'undef text'); +is([$parse->('')], [], 'empty text'); +is([$parse->('Plain comment')], [], 'plain comment'); +is([$parse->("---\n\nA comment starting with a rule")], [], 'leading rule'); +is([$parse->("---\ntitle: x\n---\ntext")], [], 'other front matter'); +is([$parse->('Text first' . block())], [], 'block not at the top'); +is([$parse->("\n" . block())], [], 'block after a blank line'); +is([$parse->("---\nhackbot:\n run: x\n")], [], 'unterminated block'); + +done_testing; diff --git a/extensions/Hackbot/template/en/default/admin/params/hackbot.html.tmpl b/extensions/Hackbot/template/en/default/admin/params/hackbot.html.tmpl new file mode 100644 index 0000000000..6f056de9a2 --- /dev/null +++ b/extensions/Hackbot/template/en/default/admin/params/hackbot.html.tmpl @@ -0,0 +1,20 @@ +[%# 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. + #%] + +[% + title = "Hackbot" + desc = "Settings for the Hackbot integration" +%] + +[% + param_descs = { + hackbot_user => + "The login of the Hackbot account. Metadata at the top of this account's comments is hidden and shown in the " + _ "comment header instead. Leave this empty to turn the integration off.", + } +%] diff --git a/extensions/Hackbot/template/en/default/hook/bug/comments-user.html.tmpl b/extensions/Hackbot/template/en/default/hook/bug/comments-user.html.tmpl new file mode 100644 index 0000000000..08457fafbc --- /dev/null +++ b/extensions/Hackbot/template/en/default/hook/bug/comments-user.html.tmpl @@ -0,0 +1,12 @@ +[%# 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. + #%] + +[% hackbot = comment.hackbot %] +[% RETURN UNLESS hackbot %] + +AI diff --git a/extensions/Hackbot/template/en/default/hook/bug_modal/activity_stream-before_comment_actions.html.tmpl b/extensions/Hackbot/template/en/default/hook/bug_modal/activity_stream-before_comment_actions.html.tmpl new file mode 100644 index 0000000000..253a385712 --- /dev/null +++ b/extensions/Hackbot/template/en/default/hook/bug_modal/activity_stream-before_comment_actions.html.tmpl @@ -0,0 +1,23 @@ +[%# 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. + #%] + +[% hackbot = comment.hackbot %] +[% RETURN UNLESS hackbot %] + + +
+ + + [% hackbot.agent FILTER html %] + + [% IF hackbot.url %] + View Run + [% END %] +
+ diff --git a/extensions/Hackbot/template/en/default/hook/bug_modal/comment_tags-before_tags.html.tmpl b/extensions/Hackbot/template/en/default/hook/bug_modal/comment_tags-before_tags.html.tmpl new file mode 100644 index 0000000000..82ae5d02fe --- /dev/null +++ b/extensions/Hackbot/template/en/default/hook/bug_modal/comment_tags-before_tags.html.tmpl @@ -0,0 +1,13 @@ +[%# 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. + #%] + +[% hackbot = comment.hackbot %] +[% RETURN UNLESS hackbot %] + +[%# Comment tags are case-insensitive. %] +[% hidden_tags.push(hackbot.agent.lower) %] diff --git a/extensions/Hackbot/template/en/default/hook/bug_modal/header-end.html.tmpl b/extensions/Hackbot/template/en/default/hook/bug_modal/header-end.html.tmpl new file mode 100644 index 0000000000..05fa23b00a --- /dev/null +++ b/extensions/Hackbot/template/en/default/hook/bug_modal/header-end.html.tmpl @@ -0,0 +1,9 @@ +[%# 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. + #%] + +[% style_urls.push('extensions/Hackbot/web/styles/hackbot.css') %] diff --git a/extensions/Hackbot/web/styles/hackbot.css b/extensions/Hackbot/web/styles/hackbot.css new file mode 100644 index 0000000000..2df99d21a3 --- /dev/null +++ b/extensions/Hackbot/web/styles/hackbot.css @@ -0,0 +1,54 @@ +/* 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. */ + +/* The header already names the agent, so its comment tag is only kept for search. */ +.comment-tag[hidden] { + display: none; +} + +.hackbot-meta { + display: flex; + overflow: hidden; + margin-inline: 8px; + border: 1px solid var(--primary-region-border-color); + border-radius: 16px; + width: max-content; + font-size: var(--font-size-small); +} + +.hackbot-meta .agent, +.hackbot-meta .run { + display: inline-flex; + gap: 4px; + align-items: center; + padding: 1px 8px; + min-height: 1.5em; +} + +.hackbot-meta .agent { + background-color: var(--control-background-color); +} + +.hackbot-meta .agent .icon::before { + color: rgb(var(--accent-color-yellow-1)); + font-family: var(--icon-font-family); + font-size: var(--icon-size-x-small); + line-height: 1; + vertical-align: middle; + font-variation-settings: 'FILL' 0, 'wght' 400, 'GRAD' 0, 'opsz' 24; + content: '\e65f'; +} + +@media screen and (width < 768px) { + .hackbot-meta { + justify-content: center; + flex-wrap: wrap; + border-radius: 8px; + width: auto; + min-width: 6em; + } +} diff --git a/extensions/Push/lib/Serialize.pm b/extensions/Push/lib/Serialize.pm index dd2d6a2b5e..f96fe79536 100644 --- a/extensions/Push/lib/Serialize.pm +++ b/extensions/Push/lib/Serialize.pm @@ -267,9 +267,15 @@ sub _attachment { sub _comment { my ($self, $comment, $is_shallow) = @_; + + # The text people read, as in bugmail: extensions can hide parts of it with + # the comment_body_full hook. body_full formats the text of other comment + # types (attachments, duplicates), so those keep the stored text. + my $body = $comment->type ? $comment->body : $comment->body_full; + my $rh = { id => _integer($comment->id), - body => _string($comment->body), + body => _string($body), creation_time => _time($comment->creation_ts), is_private => _boolean($comment->is_private), number => _integer($comment->count), diff --git a/qa/config/generate_test_data.pl b/qa/config/generate_test_data.pl index 16367f9bdf..1aab20cac7 100644 --- a/qa/config/generate_test_data.pl +++ b/qa/config/generate_test_data.pl @@ -132,7 +132,7 @@ BEGIN my @usernames = ( 'admin', 'unprivileged', 'QA_Selenium_TEST', 'canconfirm', 'tweakparams', 'permanent', 'editbugs', 'disabled', - 'editusers', 'editusers_target', 'creategroups', + 'editusers', 'editusers_target', 'creategroups', 'hackbot', ); print "creating user accounts...\n"; diff --git a/qa/config/selenium_test.conf b/qa/config/selenium_test.conf index 73976b8936..f5020b9e1b 100644 --- a/qa/config/selenium_test.conf +++ b/qa/config/selenium_test.conf @@ -58,6 +58,10 @@ 'creategroups_user_passwd' => 'bo6aazeKohch', 'creategroups_user_api_key' => 'ReStCrEaTeGrOuPsTestApiKey0123456789abcd', 'protected_group_name' => 'rest-protected-group-test', + 'hackbot_user_login' => 'hackbot@mozilla.test', + 'hackbot_user_passwd' => 'bo6aazeKohch', + 'hackbot_user_username' => 'Hackbot', + 'hackbot_user_api_key' => 'KRIY6KaNDW6sTRL0ej5CDdZVE1wUefLZqVzelRAE', 'tweakparams_user_login' => 'tweakparams@mozilla.test', 'tweakparams_user_login_truncated' => 'tweakparams@mo', 'tweakparams_user_passwd' => 'bo6aazeKohch', diff --git a/qa/t/1_test_hackbot_comments.t b/qa/t/1_test_hackbot_comments.t new file mode 100644 index 0000000000..c35da3ec18 --- /dev/null +++ b/qa/t/1_test_hackbot_comments.t @@ -0,0 +1,210 @@ +# 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. + +# Bug 2086124: Hackbot comments start with a front matter block that links them +# to the run that wrote them. hackbot-api posts them through REST, so they are +# posted the same way here, and the bug page is checked in the browser: the +# block is hidden, the header shows the agent and a link to the run, the +# agent's comment tag is hidden, and quoting the comment leaves the block out. +# The same block typed by anyone else is an ordinary comment. + +use strict; +use warnings; +use lib qw(lib ../../lib ../../local/lib/perl5); + +use QA::Util; +use QA::Tests qw(create_bug_fields); +use Test::More; +use Test::Mojo; + +my ($sel, $config) = get_selenium(); + +my $rest = "$config->{browser_url}/rest"; +my $t = Test::Mojo->new; + +sub api_key_header { + my ($user) = @_; + return {'X-Bugzilla-API-Key' => $config->{"${user}_user_api_key"}}; +} + +my $agent = 'frontend-triage'; + +# Comment tags are case-insensitive, so the agent's tag may differ in case. +my $agent_tag = 'Frontend-Triage'; + +# Matches the agent only if a tag were read as a pattern ('.' is any character). +my $lookalike_tag = 'frontend.triage'; +my $run = 'e2e-run-1'; +my $url = "https://hackbot.example/runs/$run"; +my $block = join("\n", + '---', 'hackbot:', " run: $run", + ' action: 3f2a9c1e', + " agent: $agent", + " url: $url", '---', '', ''); + +# Turn the integration on for the Hackbot test account, through the UI. +log_in($sel, $config, 'admin'); +set_parameters( + $sel, + { + 'Hackbot' => + {'hackbot_user' => {type => 'text', value => $config->{hackbot_user_login}},} + } +); + +my %bug = %{create_bug_fields($config)}; +delete $bug{creator}; +$t->post_ok( + "$rest/bug" => api_key_header('admin') => json => { + %bug, + summary => 'Hackbot comment metadata', + description => 'A bug for Hackbot to comment on', + } +)->status_is(200, 'The bug is filed'); +my $bug_id = $t->tx->res->json('/id'); + +sub add_comment { + my ($user, $text) = @_; + $t->post_ok("$rest/bug/$bug_id/comment" => api_key_header($user) => json => + {comment => $text, is_markdown => 1})->status_is(201, "$user adds a comment"); + return $t->tx->res->json('/id'); +} + +# The order matters: the field change comes right after the Hackbot comment, +# so a change header that picked up the comment before it would show the badge. +my $forged_id = add_comment('unprivileged', "${block}Typed by someone else."); +my $plain_id = add_comment('hackbot', 'A plain Hackbot comment.'); +my $hackbot_id = add_comment('hackbot', "${block}Triage result: front end."); + +$t->put_ok( + "$rest/bug/comment/$hackbot_id/tags" => api_key_header('admin') => json => { + comment_id => $hackbot_id, + add => [$agent_tag, $lookalike_tag, 'needs-review'] + } +)->status_is(200, 'The comment is tagged'); +$t->put_ok("$rest/bug/$bug_id" => api_key_header('admin') => json => + {whiteboard => 'hackbot-e2e'}) + ->status_is(200, 'A field is changed with no comment'); + +# What the page shows for a comment, read in one go. +sub comment_view { + my ($comment_id) = @_; + return $sel->driver->execute_script(<<"JS"); +const set = document.querySelector('.comment[data-id="$comment_id"]').closest('.change-set'); +const meta = set.querySelector('.hackbot-meta'); +const run = meta && meta.querySelector('.run'); +return { + text: set.querySelector('.comment-text').innerText.trim(), + agent: meta ? meta.querySelector('.agent').textContent.trim() : '', + run: run ? run.getAttribute('href') : '', + badge: [...set.querySelectorAll('.change-author .user-role')] + .some(role => role.textContent.trim() === 'AI'), + tags: Object.fromEntries([...set.querySelectorAll('.comment-tag')] + .map(tag => [tag.dataset.tag, getComputedStyle(tag).display !== 'none'])), +}; +JS +} + +# Poll the page until a JavaScript expression is true. +sub wait_for_js_ok { + my ($expression, $name) = @_; + for (1 .. 20) { + return ok(1, $name) if $sel->driver->execute_script("return !!($expression)"); + sleep 1; + } + return ok(0, $name); +} + +go_to_bug($sel, $bug_id, 1); + +my $view = comment_view($hackbot_id); +is($view->{text}, 'Triage result: front end.', 'The block is hidden'); +is($view->{agent}, $agent, 'The header names the agent'); +is($view->{run}, $url, 'The header links to the run'); +ok($view->{badge}, 'The author has the AI badge'); +ok(!$view->{tags}{$agent_tag}, 'The agent tag is hidden'); +ok($view->{tags}{'needs-review'}, 'Other tags are shown'); +ok($view->{tags}{$lookalike_tag}, + 'A tag that only looks like the agent is shown'); + +$view = comment_view($forged_id); +like( + $view->{text}, + qr/hackbot:.*run: $run/s, + "Another user's block is shown as text" +); +is($view->{agent}, '', '... with no agent in the header'); +ok(!$view->{badge}, '... and no AI badge'); + +$view = comment_view($plain_id); +is( + $view->{text}, + 'A plain Hackbot comment.', + 'A plain Hackbot comment is unchanged' +); +is($view->{agent}, '', '... with no agent in the header'); + +ok( + !$sel->driver->execute_script(<<'JS'), +return [...document.querySelectorAll('.change-set')] + .filter(set => !set.querySelector('.comment')) + .some(set => [...set.querySelectorAll('.change-author .user-role')] + .some(role => role.textContent.trim() === 'AI')); +JS + 'A field change has no AI badge' +); + +# Quoting the comment leaves the block out. +$sel->js_click_ok( + qq{//button[contains(\@class, "reply-btn") and \@data-id="$hackbot_id"]}, + 'Reply to the Hackbot comment'); +wait_for_js_ok( + q{document.querySelector('#comment').value.includes('> Triage result: front end.')}, + 'The reply quotes the comment' +); +unlike( + $sel->driver->execute_script( + q{return document.querySelector('#comment').value}), + qr/hackbot:|run: $run/, + 'The reply has no block' +); +$sel->driver->execute_script(q{document.querySelector('#comment').value = ''}); + +# Changing the tags renders them again, and the agent tag stays hidden. +my $tags = qq{.comment[data-id="$hackbot_id"] .comment-tag}; +$sel->js_click_ok( + qq{//button[contains(\@class, "tag-btn") and \@data-id="$hackbot_id"]}, + 'Open the tag editor'); +$sel->type_ok('ctag-add', 'e2e-added', 'Type a new tag'); +$sel->js_click_ok('ctag-save', 'Save the tag'); +wait_for_js_ok(qq{document.querySelector('$tags\[data-tag="e2e-added"]')}, + 'The new tag is shown'); +$view = comment_view($hackbot_id); +ok($view->{tags}{'e2e-added'}, 'The new tag is visible'); +ok(!$view->{tags}{$agent_tag}, 'The agent tag is still hidden'); + +$sel->js_click_ok( + qq{//div[contains(\@class, "comment") and \@data-id="$hackbot_id"]} + . q{//span[@data-tag="needs-review"]/a[contains(@class, "remove")]}, + 'Remove a tag' +); +wait_for_js_ok(qq{!document.querySelector('$tags\[data-tag="needs-review"]')}, + 'The tag is removed'); +ok(!comment_view($hackbot_id)->{tags}{$agent_tag}, + 'The agent tag is still hidden'); + +# Turn the integration off again: the block is shown as it is stored. +set_parameters($sel, + {'Hackbot' => {'hackbot_user' => {type => 'text', value => ''}}}); +go_to_bug($sel, $bug_id, 1); +$view = comment_view($hackbot_id); +like($view->{text}, qr/hackbot:.*agent: $agent/s, 'Off: the block is shown'); +is($view->{agent}, '', 'Off: no agent in the header'); + +logout($sel); + +done_testing(); diff --git a/skins/standard/global.css b/skins/standard/global.css index e5ab13c697..00356ebdef 100644 --- a/skins/standard/global.css +++ b/skins/standard/global.css @@ -112,6 +112,7 @@ --icon-size-large: 24px; --icon-size-medium: 20px; --icon-size-small: 16px; + --icon-size-x-small: 14px; /** Global */ --global-header-height: 48px; From 05a5f154b3beef7bfbdcc71ea19847a3544874c7 Mon Sep 17 00:00:00 2001 From: Kohei Yoshino Date: Mon, 9 Nov 2026 00:54:28 -0500 Subject: [PATCH 2/6] Bug 2086124 - Fix Perl::Critic violations in the Hackbot extension t/critic.t failed in CI: the extension added hand-written character classes, a `no warnings`, and a `$+[0]` above the counts in t/.perlcritic-history. - Use POSIX classes with /a, which match the same ASCII characters. - Capture the comment text in the front matter regex instead of reading $+[0]. - Drop `no warnings 'redefine'`: nothing defines Bugzilla::Comment::hackbot before the extension does. --- extensions/Hackbot/Extension.pm | 1 - extensions/Hackbot/lib/Metadata.pm | 14 ++++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/extensions/Hackbot/Extension.pm b/extensions/Hackbot/Extension.pm index 3db10a7784..e980cb176b 100644 --- a/extensions/Hackbot/Extension.pm +++ b/extensions/Hackbot/Extension.pm @@ -19,7 +19,6 @@ use Bugzilla::User; our $VERSION = '1'; BEGIN { - no warnings 'redefine'; *Bugzilla::Comment::hackbot = \&_comment_hackbot; } diff --git a/extensions/Hackbot/lib/Metadata.pm b/extensions/Hackbot/lib/Metadata.pm index c48fe7e17e..edfdff4617 100644 --- a/extensions/Hackbot/lib/Metadata.pm +++ b/extensions/Hackbot/lib/Metadata.pm @@ -27,9 +27,12 @@ our @EXPORT_OK = qw(parse_front_matter); # # It is only ever generated, so this reads that one shape rather than YAML in # general. Anything else is an ordinary comment that happens to start with a -# rule, and unknown keys are skipped so that fields can be added. -my $FRONT_MATTER = qr/\A---\nhackbot:\n((?: [a-z_]+: [^\n]*\n)+)---(?:\n+|\z)/; -my $IDENTIFIER = qr/\A[A-Za-z0-9._-]+\z/; +# rule, and unknown keys are skipped so that fields can be added. The /a flag +# keeps the character classes to ASCII. +my $FRONT_MATTER + = qr/\A---\nhackbot:\n((?: [[:lower:]_]+: [^\n]*\n)+)---(?:\n+|\z)(.*)\z/as; +my $FIELD = qr/^ ([[:lower:]_]+): ([^\n]*?)\s*$/am; +my $IDENTIFIER = qr/\A[[:alnum:]._-]+\z/a; # Returns the metadata hash and the comment text without the block, or an empty # list when the text doesn't start with a valid block. The url key is left out @@ -37,10 +40,9 @@ my $IDENTIFIER = qr/\A[A-Za-z0-9._-]+\z/; sub parse_front_matter { my ($text) = @_; - return unless defined $text && $text =~ $FRONT_MATTER; - my ($block, $body) = ($1, substr($text, $+[0])); + my ($block, $body) = ($text // '') =~ $FRONT_MATTER or return; - my %fields = $block =~ /^ ([a-z_]+): ([^\n]*?)\s*$/mg; + my %fields = $block =~ /$FIELD/g; my %metadata; foreach my $key (qw(run action agent)) { From 88e16522ccf571a9bdba756070fb7fad4b1819c9 Mon Sep 17 00:00:00 2001 From: Kohei Yoshino Date: Mon, 9 Nov 2026 00:54:28 -0500 Subject: [PATCH 3/6] Bug 2086124 - Read Hackbot metadata only from plain comments Attachment, duplicate and other typed comments are formatted without the comment_body_full hook, so a front matter block in them stayed visible while the header still showed the agent and hid its tag. Only plain comments are parsed now. The DB test adds an attachment with a block in its comment. --- extensions/Hackbot/Extension.pm | 6 ++++-- extensions/Hackbot/t/bmo/comment-metadata.t | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/extensions/Hackbot/Extension.pm b/extensions/Hackbot/Extension.pm index e980cb176b..19d108d9b8 100644 --- a/extensions/Hackbot/Extension.pm +++ b/extensions/Hackbot/Extension.pm @@ -32,13 +32,15 @@ sub _hackbot_user_id { } # Anyone can type a front matter block into a comment, so only the Hackbot -# account's comments are parsed. Returns the same list as parse_front_matter. +# account's comments are parsed, and only plain ones: the text of other types +# (attachments, duplicates) is formatted without the comment_body_full hook, so +# their block would stay visible. Returns the same list as parse_front_matter. # A page reads each comment several times (header, badge, tags, text), so the # result is kept on the comment for as long as its text is unchanged. sub _parse_comment { my ($comment) = @_; my $user_id = _hackbot_user_id() or return; - return unless ($comment->{who} // 0) == $user_id; + return if ($comment->{who} // 0) != $user_id || $comment->type; my $body = $comment->body; my $parsed = $comment->{hackbot_front_matter}; diff --git a/extensions/Hackbot/t/bmo/comment-metadata.t b/extensions/Hackbot/t/bmo/comment-metadata.t index edb7c77f67..8f388b7ee3 100644 --- a/extensions/Hackbot/t/bmo/comment-metadata.t +++ b/extensions/Hackbot/t/bmo/comment-metadata.t @@ -24,6 +24,7 @@ BEGIN { use Bugzilla; BEGIN { Bugzilla->extensions } use Bugzilla::App (); +use Bugzilla::Bug; use Bugzilla::Comment; use Bugzilla::Config qw(SetParam write_params); use Bugzilla::Constants qw(bz_locations); @@ -214,6 +215,25 @@ my $plain = post_comment($hackbot, 'A plain comment'); is($plain->body_full, 'A plain comment', 'plain Hackbot comment is unchanged'); is($plain->hackbot, undef, 'plain Hackbot comment has no metadata'); +# Attachment comments are shown through their own template, which doesn't hide +# the block, so it isn't read there either. +$t->post_ok( + "/rest/bug/$bug_id/attachment" => api_headers($hackbot) => json => { + ids => [$bug_id], + data => 'cGF0Y2g=', + file_name => 'fix.txt', + summary => 'fix', + content_type => 'text/plain', + comment => "$block$text", + } +)->status_is(201, 'the attachment is added') + or diag $t->tx->res->body; +my $attachment_comment + = Bugzilla::Bug->new($bug_id)->comments({order => 'oldest_to_newest'})->[-1]; +ok($attachment_comment->type, 'the attachment has its own comment type'); +is($attachment_comment->hackbot, undef, + 'an attachment comment has no metadata'); + # The hackbot_user param. my $check = \&Bugzilla::Extension::Hackbot::Config::check_hackbot_user; is($check->(''), '', 'an empty login is valid'); From d63923d1735b5a651a308a15fff3ce7ce4a7464c Mon Sep 17 00:00:00 2001 From: Kohei Yoshino Date: Mon, 9 Nov 2026 01:29:18 -0500 Subject: [PATCH 4/6] [skip ci] Fix icon vertical alignment --- extensions/Hackbot/web/styles/hackbot.css | 1 - 1 file changed, 1 deletion(-) diff --git a/extensions/Hackbot/web/styles/hackbot.css b/extensions/Hackbot/web/styles/hackbot.css index 2df99d21a3..6e95be70d6 100644 --- a/extensions/Hackbot/web/styles/hackbot.css +++ b/extensions/Hackbot/web/styles/hackbot.css @@ -37,7 +37,6 @@ color: rgb(var(--accent-color-yellow-1)); font-family: var(--icon-font-family); font-size: var(--icon-size-x-small); - line-height: 1; vertical-align: middle; font-variation-settings: 'FILL' 0, 'wght' 400, 'GRAD' 0, 'opsz' 24; content: '\e65f'; From 605a83c37c54f7212d8b3e09de70a116a450837e Mon Sep 17 00:00:00 2001 From: Kohei Yoshino Date: Tue, 10 Nov 2026 21:26:56 -0500 Subject: [PATCH 5/6] Bug 2086124 - Render the inline edit response the way the bug page does Saving an inline edit put the HTML of the raw text in the page, so a Hackbot comment showed its front matter block until reload. The response is now rendered from body_full, which applies the comment_body_full hook, like the bug page. --- extensions/EditComments/lib/WebService.pm | 8 ++++++-- extensions/Hackbot/t/bmo/comment-metadata.t | 13 ++++++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/extensions/EditComments/lib/WebService.pm b/extensions/EditComments/lib/WebService.pm index 74932a5154..23ec807f01 100644 --- a/extensions/EditComments/lib/WebService.pm +++ b/extensions/EditComments/lib/WebService.pm @@ -123,10 +123,14 @@ sub update_comment { sync_fulltext => 1, }); + # Render what the bug page shows, which can differ from the stored text: the + # comment_body_full hook lets extensions hide part of it (eg. Hackbot's front + # matter). + my $body = $comment->body_full({exclude_attachment => 1}); my $html = $comment->is_markdown && Bugzilla->params->{use_markdown} - ? Bugzilla->markdown->render_html($new_comment, $bug) - : Bugzilla::Template::quoteUrls($new_comment, $bug); + ? Bugzilla->markdown->render_html($body, $bug) + : Bugzilla::Template::quoteUrls($body, $bug); # Respond with the updated comment and number of revisions return { diff --git a/extensions/Hackbot/t/bmo/comment-metadata.t b/extensions/Hackbot/t/bmo/comment-metadata.t index 8f388b7ee3..af89e37ba8 100644 --- a/extensions/Hackbot/t/bmo/comment-metadata.t +++ b/extensions/Hackbot/t/bmo/comment-metadata.t @@ -42,7 +42,7 @@ use Test::Mojo; # request, so set them there, and put them back afterwards. Bugmail is sent # while the comment is posted and written to a file, so it can be read here. my %original_params = map { $_ => Bugzilla->params->{$_} // '' } - qw(hackbot_user use_mailer_queue mail_delivery_method); + qw(hackbot_user use_mailer_queue mail_delivery_method edit_comments_admins_group); sub set_params { my (%params) = @_; @@ -206,6 +206,17 @@ $comment->update_text({ is($comment->hackbot, undef, 'a removed block leaves no metadata'); is($comment->body_full, 'Edited, with no block.', '... and the text is whole'); +# Saving an inline edit returns the HTML the page shows, without the block. +# Admins edit other people's comments as edit-comments admins. +set_params(edit_comments_admins_group => 'admin'); +my $edited = post_comment($hackbot, "$block$text"); +my $edit_url = '/rest/editcomments/comment/' . $edited->id; +$t->put_ok($edit_url => api_headers($admin) => json => + {new_comment => "${block}Edited text."})->status_is(200); +my $edit_html = $t->tx->res->json('/html'); +unlike($edit_html, qr/hackbot:|run: abc-123/, 'the edit response has no block'); +like($edit_html, qr/Edited text\./, '... and has the new text'); + # Anyone can type the block. my $forged = post_comment($other, "$block$text"); is($forged->body_full, "$block$text", 'another user: the block stays as text'); From 892758fd90a4e04d765649a272555086a1e75502 Mon Sep 17 00:00:00 2001 From: Kohei Yoshino Date: Tue, 10 Nov 2026 21:38:41 -0500 Subject: [PATCH 6/6] [skip ci] Match hackbot.css break point with bug_modal.css --- extensions/Hackbot/web/styles/hackbot.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/Hackbot/web/styles/hackbot.css b/extensions/Hackbot/web/styles/hackbot.css index 6e95be70d6..cb8f21be93 100644 --- a/extensions/Hackbot/web/styles/hackbot.css +++ b/extensions/Hackbot/web/styles/hackbot.css @@ -42,7 +42,7 @@ content: '\e65f'; } -@media screen and (width < 768px) { +@media screen and (max-width: 799px) { .hackbot-meta { justify-content: center; flex-wrap: wrap;