Bug 1823503 - After add comment, sometimes all status flags are changed to fixed by itself.

extensions/BugModal/Extension.pm

@@ -201,6 +201,14 @@ sub template_before_process {
     file => 'bug/edit.html.tmpl', vars => $vars,
   });
 
+  my $tracking_flags = Bugzilla::Extension::TrackingFlags::Flag->match({
+    product   => $bug->product,
+    component => $bug->component,
+    bug_id    => $bug->id,
+    is_active => 1,
+  });
+  $vars->{tracking_flags} = $tracking_flags;
+
   if (any { $bug->product eq $_ } READABLE_BUG_STATUS_PRODUCTS) {
     my @flags = map { {name => $_->name, status => $_->status} } @{$bug->flags};
     $vars->{readable_bug_status_json} = encode_json({
@@ -212,7 +220,7 @@ sub template_before_process {
       status           => $bug->bug_status,
       flags            => \@flags,
       target_milestone => $bug->target_milestone,
-      map { $_->name => $_->bug_flag($bug->id)->value } @{$vars->{tracking_flags}},
+      map { $_->name => $_->bug_flag($bug->id)->value } @{$tracking_flags},
     });
 
     # HTML4 attributes cannot be longer than this, so just skip it in this case.
@@ -273,24 +281,46 @@ sub template_before_process {
 
   # group tracking flags by version to allow for a better tabular output
   my @tracking_table;
-  my $tracking_flags = $vars->{tracking_flags};
-  foreach my $flag (@$tracking_flags) {
-    my $flag_type = $flag->flag_type;
-    my $type      = 'status';
-    my $name      = $flag->description;
-    if ($flag_type eq 'tracking' && $name =~ /^(tracking|status)-(.+)/) {
-      ($type, $name) = ($1, $2);
+  foreach my $flag (@{$tracking_flags}) {
+    my $flag_type   = $flag->flag_type;
+    my $type        = 'status';
+    my $name        = $flag->name;
+    my $description = $flag->description;
+    my $value       = $flag->bug_flag($bug->id)->value;
+
+    # Use short version of tracking flag description for the UI description
+    if ($flag_type eq 'tracking' && $description =~ /^(tracking|status)-(.+)/) {
+      ($type, $description) = ($1, $2);
     }
 
+    # Only include values that are active and the current user can set
+    my @values = ();
+    foreach my $value (@{$flag->values}) {
+      next if !$value->is_active || !$flag->can_set_value($value->name);
+      push @values, {id => $value->id, name => $value->name};
+    }
+
+    # If we already have a similar description, then add this flag to the same row
+    # in the UI. Otherwise it will be on a different row.
     my ($existing)
-      = grep { $_->{type} eq $flag_type && $_->{name} eq $name } @tracking_table;
+      = grep { $_->{type} eq $flag_type && $_->{description} eq $description }
+      @tracking_table;
     if ($existing) {
-      $existing->{$type} = $flag;
+      $existing->{$type} = {name => $name, value => $value, values => \@values};
     }
     else {
-      push @tracking_table, {$type => $flag, name => $name, type => $flag_type,};
+      push @tracking_table,
+        {
+        $type       => {name => $name, value => $value, values => \@values},
+        description => $description,
+        type        => $flag_type,
+        };
     }
   }
+
+  # Sort by description
+  @tracking_table
+    = sort { $a->{description} cmp $b->{description} } @tracking_table;
   $vars->{tracking_flags_table} = \@tracking_table;
 
   # for the "View -> Hide Treeherder Comments" menu item

extensions/BugModal/template/en/default/bug_modal/header.html.tmpl

@@ -69,13 +69,6 @@
 %]
 
 [% javascript = BLOCK %]
-  [%# add tracking flags JSON if available %]
-  [% IF tracking_flags %]
-    [% javascript_urls.push("extensions/TrackingFlags/web/js/flags.js") %]
-    var tracking_flags_str = "[% tracking_flags_json FILTER js %]";
-    var TrackingFlags = $.parseJSON(tracking_flags_str);
-  [% END %]
-
   [%# update last-visited %]
   [% IF user.id %]
     document.addEventListener('DOMContentLoaded', () => show_new_changes_indicator(), { once: true });

extensions/BugModal/template/en/default/bug_modal/tracking_flags.html.tmpl

@@ -11,19 +11,17 @@
   #%]
 
 [%
-  flags = [];
-  set_flags = [];
-  FOREACH flag IN tracking_flags;
-    NEXT UNLESS flag.flag_type == type;
-    flags.push(flag);
-    NEXT IF flag.bug_flag(bug.id).value == "---";
-    set_flags.push(flag);
+  RETURN UNLESS tracking_flags_table.size;
+  set_flags = 0;
+  FOREACH row IN tracking_flags_table;
+    NEXT IF (!row.tracking || row.tracking.value == "---")
+      && (!row.status || row.status.value == "---");
+    set_flags = 1;
   END;
-  RETURN UNLESS flags.size;
 %]
 
 [%# view %]
-[% IF set_flags.size %]
+[% IF set_flags %]
   <div class="flags edit-hide">
     <table class="layout-table tracking-flags">
       [% IF type == "tracking" %]
@@ -34,31 +32,30 @@
         </tr>
       [% END %]
       [% FOREACH row IN tracking_flags_table %]
-        [%
+        [%    
           NEXT UNLESS row.type == type;
-          tracking_value = row.tracking.bug_flag(bug_id).value || "---";
-          status_value = row.status.bug_flag(bug_id).value || "---";
-          NEXT IF tracking_value == "---" && status_value == "---";
+          NEXT IF (!row.tracking || row.tracking.value == "---")
+            && (!row.status || row.status.value == "---");
         %]
         <tr>
-          <td class="tracking-flag-name">[% row.name FILTER html %]</td>
+          <td class="tracking-flag-name">[% row.description FILTER html %]</td>
           [% IF type == "tracking" %]
             <td class="tracking-flag-tracking">
-              [% IF tracking_value != '---' %]
+              [% IF row.tracking.value != '---' %]
                 <a href="[% basepath FILTER none %]buglist.cgi?f1=[% row.tracking.name FILTER uri ~%]
-                  &amp;o1=equals&amp;v1=[% tracking_value FILTER uri %]">
+                  &amp;o1=equals&amp;v1=[% row.tracking.value FILTER uri %]">
               [% END %]
-              [%~ tracking_value FILTER html ~%]
-              [% '</a>' IF tracking_value != '---' %]
+              [%~ row.tracking.value FILTER html ~%]
+              [% '</a>' IF row.tracking.value != '---' %]
             </td>
           [% END %]
           <td class="tracking-flag-status">
-            [% IF status_value != '---' %]
+            [% IF row.status.value != '---' %]
               <a href="[% basepath FILTER none %]buglist.cgi?f1=[% row.status.name FILTER uri ~%]
-                &amp;o1=equals&amp;v1=[% status_value FILTER uri %]">
+                &amp;o1=equals&amp;v1=[% row.status.value FILTER uri %]">
             [% END %]
-            [%~ status_value FILTER html ~%]
-            [% '</a>' IF status_value != '---' %]
+            [%~ row.status.value FILTER html ~%]
+            [% '</a>' IF row.status.value != '---' %]
           </td>
         </tr>
       [% END %]
@@ -79,7 +76,7 @@
     [% FOREACH row IN tracking_flags_table %]
       [% NEXT UNLESS row.type == type %]
       <tr>
-        <td class="tracking-flag-name">[% row.name FILTER html %]</td>
+        <td class="tracking-flag-name">[% row.description FILTER html %]</td>
         [% IF type == "tracking" %]
           <td class="tracking-flag-tracking">[% INCLUDE tf_select flag=row.tracking %]</td>
         [% END %]
@@ -94,15 +91,11 @@
   <input type="hidden" id="[% flag.name FILTER html %]-dirty">
   <select id="[% flag.name FILTER html %]" name="[% flag.name FILTER html %]">
     [%
-      flag_bug_value = flag.bug_flag(bug_id).value;
       FOREACH value IN flag.values;
-        IF value.name != flag_bug_value;
-          NEXT IF !value.is_active || !flag.can_set_value(value.name);
-        END;
     %]
         <option value="[% value.name FILTER html %]"
           id="v[% value.id FILTER html %]_[% flag.name FILTER html %]"
-          [% " selected" IF flag_bug_value == value.name %]
+          [% " selected" IF flag.value == value.name %]
         >
           [% value.name FILTER html %]
         </option>

extensions/BugModal/web/bug_modal.js

@@ -960,12 +960,6 @@ $(function() {
             }
         });
 
-    // tracking flags
-    $('.tracking-flags select')
-        .change(function(event) {
-            tracking_flag_change(event.target);
-        });
-
     // take button
     $('.take-btn')
         .click(function(event) {

extensions/TrackingFlags/Extension.pm

@@ -99,7 +99,8 @@ sub template_before_process {
   elsif ($file eq 'bug/edit.html.tmpl'
     || $file eq 'bug/show.xml.tmpl'
     || $file eq 'email/bugmail.html.tmpl'
-    || $file eq 'email/bugmail.txt.tmpl')
+    || $file eq 'email/bugmail.txt.tmpl'
+    || $file eq 'bug_modal/edit.html/tmpl')
   {
     # note: bug/edit.html.tmpl doesn't support multiple bugs
     my $bug = exists $vars->{'bugs'} ? $vars->{'bugs'}[0] : $vars->{'bug'};
@@ -112,7 +113,6 @@ sub template_before_process {
         is_active => 1,
       });
 
-      $vars->{tracking_flags}      = $flags;
       $vars->{tracking_flags_json} = _flags_to_json($flags);
     }
 
@@ -129,15 +129,13 @@ sub template_before_process {
 sub _flags_to_json {
   my ($flags) = @_;
 
-  my $json = {flags => {}, types => [], comments => {},};
+  my $json = {types => [], comments => {}};
 
   my %type_map = map { $_->{name} => $_ } @{FLAG_TYPES()};
   foreach my $flag (@$flags) {
     my $flag_type = $flag->flag_type;
 
-    $json->{flags}->{$flag_type}->{$flag->name} = $flag->bug_flag->value;
-
-    if ($type_map{$flag_type}->{collapsed} && !grep { $_ eq $flag_type }
+    if ($type_map{$flag_type}->{collapsed} && none { $_ eq $flag_type }
       @{$json->{types}})
     {
       push @{$json->{types}}, $flag_type;

extensions/TrackingFlags/template/en/default/hook/bug_modal/header-end.html.tmpl

@@ -0,0 +1,16 @@
+[%# 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.
+  #%]
+
+[% javascript_urls.push('extensions/TrackingFlags/web/js/flags.js') %]
+
+[% IF tracking_flags_json %]
+  <script [% script_nonce FILTER none %]>                                                                                                                                                                  
+    var tracking_flags_str = "[% tracking_flags_json FILTER js %]";
+    var TrackingFlags = JSON.parse(tracking_flags_str);
+  </script>
+[% END %]

extensions/TrackingFlags/web/js/flags.js

@@ -6,94 +6,57 @@
  * defined by the Mozilla Public License, v. 2.0.
  */
 
-var Dom = YAHOO.util.Dom;
+$(function() {
+    'use strict';
 
-function hide_tracking_flags() {
-    for (var i = 0, l = TrackingFlags.types.length; i < l; i++) {
-        var flag_type = TrackingFlags.types[i];
-        for (var field in TrackingFlags.flags[flag_type]) {
-            var el = Dom.get(field);
-            var value = el ? el.value : TrackingFlags.flags[flag_type][field];
-            if (el && (value != TrackingFlags.flags[flag_type][field])) {
-                show_tracking_flags(flag_type);
-                return;
-            }
-            if (value == '---') {
-                Dom.addClass('row_' + field, 'bz_default_hidden');
-            } else {
-                Dom.addClass(field, 'bz_default_hidden');
-                Dom.removeClass('ro_' + field, 'bz_default_hidden');
-            }
-        }
-    }
-}
+    // tracking flags
+    $('.tracking-flags select')
+        .change(function(event) {
+            tracking_flag_change(event.target);
+        });
 
-function show_tracking_flags(flag_type) {
-    Dom.addClass('edit_' + flag_type + '_flags_action', 'bz_default_hidden');
-    for (var field in TrackingFlags.flags[flag_type]) {
-        if (Dom.get(field).value == '---') {
-            Dom.removeClass('row_' + field, 'bz_default_hidden');
+    function tracking_flag_change(e) {
+        var value = e.value;
+        var prefill;
+        if (TrackingFlags.comments[e.name])
+            prefill = TrackingFlags.comments[e.name][e.value];
+        if (!prefill) {
+            var cr = document.getElementById('cr_' + e.id);
+            if (cr) cr.parentElement.removeChild(cr);
+            return;
+        }
+        if (!document.getElementById('cr_' + e.id)) {
+            // create "comment required"
+            var span = document.createElement('span');
+            span.id = 'cr_' + e.id;
+            span.appendChild(document.createTextNode(' ('));
+            var a = document.createElement('a');
+            a.appendChild(document.createTextNode('comment required'));
+            a.href = '#';
+            a.onclick = function(event) {
+                event.preventDefault();
+                var c = document.getElementById('comment');
+                c.focus();
+                c.select();
+                var btn = document.getElementById('add_comment') || document.getElementById('add-comment');
+                if (btn) btn.scrollIntoView();
+            };
+            span.appendChild(a);
+            span.appendChild(document.createTextNode(')'));
+            e.parentNode.appendChild(span);
+        }
+        // prefill comment
+        var commentEl = document.getElementById('comment');
+        if (!commentEl) return;
+        var value = commentEl.value;
+        if (value == prefill) return;
+        if (value == '') {
+            commentEl.value = prefill;
+            a.innerHTML = 'comment required';
         } else {
-            Dom.removeClass(field, 'bz_default_hidden');
-            Dom.addClass('ro_' + field, 'bz_default_hidden');
+            commentEl.value = prefill + "\n\n" + value;
+            a.innerHTML = 'comment updated';
         }
     }
-}
-
-function tracking_flag_change(e) {
-    var value = e.value;
-    var prefill;
-    if (TrackingFlags.comments[e.name])
-        prefill = TrackingFlags.comments[e.name][e.value];
-    if (!prefill) {
-        var cr = document.getElementById('cr_' + e.id);
-        if (cr)
-            cr.parentElement.removeChild(cr);
-        return;
-    }
-    if (!document.getElementById('cr_' + e.id)) {
-        // create "comment required"
-        var span = document.createElement('span');
-        span.id = 'cr_' + e.id;
-        span.appendChild(document.createTextNode(' ('));
-        var a = document.createElement('a');
-        a.appendChild(document.createTextNode('comment required'));
-        a.href = '#';
-        a.onclick = function(event) {
-            event.preventDefault();
-            var c = document.getElementById('comment');
-            c.focus();
-            c.select();
-            var btn = document.getElementById('add_comment') || document.getElementById('add-comment');
-            if (btn)
-                btn.scrollIntoView();
-        };
-        span.appendChild(a);
-        span.appendChild(document.createTextNode(')'));
-        e.parentNode.appendChild(span);
-    }
-    // prefill comment
-    var commentEl = document.getElementById('comment');
-    if (!commentEl)
-        return;
-    var value = commentEl.value;
-    if (value == prefill)
-        return;
-    if (value == '') {
-        commentEl.value = prefill;
-        a.innerHTML = 'comment required';
-    } else {
-        commentEl.value = prefill + "\n\n" + value;
-        a.innerHTML = 'comment updated';
-    }
-}
-
-YAHOO.util.Event.onDOMReady(function() {
-    var edit_tracking_links = Dom.getElementsByClassName('edit_tracking_flags_link');
-    for (var i = 0, l = edit_tracking_links.length; i < l; i++) {
-        YAHOO.util.Event.addListener(edit_tracking_links[i], 'click', function(e) {
-            e.preventDefault();
-            show_tracking_flags(this.name);
-        });
-    }
 });
+