From cc7887fd4fea76fdfafcea11b4d1bb45f642a3bc Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Tue, 13 Oct 2026 12:14:39 +0200 Subject: [PATCH 1/2] Bug 2081207 - Add a Crash Data URL field for the graph and table Use cf_crash_data_url for Socorro filters in both widgets, replacing bug_file_loc as the table's filter source. Require the Socorro search URL prefix and a nonempty crash signature. Use MEDIUMTEXT for long URLs. --- Bugzilla/Template.pm | 11 +- extensions/BMO/Extension.pm | 53 ++++++ extensions/BMO/lib/Data.pm | 2 +- ...-custom_field-cf_crash_signature.html.tmpl | 42 ++++- .../hook/global/user-error-errors.html.tmpl | 9 + extensions/BMO/web/js/firefox-crash-table.js | 17 +- extensions/BMO/web/socorro-lens/main.js | 22 ++- .../en/default/bug_modal/create.html.tmpl | 2 +- .../en/default/bug_modal/edit.html.tmpl | 2 +- qa/t/rest_bug_crash_data_url.t | 164 ++++++++++++++++++ 10 files changed, 302 insertions(+), 22 deletions(-) create mode 100644 qa/t/rest_bug_crash_data_url.t diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm index adf35a64b3..114bb26cb7 100644 --- a/Bugzilla/Template.pm +++ b/Bugzilla/Template.pm @@ -1047,16 +1047,23 @@ sub create { 'default_authorizer' => sub { return Bugzilla::Auth->new() }, 'socorro_lens_url' => sub { - my ($sigs) = @_; + my ($sigs, $crash_data_url) = @_; # strip [@ ] from sigs my @sigs = map {/^\[\@\s*(.+?)\s*\]$/} @$sigs; return '' unless @sigs; + my @query = ('id' => 'socorro-lens.html', 's' => join("\\", @sigs)); + + # Pass the search URL to socorro-lens for filtering. + push @query, 'url' => $crash_data_url + if defined $crash_data_url + && $crash_data_url =~ m{^https://crash-stats\.mozilla\.org/search/\?}; + # use a URI object to encode the query string part. my $uri = URI->new(Bugzilla->localconfig->urlbase . 'page.cgi'); - $uri->query_form({'id' => 'socorro-lens.html', 's' => join("\\", @sigs)}); + $uri->query_form(\@query); return $uri; }, }, diff --git a/extensions/BMO/Extension.pm b/extensions/BMO/Extension.pm index 5fc433392f..cf5b40c149 100644 --- a/extensions/BMO/Extension.pm +++ b/extensions/BMO/Extension.pm @@ -782,6 +782,34 @@ sub object_columns { ); } +sub object_validators { + my ($self, $args) = @_; + my ($class, $validators) = @$args{qw(class validators)}; + return unless $class->isa('Bugzilla::Bug'); + $validators->{cf_crash_data_url} = \&_check_crash_data_url; +} + +sub _check_crash_data_url { + my ($invocant, $url) = @_; + my $prefix = 'https://crash-stats.mozilla.org/search/?'; + $url = defined $url ? trim($url) : ''; + return $url if $url eq '' || index($url, $prefix) == 0; + ThrowUserError('crash_data_url_invalid', {url => $url, prefix => $prefix}); +} + +# Check after both fields have been validated so simultaneous edits work. +sub _check_crash_data_url_has_signature { + my ($signature, $url) = @_; + ThrowUserError('crash_data_url_without_signature') + if ($url // '') ne '' && ($signature // '') eq ''; +} + +sub webservice_error_codes { + my ($self, $args) = @_; + $args->{error_map}->{crash_data_url_invalid} = 112; # Bug URL errors + $args->{error_map}->{crash_data_url_without_signature} = 112; +} + sub object_update_columns { my ($self, $args) = @_; return unless $args->{object}->isa('Bugzilla::Product'); @@ -1111,6 +1139,9 @@ sub bug_end_of_create_validators { my ($self, $args) = @_; my $params = $args->{params}; + _check_crash_data_url_has_signature($params->{cf_crash_signature}, + $params->{cf_crash_data_url}); + # silently drop changes to the triaged keyword from users without permissions # to triage return if Bugzilla->user->can_triage; @@ -1148,6 +1179,14 @@ sub object_start_of_update { my ($new_bug, $old_bug) = @$args{qw( object old_object )}; return unless $new_bug->isa('Bugzilla::Bug'); + # Allow unrelated edits even if an existing URL lacks a signature. + if (any { ($new_bug->$_ // '') ne ($old_bug->$_ // '') } + qw(cf_crash_signature cf_crash_data_url)) + { + _check_crash_data_url_has_signature($new_bug->cf_crash_signature, + $new_bug->cf_crash_data_url); + } + # if either the assignee or status has changed return unless $old_bug->assigned_to->id != $new_bug->assigned_to->id @@ -1463,6 +1502,20 @@ sub install_update_db { }); } + # Use MEDIUMTEXT to support search URLs longer than 255 characters. + unless (Bugzilla::Field->new({name => 'cf_crash_data_url'})) { + Bugzilla::Field->create({ + name => 'cf_crash_data_url', + description => 'Crash Data URL', + type => FIELD_TYPE_TEXTAREA, + mailhead => 0, + enter_bug => 1, + obsolete => 0, + custom => 1, + buglist => 0, + }); + } + # Add default security group id column if (!$dbh->bz_column_info('products', 'security_group_id')) { $dbh->bz_add_column( diff --git a/extensions/BMO/lib/Data.pm b/extensions/BMO/lib/Data.pm index 23018f0b2c..8e4deb800b 100644 --- a/extensions/BMO/lib/Data.pm +++ b/extensions/BMO/lib/Data.pm @@ -107,7 +107,7 @@ tie( "Infrastructure & Operations" => ["RelOps", "RelOps: Puppet", "DCOps",], }, qr/^cf_office$/ => {"mozilla.org" => ["Server Operations: Desktop Issues"],}, - qr/^cf_crash_signature$/ => { + qr/^cf_crash_(?:signature|data_url)$/ => { "Add-on SDK" => [], "addons.mozilla.org" => [], "Android Background Services" => [], diff --git a/extensions/BMO/template/en/default/hook/bug_modal/edit-custom_field-cf_crash_signature.html.tmpl b/extensions/BMO/template/en/default/hook/bug_modal/edit-custom_field-cf_crash_signature.html.tmpl index 630ba23273..d163d50e17 100644 --- a/extensions/BMO/template/en/default/hook/bug_modal/edit-custom_field-cf_crash_signature.html.tmpl +++ b/extensions/BMO/template/en/default/hook/bug_modal/edit-custom_field-cf_crash_signature.html.tmpl @@ -27,14 +27,46 @@ [% END %] [% END %] - [%# socorro_lens_url() is defined in Bugzilla/Template.pm %] - [% SET lens_url = socorro_lens_url(split_cf_crash_signature) %] - [% IF lens_url %] - - [% END %] [% ELSIF value %] [% value FILTER html FILTER html_line_break %] [% ELSE%] None [% END %] [% END %] + +[% + rendered_custom_fields.push('cf_crash_data_url'); + WRAPPER bug_modal/field.html.tmpl + label = "URL" + help = "https://wiki.mozilla.org/BMO/UserGuide/BugFields#crash_data_url" + field = bug_fields.cf_crash_data_url + field_type = constants.FIELD_TYPE_FREETEXT + hide_on_view = bug.cf_crash_data_url == "" + default_value = "https://crash-stats.mozilla.org/search/?…" +%] + [%# firefox-crash-table.js reads this attribute independently of the link. %] + + [%# is_safe_url() rejects valid URLs ending in ")". %] + [% IF bug.cf_crash_data_url.match('^https://crash-stats\.mozilla\.org/search/\?') %] + + [%~ bug.cf_crash_data_url FILTER pretty_url FILTER truncate(40) FILTER html ~%] + + [% ELSE %] + [% bug.cf_crash_data_url FILTER html %] + [% END %] + +[% END %] + +[% IF split_cf_crash_signature.size %] + [%# socorro_lens_url() is defined in Bugzilla/Template.pm %] + [% SET lens_url = socorro_lens_url(split_cf_crash_signature, bug.cf_crash_data_url) %] + [% IF lens_url %] +
+
+
+ +
+
+ [% END %] +[% END %] diff --git a/extensions/BMO/template/en/default/hook/global/user-error-errors.html.tmpl b/extensions/BMO/template/en/default/hook/global/user-error-errors.html.tmpl index 34ba287a33..6bb0b65a06 100644 --- a/extensions/BMO/template/en/default/hook/global/user-error-errors.html.tmpl +++ b/extensions/BMO/template/en/default/hook/global/user-error-errors.html.tmpl @@ -60,4 +60,13 @@ [% title = "Missing Bug Severity" %] Severity of S1-S4 is required to mark a [% terms.bug %] as triaged. +[% ELSIF error == "crash_data_url_invalid" %] + [% title = "Invalid Crash Data URL" %] + [% url FILTER html %] is not a valid Crash Data URL: it must be a + Socorro search URL starting with [% prefix FILTER html %]. + +[% ELSIF error == "crash_data_url_without_signature" %] + [% title = "Missing Crash Signature" %] + A Crash Data URL can only be set on [% terms.abug %] with a crash signature. + [% END %] diff --git a/extensions/BMO/web/js/firefox-crash-table.js b/extensions/BMO/web/js/firefox-crash-table.js index 1f547cfe10..8e6a31d961 100644 --- a/extensions/BMO/web/js/firefox-crash-table.js +++ b/extensions/BMO/web/js/firefox-crash-table.js @@ -131,19 +131,16 @@ window.addEventListener('DOMContentLoaded', () => { }); const extraSocorroArgs = [] - const baseUrl = "https://crash-stats.mozilla.org/search/"; + const baseUrl = "https://crash-stats.mozilla.org/search/?"; const sayNo = new Set(["_columns", "_facets", "_facets_size", "_sort", "_results_number", "date", "channel", "product", "version", "build_id"]); - const urlSelector = oldWay ? "bz_url_edit_container" : "field-value-bug_file_loc"; - document.querySelectorAll("#" + urlSelector + " a").forEach(a => { - if (a.href.startsWith(baseUrl)) { - const params = new URLSearchParams(a.href.slice(baseUrl.length)); - for (let p of params) { - if (!sayNo.has(p[0])) { - extraSocorroArgs.push(p[0] + "=" + encodeURIComponent(p[1])); - } + const crashDataUrl = document.querySelector("#field-value-cf_crash_data_url [data-crash-data-url]")?.dataset.crashDataUrl; + if (crashDataUrl?.startsWith(baseUrl)) { + for (const [key, value] of new URL(crashDataUrl).searchParams) { + if (!sayNo.has(key)) { + extraSocorroArgs.push(encodeURIComponent(key) + "=" + encodeURIComponent(value)); } } - }); + } const hgurlPattern = new RegExp("^http[s]?://hg\\.mozilla\\.org/(?:releases/)?mozilla-([^/]*)/rev/([0-9a-f]+)$"); const esrPattern = new RegExp("^esr[0-9]+$"); const repos = new Set(["central", "beta", "release"]); diff --git a/extensions/BMO/web/socorro-lens/main.js b/extensions/BMO/web/socorro-lens/main.js index 9a32a58dbb..1511940a37 100644 --- a/extensions/BMO/web/socorro-lens/main.js +++ b/extensions/BMO/web/socorro-lens/main.js @@ -247,6 +247,23 @@ function getSignaturesFromURL(search, match) { return result; } +// Signatures come from the bug; dates use the graph's fixed range. +var IGNORED_SEARCH_PARAMS = ["signature", "date", "channel", "product", "version", "build_id"]; + +function getExtraParamsFromURL(search) { + var url = (new URLSearchParams(search)).get('url'); + if (!url || !url.startsWith("https://crash-stats.mozilla.org/search/?")) { + return ""; + } + var extra = ""; + (new URL(url)).searchParams.forEach(function (value, key) { + if (!key.startsWith("_") && !IGNORED_SEARCH_PARAMS.includes(key)) { + extra = extra + "&" + encodeURIComponent(key) + "=" + encodeURIComponent(value); + } + }); + return extra; +} + function draw() { var channel = document.querySelector('select[name="channel"]').selectedOptions[0].value; items = MG.convert.date(items, 'date'); @@ -304,6 +321,7 @@ function redraw(channel) { function loadGraph(search, match = 'exact') { // Get all signatures from the Bugzilla page var signatures = getSignaturesFromURL(search, match); + var extra = getExtraParamsFromURL(search); // Initialize chart data // NOTE: Our Socorro query will request 179 days' worth of data: starting // 180 calendar-days ago, ending at (**and not including**) yesterday. @@ -335,9 +353,9 @@ function loadGraph(search, match = 'exact') { var processed_data = 0; // Set the report URL, this will be used to load the data report on click if (!globals.url[i]) globals.url[i] = ""; - globals.url[i] = globals.url_base + signatures[i]; + globals.url[i] = globals.url_base + signatures[i] + extra; // Iterate through the Socorro data and create the chart data object - var url = "https://crash-stats.mozilla.org/api/SuperSearch/?" + signatures[i] + "&date=%3E%3D" + start_date + "&date=%3C%3D" + end_date + "&_histogram.date=release_channel&_histogram_interval=1d&_results_number=0"; + var url = "https://crash-stats.mozilla.org/api/SuperSearch/?" + signatures[i] + extra + "&date=%3E%3D" + start_date + "&date=%3C%3D" + end_date + "&_histogram.date=release_channel&_histogram_interval=1d&_results_number=0"; url = url.replace("/?&signature", "/?signature"); d3.json(url, function (data) { if (data.total > 0) { diff --git a/extensions/BugModal/template/en/default/bug_modal/create.html.tmpl b/extensions/BugModal/template/en/default/bug_modal/create.html.tmpl index c0d691c36d..2339588948 100644 --- a/extensions/BugModal/template/en/default/bug_modal/create.html.tmpl +++ b/extensions/BugModal/template/en/default/bug_modal/create.html.tmpl @@ -640,7 +640,7 @@ [% FOREACH field IN custom_fields; NEXT IF field.type != constants.FIELD_TYPE_TEXTAREA; - NEXT IF field.name == "cf_crash_signature"; # Rendered below + NEXT IF field.name == "cf_crash_signature" || field.name == "cf_crash_data_url"; # Rendered below Hook.process('custom_field-' _ field.name); NEXT IF rendered_custom_fields.exists(field.name); INCLUDE bug_modal/field.html.tmpl diff --git a/extensions/BugModal/template/en/default/bug_modal/edit.html.tmpl b/extensions/BugModal/template/en/default/bug_modal/edit.html.tmpl index b15faae4c6..7fc6083c98 100644 --- a/extensions/BugModal/template/en/default/bug_modal/edit.html.tmpl +++ b/extensions/BugModal/template/en/default/bug_modal/edit.html.tmpl @@ -1255,7 +1255,7 @@ [% FOREACH field IN custom_fields; NEXT IF field.type != constants.FIELD_TYPE_TEXTAREA; - NEXT IF field.name == "cf_crash_signature"; # Rendered below + NEXT IF field.name == "cf_crash_signature" || field.name == "cf_crash_data_url"; # Rendered below Hook.process('custom_field-' _ field.name); NEXT IF rendered_custom_fields.exists(field.name); INCLUDE bug_modal/field.html.tmpl diff --git a/qa/t/rest_bug_crash_data_url.t b/qa/t/rest_bug_crash_data_url.t new file mode 100644 index 0000000000..931c587ab0 --- /dev/null +++ b/qa/t/rest_bug_crash_data_url.t @@ -0,0 +1,164 @@ +#!/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. + +# REST validation and bug-page rendering for cf_crash_data_url. + +use 5.10.1; +use strict; +use warnings; +use lib qw(lib ../../lib ../../local/lib/perl5); + +use Bugzilla; +use QA::Util qw(get_config); +use QA::Tests qw(create_bug_fields); +use QA::REST::Util qw(api_headers); + +use Mojo::URL; +use Test::Mojo; +use Test::More; + +use constant SIGNATURE => '[@ OOM | small]'; +use constant CRASH_URL => + 'https://crash-stats.mozilla.org/search/?signature=%3DOOM%20%7C%20small&platform=Linux'; + +# Regression case: is_safe_url() rejects URLs ending in ")". +use constant PAREN_URL => + 'https://crash-stats.mozilla.org/search/?moz_crash_reason=~MOZ_CRASH()'; + +my $config = get_config(); +my $api_key = $config->{editbugs_user_api_key}; +my $url = Bugzilla->localconfig->urlbase; + +my $t = Test::Mojo->new(); + +# Firefox exposes both crash fields. +sub new_crash_bug { + my (%fields) = @_; + my $bug = create_bug_fields($config); + $bug->{product} = 'Firefox'; + $bug->{component} = 'General'; + $bug->{version} = 'unspecified'; + $bug->{target_milestone} = '---'; + return {%$bug, %fields}; +} + +sub check_crash_fields { + my ($bug_id, $signature, $crash_url, $desc) = @_; + $t->get_ok($url + . "rest/bug/$bug_id?include_fields=cf_crash_signature,cf_crash_data_url" => + api_headers($api_key))->status_is(200) + ->json_is('/bugs/0/cf_crash_signature', $signature, + "$desc: signature has the right value") + ->json_is('/bugs/0/cf_crash_data_url', $crash_url, + "$desc: crash data url has the right value"); +} + +sub update_is_rejected { + my ($bug_id, $fields, $message, $desc) = @_; + $t->put_ok($url . "rest/bug/$bug_id" => api_headers($api_key) => json => $fields) + ->status_is(400)->json_is('/code' => 112) + ->json_like('/message' => $message, "$desc is rejected"); +} + +my $invalid_url = qr/is not a valid Crash Data URL/; +my $no_signature + = qr/A Crash Data URL can only be set on a bug with a crash signature/; + +# Creation. + +$t->post_ok($url + . 'rest/bug' => api_headers($api_key) => json => + new_crash_bug(cf_crash_signature => SIGNATURE, cf_crash_data_url => CRASH_URL)) + ->status_is(200)->json_has('/id'); +my $bug_id = $t->tx->res->json->{id}; + +check_crash_fields($bug_id, SIGNATURE, CRASH_URL, 'After create'); + +$t->post_ok($url + . 'rest/bug' => api_headers($api_key) => json => + new_crash_bug(cf_crash_data_url => CRASH_URL))->status_is(400) + ->json_is('/code' => 112) + ->json_like('/message' => $no_signature, + 'A crash data url without a signature is rejected on create'); + +$t->post_ok($url + . 'rest/bug' => api_headers($api_key) => json => new_crash_bug( + cf_crash_signature => SIGNATURE, + cf_crash_data_url => 'https://example.com/search/?platform=Linux' + ) +)->status_is(400)->json_is('/code' => 112) + ->json_like('/message' => $invalid_url, + 'A non Socorro search url is rejected on create'); + +# URL validation and trimming. + +foreach my $bad_url ( + 'http://crash-stats.mozilla.org/search/?platform=Linux', + 'https://crash-stats.mozilla.org/signature/?signature=foo', + 'javascript:alert(1)', + ) +{ + update_is_rejected($bug_id, {cf_crash_data_url => $bad_url}, + $invalid_url, $bad_url); +} + +check_crash_fields($bug_id, SIGNATURE, CRASH_URL, 'After rejected urls'); + +$t->put_ok($url + . "rest/bug/$bug_id" => api_headers($api_key) => json => + {cf_crash_data_url => ' ' . PAREN_URL . ' '})->status_is(200); + +check_crash_fields($bug_id, SIGNATURE, PAREN_URL, 'After update (trimmed)'); + +# URL rendering for the table and graph. + +$t->get_ok($url . "show_bug.cgi?id=$bug_id")->status_is(200) + ->attr_is('#field-value-cf_crash_data_url [data-crash-data-url]', + 'data-crash-data-url', PAREN_URL, 'The crash table gets the crash data url') + ->attr_is('#field-value-cf_crash_data_url a', 'href', PAREN_URL, + 'The crash data url is a link, even when ending with ")"'); + +my $graph_src = $t->tx->res->dom->at('#chart')->attr('src'); +is(Mojo::URL->new($graph_src)->query->param('url'), + PAREN_URL, 'The crash graph gets the crash data url'); + +# Require a signature when the URL is set. + +update_is_rejected($bug_id, {cf_crash_signature => ''}, + $no_signature, 'Removing the signature but not the crash data url'); + +check_crash_fields($bug_id, SIGNATURE, PAREN_URL, 'After rejected update'); + +$t->put_ok($url + . "rest/bug/$bug_id" => api_headers($api_key) => json => + {cf_crash_signature => '', cf_crash_data_url => ''})->status_is(200); + +check_crash_fields($bug_id, '', '', 'After removing both'); + +update_is_rejected($bug_id, {cf_crash_data_url => CRASH_URL}, + $no_signature, 'Setting a crash data url without a signature'); + +$t->put_ok($url + . "rest/bug/$bug_id" => api_headers($api_key) => json => + {cf_crash_signature => SIGNATURE, cf_crash_data_url => CRASH_URL}) + ->status_is(200); + +check_crash_fields($bug_id, SIGNATURE, CRASH_URL, 'After setting both'); + +# Clearing the URL removes the graph's extra filters. + +$t->put_ok($url + . "rest/bug/$bug_id" => api_headers($api_key) => json => + {cf_crash_data_url => ''})->status_is(200); + +$t->get_ok($url . "show_bug.cgi?id=$bug_id")->status_is(200); +$graph_src = $t->tx->res->dom->at('#chart')->attr('src'); +ok(!defined Mojo::URL->new($graph_src)->query->param('url'), + 'The crash graph is not filtered without a crash data url'); + +done_testing(); From 3e68e6589aca27eb597e2d94570071b279e087d0 Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Wed, 14 Oct 2026 09:30:45 +0200 Subject: [PATCH 2/2] Bug 2081207 - Don't break bug updates before checksetup creates cf_crash_data_url The new code is deployed before checksetup.pl runs, and until then Bugzilla::Bug has no cf_crash_data_url accessor, so the signature check in object_start_of_update made every bug update die. --- Bugzilla/Bug.pm | 5 +++ extensions/BMO/Extension.pm | 33 +++++++++++++---- .../hook/bug/show-header-end.html.tmpl | 1 + .../hook/bug_modal/header-end.html.tmpl | 1 + .../en/default/pages/socorro-lens.html.tmpl | 1 + extensions/BMO/web/js/crash-data-url.js | 37 +++++++++++++++++++ extensions/BMO/web/js/firefox-crash-table.js | 17 ++++----- extensions/BMO/web/socorro-lens/main.js | 16 ++------ qa/t/rest_bug_crash_data_url.t | 17 +++++++++ 9 files changed, 97 insertions(+), 31 deletions(-) create mode 100644 extensions/BMO/web/js/crash-data-url.js diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 062cb43d45..db748d74d2 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -2740,6 +2740,11 @@ sub set_all { # do that here, because if they *did* change the assignee, qa, or CC, # then we don't want to check the original ones, only the new ones. $self->_check_strict_isolation() if $product_changed; + + # BMO - allow extensions to validate the final values. Unlike + # object_end_of_set_all, this runs after custom fields have been set. + Bugzilla::Hook::process('bug_end_of_set_all', + {bug => $self, params => $params}); } # Helper for set_all that helps with fields that have an "add/remove" diff --git a/extensions/BMO/Extension.pm b/extensions/BMO/Extension.pm index cf5b40c149..630357195f 100644 --- a/extensions/BMO/Extension.pm +++ b/extensions/BMO/Extension.pm @@ -1172,6 +1172,31 @@ sub db_sanitize { Bugzilla->dbh->do("TRUNCATE TABLE bug_user_agent"); } +sub bug_start_of_set_all { + my ($self, $args) = @_; + my $bug = $args->{bug}; + + # Remember the crash fields so bug_end_of_set_all can tell if they changed. + # The accessor only exists once checksetup.pl has created the field. + $bug->{_crash_fields_before_set_all} + = [map { $bug->$_ // '' } qw(cf_crash_signature cf_crash_data_url)] + if $bug->can('cf_crash_data_url'); +} + +# Check here rather than in update() so that a mass edit is rejected before +# any bug is written. +sub bug_end_of_set_all { + my ($self, $args) = @_; + my $bug = $args->{bug}; + my $before = delete $bug->{_crash_fields_before_set_all} or return; + my ($signature, $url) + = map { $bug->$_ // '' } qw(cf_crash_signature cf_crash_data_url); + + # Allow unrelated edits even if an existing URL lacks a signature. + return if $signature eq $before->[0] && $url eq $before->[1]; + _check_crash_data_url_has_signature($signature, $url); +} + # bugs in an ASSIGNED state must be assigned to a real person # reset bugs to NEW if the assignee is nobody/.bugs$ sub object_start_of_update { @@ -1179,14 +1204,6 @@ sub object_start_of_update { my ($new_bug, $old_bug) = @$args{qw( object old_object )}; return unless $new_bug->isa('Bugzilla::Bug'); - # Allow unrelated edits even if an existing URL lacks a signature. - if (any { ($new_bug->$_ // '') ne ($old_bug->$_ // '') } - qw(cf_crash_signature cf_crash_data_url)) - { - _check_crash_data_url_has_signature($new_bug->cf_crash_signature, - $new_bug->cf_crash_data_url); - } - # if either the assignee or status has changed return unless $old_bug->assigned_to->id != $new_bug->assigned_to->id diff --git a/extensions/BMO/template/en/default/hook/bug/show-header-end.html.tmpl b/extensions/BMO/template/en/default/hook/bug/show-header-end.html.tmpl index 9b806a3760..8f0a016b0c 100644 --- a/extensions/BMO/template/en/default/hook/bug/show-header-end.html.tmpl +++ b/extensions/BMO/template/en/default/hook/bug/show-header-end.html.tmpl @@ -8,6 +8,7 @@ [% style_urls.push('extensions/BMO/web/styles/edit_bug.css') %] [% javascript_urls.push('extensions/BMO/web/js/edit_bug.js') %] +[% javascript_urls.push("extensions/BMO/web/js/crash-data-url.js") %] [% javascript_urls.push("extensions/BMO/web/js/firefox-crash-table.js") %] [% IF user.in_group('bounty-team') %] [% javascript_urls.push('extensions/BMO/web/js/attachment_bounty_form.js') %] diff --git a/extensions/BMO/template/en/default/hook/bug_modal/header-end.html.tmpl b/extensions/BMO/template/en/default/hook/bug_modal/header-end.html.tmpl index 1192e9c5e8..d91d1d6124 100644 --- a/extensions/BMO/template/en/default/hook/bug_modal/header-end.html.tmpl +++ b/extensions/BMO/template/en/default/hook/bug_modal/header-end.html.tmpl @@ -7,4 +7,5 @@ #%] [% style_urls.push("extensions/BMO/web/styles/bug_modal.css") %] +[% javascript_urls.push("extensions/BMO/web/js/crash-data-url.js") %] [% javascript_urls.push("extensions/BMO/web/js/firefox-crash-table.js") %] diff --git a/extensions/BMO/template/en/default/pages/socorro-lens.html.tmpl b/extensions/BMO/template/en/default/pages/socorro-lens.html.tmpl index 85a49b1089..a7ab2d3dfa 100644 --- a/extensions/BMO/template/en/default/pages/socorro-lens.html.tmpl +++ b/extensions/BMO/template/en/default/pages/socorro-lens.html.tmpl @@ -8,6 +8,7 @@ + diff --git a/extensions/BMO/web/js/crash-data-url.js b/extensions/BMO/web/js/crash-data-url.js new file mode 100644 index 0000000000..afce08a6ae --- /dev/null +++ b/extensions/BMO/web/js/crash-data-url.js @@ -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/. */ + +/** + * Extract the Socorro filters of a Crash Data URL. + * Shared by the crash graph (socorro-lens) and the crash table so that both + * apply the same filters. + */ +var CrashDataUrl = (function () { + "use strict"; + + var SEARCH_URL = "https://crash-stats.mozilla.org/search/?"; + + // Signatures come from the bug, and each widget sets its own date range, + // channels and versions. Parameters starting with "_" are display options. + var IGNORED_PARAMS = ["signature", "date", "channel", "product", "version", "build_id"]; + + // Return the filters as an array of encoded "key=value" strings. + function getFilters(url) { + var filters = []; + if (!url || !url.startsWith(SEARCH_URL)) { + return filters; + } + (new URL(url)).searchParams.forEach(function (value, key) { + if (!key.startsWith("_") && !IGNORED_PARAMS.includes(key)) { + filters.push(encodeURIComponent(key) + "=" + encodeURIComponent(value)); + } + }); + return filters; + } + + return { + SEARCH_URL: SEARCH_URL, + getFilters: getFilters, + }; +})(); diff --git a/extensions/BMO/web/js/firefox-crash-table.js b/extensions/BMO/web/js/firefox-crash-table.js index 8e6a31d961..83104f5e3a 100644 --- a/extensions/BMO/web/js/firefox-crash-table.js +++ b/extensions/BMO/web/js/firefox-crash-table.js @@ -130,17 +130,14 @@ window.addEventListener('DOMContentLoaded', () => { productDetails = getMajors(data); }); - const extraSocorroArgs = [] - const baseUrl = "https://crash-stats.mozilla.org/search/?"; - const sayNo = new Set(["_columns", "_facets", "_facets_size", "_sort", "_results_number", "date", "channel", "product", "version", "build_id"]); - const crashDataUrl = document.querySelector("#field-value-cf_crash_data_url [data-crash-data-url]")?.dataset.crashDataUrl; - if (crashDataUrl?.startsWith(baseUrl)) { - for (const [key, value] of new URL(crashDataUrl).searchParams) { - if (!sayNo.has(key)) { - extraSocorroArgs.push(encodeURIComponent(key) + "=" + encodeURIComponent(value)); - } - } + let crashDataUrl = document.querySelector("#field-value-cf_crash_data_url [data-crash-data-url]")?.dataset.crashDataUrl; + if (!crashDataUrl) { + // Older bugs have their Socorro search URL in the URL field. + const urlSelector = oldWay ? "bz_url_edit_container" : "field-value-bug_file_loc"; + crashDataUrl = [...document.querySelectorAll("#" + urlSelector + " a")] + .find(a => a.href.startsWith(CrashDataUrl.SEARCH_URL))?.href; } + const extraSocorroArgs = CrashDataUrl.getFilters(crashDataUrl); const hgurlPattern = new RegExp("^http[s]?://hg\\.mozilla\\.org/(?:releases/)?mozilla-([^/]*)/rev/([0-9a-f]+)$"); const esrPattern = new RegExp("^esr[0-9]+$"); const repos = new Set(["central", "beta", "release"]); diff --git a/extensions/BMO/web/socorro-lens/main.js b/extensions/BMO/web/socorro-lens/main.js index 1511940a37..afe88da85f 100644 --- a/extensions/BMO/web/socorro-lens/main.js +++ b/extensions/BMO/web/socorro-lens/main.js @@ -247,21 +247,11 @@ function getSignaturesFromURL(search, match) { return result; } -// Signatures come from the bug; dates use the graph's fixed range. -var IGNORED_SEARCH_PARAMS = ["signature", "date", "channel", "product", "version", "build_id"]; - function getExtraParamsFromURL(search) { var url = (new URLSearchParams(search)).get('url'); - if (!url || !url.startsWith("https://crash-stats.mozilla.org/search/?")) { - return ""; - } - var extra = ""; - (new URL(url)).searchParams.forEach(function (value, key) { - if (!key.startsWith("_") && !IGNORED_SEARCH_PARAMS.includes(key)) { - extra = extra + "&" + encodeURIComponent(key) + "=" + encodeURIComponent(value); - } - }); - return extra; + return CrashDataUrl.getFilters(url).map(function (filter) { + return "&" + filter; + }).join(""); } function draw() { diff --git a/qa/t/rest_bug_crash_data_url.t b/qa/t/rest_bug_crash_data_url.t index 931c587ab0..d680176fe8 100644 --- a/qa/t/rest_bug_crash_data_url.t +++ b/qa/t/rest_bug_crash_data_url.t @@ -150,6 +150,23 @@ $t->put_ok($url check_crash_fields($bug_id, SIGNATURE, CRASH_URL, 'After setting both'); +# A mass edit is rejected before any bug is updated. + +$t->post_ok( + $url . 'rest/bug' => api_headers($api_key) => json => new_crash_bug()) + ->status_is(200)->json_has('/id'); +my $unsigned_bug_id = $t->tx->res->json->{id}; + +update_is_rejected( + $bug_id, + {ids => [$bug_id, $unsigned_bug_id], cf_crash_data_url => PAREN_URL}, + $no_signature, + 'Setting a crash data url on several bugs, one without a signature' +); + +check_crash_fields($bug_id, SIGNATURE, CRASH_URL, 'After rejected mass edit'); +check_crash_fields($unsigned_bug_id, '', '', 'After rejected mass edit'); + # Clearing the URL removes the graph's extra filters. $t->put_ok($url