125660 - Templatise process_bug.cgi

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Gervase Markham [:gerv]]

This file needs templatising. (Well, it needs a complete reorganisation, but we
don't have time for that, so templatisation will have to do.) It should probably
wait until show_bug.cgi and long_list.cgi are done, because those patches touch
this file.

Gerv

Comment 1

[Gervase Markham [:gerv]]

Taking.

Gerv

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Taking.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Err, taking.

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Gerv, take this back if you already have code for it.  I just figured you had
enough to do after looking at the list of 2.16 blockers.

Comment 5

[Gervase Markham [:gerv]]

I have no code for this. I have the day off today, so if I don't get around to
it and post a patch today, consider it yours :-)

Gerv

Comment 6

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: templatizes process_bug.cgi

process_bug.cgi is a particularly difficult script to templatize because it
uses iterative output to display partial results to the user when it processes
multiple bugs.	Supporting iterative output with a single template is
complicated in this case because of the amount of processing that takes place
between outputs.

The two templatization options are a minimalist templatization that follows the
flow of the current script by processing a template each time output is called
for and a maximalist templatization that puts everything in a single template
which calls functions for processing as appropriate.

I opted for the minimalist approach because it is the smallest amount of change
that resolves this blocker of the overdue 2.16 release and because maximalist
templatization requires extensive code reworking, a project that will need
thought and planning and may be better carried out in the context of a more
general redesign of the bug processing code.

Comment 7

[Gervase Markham [:gerv]]

Comment on attachment 77888 [details] [diff] [review]
patch v1: templatizes process_bug.cgi

This is excellent - a great first attempt at a horrible templatisation :-)
I think you are definitely right to use lots of little templates; that
may give us a migration headache in the future, but we'll cross that bridge
when we come to it.

This is a review of the patch, not the complete file (in the case of
the CGI.) After you've waded through this lot, I'll apply it and review
it again.

>-PutHeader ("Bug processed");
>+# Start displaying the response page.
>+$template->process("process/start.html.tmpl", $vars)
>+  || ThrowTemplateError("Template process failed: " . $template->error())
>+  && exit;

ThrowTemplateError already includes "Template process failed"; all you need to
pass it is $template->error(). Same for all instances.

>-    print PuntTryAgain(qq{
>+    print ThrowUserError(qq{
> Only the owner or submitter of the bug, or a sufficiently
> empowered user, may make that change to the $f field.
> <TABLE>
>@@ -365,8 +299,6 @@
> <TR><TH ALIGN="right">New value:</TH><TD>$newvalue</TD></TR>
> </TABLE>
> });
>-    PutFooter();
>-    exit();

Is this error message complex enough for its own template? I was discussing
this with bbaetz earlier; when do we decide a text is complex enough to move
it out to a template rather than keeping it in the CGI? Something we need to
think about.

>+    $vars->{'form'} = \%::FORM;

You could set this once, at the beginning, as it's used quite often.

>+            ThrowUserError("The bug id $::FORM{'id'} is invalid. Please reload this bug
>+                            and try again.");

This will then say "please press back and try again." I'm only reading the
patch, so I can't see exactly when this message is used. If "please press
back" is inappropriate, we may have to add another parameter to the
ThrowUserError call to suppress this message.

>+    my $escaped_knob = html_quote($::FORM{'knob'});
>+    ThrowCodeError("Unknown action $escaped_knob!\n");

escaped_knob - what a wonderful image that conjures up.

>+            ThrowUserError("Unknown keyword named <code>" . html_quote($keyword) . 
>+                           "</code>. <P>The legal keyword names are 
>+                            <A HREF=describekeywords.cgi>listed here</A>.");

Nit: lowercase the HTML.

>+            ThrowUserError("You must determine a target milestone for bug $id
>+                            if you are going to accept it.  Part of accepting 
>+                            a bug is giving an estimate of when it will be fixed.", 
>+                           "", 
>+                           1);

This will make the error titleless. If you want to not pass something and
use the default, pass undef. Same for several other instances. You might
also want to pass the string "unlock tables" instead of 1; it doesn't
matter what you pass, but it makes the code more readable.

>+                    ThrowUserError("Dependency loop detected!<P>" .
>+                      "The following bug(s) would appear on both the \"depends on\" " .
>+                      "and \"blocks\" parts of the dependency tree if these changes " .
>+                      "are committed: $both<br>" .
>+                      "This would create a circular dependency, which is not allowed.",

Here (and perhaps in other places), you can remove all the end-of-line 
quotes and concatenations, i.e.

>+                    ThrowUserError("Dependency loop detected!<P>
>+                      The following bug(s) would appear on both the \"depends on\" 
>+                      and \"blocks\" parts of the dependency tree if these changes 
>+                      are committed: $both<br>
>+                      This would create a circular dependency, which is not allowed.",

This has the advantage of making the message easier to reformat, and making
Perl do less work.

You are also missing a </p> here.

>+# End the response page.
>+$template->process("process/end.html.tmpl", $vars)
>+  || ThrowTemplateError("Template process failed: " . $template->error())
>+  && exit;

Remind me why middle and end aren't the same template?

>+[%# Generate hidden form fields for non-excluded fields. %]
>+[% FOREACH field = form %]
>+  [% NEXT IF field.key.search(exclude) %]
>+  <input type="hidden" name="[% field.key %]" value="[% field.value FILTER html %]">

Are there any fields which should not be FILTER html'ed?

>+  <a href="show_bug.cgi?id=[% duplicate_bug_id %]">Throw away my changes, 
>+    and go revisit bug [% duplicate_bug_id %]</a>

Please remove the word "go".

>+++ template/default/process/mid-air.html.tmpl	5 Apr 2002 19:39:00 -0000

>+  # comments: array; all the comments on the bug.

This is true; but you might want to mention it's an array of hashes, or say
that its makeup is documented in another template, or something.

>+    <form method="post">
>+      [% PROCESS "global/hidden-fields.html.tmpl" exclude="^(Bugzilla|LDAP)_(login|password)$" %]
>+      <input type="submit" value="Submit my changes anyway">
>+        This will cause all of the above changes to be overwritten
>+        [% ", except for the added comments" IF comments.size > start_at %]
>+    </form>

Nit: you need a full stop. And "comment(s)" ?

>+  </li>
>+  <li>
>+    <a href="show_bug.cgi?id=[% bug_id %]">Throw away my changes, 
>+      and go revisit bug [% bug_id %]</a>

Please remove the word "go" :-)

>+[% 
>+  title = {
>+    'bug' => "Changes submitted for bug" , 
>+    'dupe' => "Duplicate notation added to bug" , 
>+    'dep' => "Checking for dependency changes on bug" , 
>+  } 
>+%]

Will attachment fu eventually use this template also?

>+[%# INTERFACE:
>+  # form: hash; the form values submitted to the script
>+  #
>+  # verify_fields: boolean; whether or not to verify 
>+  #   the version, component, and target milestone fields

Nit: to prevent interface comments becoming enormous, I tend to omit the blank
line between elements.

>+[% USE mycgi = CGI %]

Ooh :-) What does this do? I think I can guess.

>+  <p>
>+  [% IF usetargetmilestone %]
>+    You are moving the bug(s) to the product <b>[% formfields.product %]</b>, 
>+    and now the version, component, and/or target milestone fields are not correct 
>+    (or perhaps they were not correct in the first place).  In any case, 
>+    please set the correct version, component, and target milestone now:

I think we should remove the brackets and (in any case). It's pedantry which
doesn't enlighten the user.

>+  <table>
>+    <tr>
>+      <td>
>+        <b>Version:</b><br>
>+        [% mycgi.popup_menu(Name => "version" , Values => versions) %]
>+      </td>
>+      <td>
>+        <b>Component:</b><br>
>+        [% mycgi.popup_menu(Name => "component" , Values => components) %]
>+      </td>
>+      [% IF use_target_milestone %]
>+        <td>
>+          <b>Target Milestone:</b><br>
>+          [% mycgi.popup_menu(Name => "target_milestone" , Values => milestones) %]
>+        </td>
>+      [% END %]
>+    </tr>
>+  </table>
>+
>+[% END %]
>+       
>+[% IF verify_bug_group %]
>+  <h3>Verify Bug Group</h3>
>+
>+  <p>
>+    Do you want to add the bug to its new product's group (if any)?
>+  </p>
>+
>+  <p>
>+    <input type="radio" name="addtonewgroup" value="no"><b>no</b><br>
>+    <input type="radio" name="addtonewgroup" value="yes"><b>yes</b><br>
>+    <input type="radio" name="addtonewgroup" value="yesifinold" checked>
>+      <b>yes, but only if the bug was in its old product's group</b><br>
>+  </p>

We could do with a better way of explaining product groups, too, but I can't
think of one.

Comment 8

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v2: review fixes

>This is a review of the patch, not the complete file (in the case of
>the CGI.) After you've waded through this lot, I'll apply it and review
>it again.

I did my best not to rewrite major parts of the script, even parts that were
itching for it, so you may find that it is still easier to review the patch
than the script as a whole.

>ThrowTemplateError already includes "Template process failed"; all you need to

>pass it is $template->error(). Same for all instances.

Done.

>Is this error message complex enough for its own template?

Not sure, but it could be much simpler in any case, so I have done that.

>>+    $vars->{'form'} = \%::FORM;
>
>You could set this once, at the beginning, as it's used quite often.

There are three instances of this, one of which hacks a form value first, so
it's not quite that simple.  I could move it up to the top for the other two
instances, but I think it's a bit clearer this way since none of the instances
are associated with the general processing output (one is for product
verification, the second for duplicate cc: confirmation, and the third for
mid-air collision notification).

>This will then say "please press back and try again."

Fixed by removing the redundant (and inaccurate) message.

>Nit: lowercase the HTML.

Done.


>This will make the error titleless. If you want to not pass something and
>use the default, pass undef.

Actually the empty string evaluates to false, causing "$error ||= 'Error';" to
return "Error" if $error contains the empty string.  Nevertheless, "undef" is a
bit clearer here, so done.

>You might also want to pass the string "unlock tables" instead of 1

Good idea; done.

>Here (and perhaps in other places), you can remove all the end-of-line 
>quotes and concatenations, i.e.

Done.  That looks like it was the last one.

>You are also missing a </p> here.

Fixed.

>Remind me why middle and end aren't the same template?

Middle and end are actually mutually exclusive.  Middle runs when the bug being
processed is in the last query results (and generates its own HTML footer via
the "show_bug" function), while end runs otherwise and generates an HTML
footer.

>Are there any fields which should not be FILTER html'ed?

Not that I know of.  The old version of the script value_quotes all of them.

>Please remove the word "go".

Done.

>This is true; but you might want to mention it's an array of hashes, or say
>that its makeup is documented in another template, or something.

Done.

>Nit: you need a full stop. And "comment(s)" ?

Done and done.

>Please remove the word "go" :-)

Done.

>Will attachment fu eventually use this template also?

Probably.

>Nit: to prevent interface comments becoming enormous, I tend to omit the blank

>line between elements.

Done.

>Ooh :-) What does this do? I think I can guess.

It uses the "CGI" standard Perl module for (in this case) generating select
menus.

>I think we should remove the brackets and (in any case). It's pedantry which
>doesn't enlighten the user.

Done.

>We could do with a better way of explaining product groups, too, but I can't
>think of one.

If you think of one, file a bug!

Comment 9

[Gervase Markham [:gerv]]

Comment on attachment 77922 [details] [diff] [review]
patch v2: review fixes

>+  || ThrowTemplateError($template->error())
>+  && exit;

You can eliminate "&& exit" in all cases, too. All Throw* functions
are guaranteed not to return - i.e. they all call exit(). This is
a convention so there's no possibility of confusion.

>@@ -357,16 +291,10 @@
>     SendSQL("UNLOCK TABLES");
>     $oldvalue = value_quote($oldvalue);
>     $newvalue = value_quote($newvalue);

Why are we using value_quote rather than html_quote here?

>-    print PuntTryAgain(qq{
>-Only the owner or submitter of the bug, or a sufficiently
>-empowered user, may make that change to the $f field.
>-<TABLE>
>-<TR><TH ALIGN="right">Old value:</TH><TD>$oldvalue</TD></TR>
>-<TR><TH ALIGN="right">New value:</TH><TD>$newvalue</TD></TR>
>-</TABLE>
>-});
>-    PutFooter();
>-    exit();
>+    ThrowUserError("You tried to change the <strong>$f</strong> field 
>+                    from <em>$oldvalue</em> to <em>$newvalue</em>, 
>+                    but only the owner or submitter of the bug, or a 
>+                    sufficiently empowered user, may change that field.");

Does $f need quoting? (Maybe it is, and I can't see it in the context.)

>+            ThrowUserError("The bug id $::FORM{'id'} is invalid.");

Doesn't taint complain about this? Or do we detaint_natural it somewhere?

>-                    PuntTryAgain("$orig is not a legal bug number");
>+                    ThrowUserError("$orig is not a legal bug number", undef, "UNLOCK TABLES");

Hmm. Putting it in caps may imply to people that you can pass any old
SQL in there and it'll get executed (as caps is our standard for
SQL keywords.) What do you think? Lowercase might be a better choice.

>+[%# INTERFACE:
>+  #   No variables need to be passed into this template.
>+  #%]
>+
>+[% PROCESS global/header
>+  title = "Bug processed"
>+%]
>+
>+<hr>

Does this need to be a template on its own? Can't you just ask
for the header directly? This would mean losing the <hr>, maybe - 
does that negatively affect the aesthetics?

>+[% USE mycgi = CGI %]

All of this is kind of cool, but we haven't done it this way anywhere
else (we've rolled our own blocks, usually.) Do you think we should
be consistent, and keep doing that, or look to moving towards CGI.pm
in more areas, or just allow people to do either?

I know very little about CGI.pm - perhaps all the developers need to
learn about it...

OK, that's me finished for tonight :-) If you can do another version 
before you go home, I'll be able to test it more tomorrow (Saturday) 
and we can wrap it up early next week :-)

Gerv

Comment 10

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v3: review fixes

>You can eliminate "&& exit" in all cases, too. All Throw* functions
>are guaranteed not to return - i.e. they all call exit(). This is
>a convention so there's no possibility of confusion.

Done.

>Why are we using value_quote rather than html_quote here?

Looks like a bug.  Fixed.

>Does $f need quoting? (Maybe it is, and I can't see it in the context.)

No, $f is a known field name, and no field names need quoting.

>Doesn't taint complain about this? Or do we detaint_natural it somewhere?

Not sure.  The code before the error message could probably use some cleaning
up, but I'm loathe to do it as part of this templatization at this stage in the
game.  It would be better to leave that to another bug.

>Hmm. Putting it in caps may imply to people that you can pass any old
>SQL in there and it'll get executed (as caps is our standard for
>SQL keywords.) What do you think? Lowercase might be a better choice.

I lower-cased it and changed it to "unlock," which is descriptive but can't be
mistaken for valid SQL.

>Does this need to be a template on its own? Can't you just ask
>for the header directly? This would mean losing the <hr>, maybe - 
>does that negatively affect the aesthetics?

I don't think it matters much either way, but in this patch both start and end
have been eliminated (and middle has been renamed to next-bug).

>All of this is kind of cool, but we haven't done it this way anywhere
>else (we've rolled our own blocks, usually.) Do you think we should
>be consistent, and keep doing that, or look to moving towards CGI.pm
>in more areas, or just allow people to do either?

There have been interesting discussions on the TT mailing list about the
relative merits of using libraries like CGI.pm to generate HTML content vs.
in-lining the content.	The basic argument for the in-line folks is that it
makes the HTML easier to read (others dispute this).

I think if we're going to use functions (whether BLOCKS or library calls) to
reduce redundant code, then we've already lost whatever extra readability
in-lining adds, in which case the only question is whether to roll our own or
use a library.	I'm a firm believer in libraries, especially ones that have
already been written and are widely deployed, and even more especially ones
that come standard with Perl.

However, at this stage in the game I don't think we should make any kind of
requirement.  We should just let people do what they want and see what proves
itself in practice.

>I know very little about CGI.pm - perhaps all the developers need to
>learn about it...

Yeah, especially since I want to replace CGI.pl with it in Bugzilla. ;-)

>OK, that's me finished for tonight :-) If you can do another version 
>before you go home, I'll be able to test it more tomorrow (Saturday) 
>and we can wrap it up early next week :-)

Here it is.

Comment 11

[Gervase Markham [:gerv]]

Comment on attachment 77968 [details] [diff] [review]
patch v3: review fixes

>+            ThrowUserError("You have to specify a <b>comment</b> on this change.  
>+                            Please give some words on the reason for your change.");

This (old) wording is a bit ugly. How about:

"Making a change of this sort requires a comment explaining your actions."

Then they'll get "please press back and try again", which should round it off
nicely.


>+          ThrowUserError("You cannot reassign to a bug to nobody.  Unless you
>+                          intentionally cleared out the \"Reassign bug to\" 
>+                          field, " . Param("browserbugmessage"));

This is inconsistent with above; field names tend to be <b> in error messages.

>+            ThrowUserError("You must specify a component whose owner 
>+                            should get assigned these bugs.");

"whose owner these bugs should be assigned to." is better English - 
but is that the focus of this error? I can't tell from the context, 
but would something like "You must specify the component to which 
these bugs will belong." be better?

>+            ThrowUserError("You must specify a valid bug number of which this bug
>+                            is a duplicate.  The bug has not been changed.")

We don't add "The bug has not been changed" anywhere else - I don't 
think there's any reason that the user would be more afraid of that 
here than anywhere else. And we do say "press back and try again". 
So we could remove that sentence.

>+            ThrowUserError("Unknown keyword named <code>" . html_quote($keyword) . 
>+                           "</code>. <p>The legal keyword names are 
>+                            <a href=\"describekeywords.cgi\">listed here</a></p>.");

"Keyword $keyword is not a legal keyword. The list of legal 
keywords is <a>here</a>".

>-                    PuntTryAgain("$orig is not a legal bug number");
>+                    ThrowUserError("$orig is not a legal bug number", undef, "unlock");

Can we sync up the error text of this (and subsequent) illegal-bug-no 
error messages with the previous one?

>+                    ThrowUserError("You can't make a bug blocked or dependent on itself.",
>+                                   undef,
>+                                   "unlock");

"You cannot make a bug block itself, or depend on itself."

>+                    ThrowUserError(qq|Dependency loop detected!<p>
>+                      The following bug(s) would appear on both the "depends on"
>+                      and "blocks" parts of the dependency tree if these changes
>+                      are committed: $both<br>This would create a circular 
>+                      dependency, which is not allowed.</p>|,
>+                      undef,
>+                      "unlock");

"Dependency Loop Detected" should be the title.  

Change "these are committed" to "this change is made".

>+  [% IF usetargetmilestone %]
>+    You are moving the bug(s) to the product <b>[% formfields.product %]</b>, 
>+    and now the version, component, and/or target milestone fields are not
>+    correct.  Please set the correct version, component, and target milestone now:
>+  [% ELSE %]
>+    You are moving the bug(s) to the product <b>[% formfields.product %]</b>, 
>+    and now the version, and component fields are not correct.  In any case, 
>+    please set the correct version and component now:
>+  [% END %] 
>+  <p>

You missed some of the wording change in the second one of these, and 
formfields.product doesn't work. How about:

>+    You are moving the bug(s) to the product <b>[% formfields.product %]</b>, 
>+    and the version and component fields are no longer correct. 
>+    Please set the correct version and component now:

(and the analogous thing for the other version.) Eventually, we should 
miss things off here if there's only one possible value. But that's a 
Different Bug(TM).

So there's only one real bug in the above (the problem with
formfields.product).
 I've also tested the patch, and it seems to work well. So I think we are 
pretty close here :-)

Gerv

Comment 12

[Bradley Baetz (:bbaetz)]

Whats wrong with 1 as a truth argument? It is standard programming practice, and
'unlock' gives the impression that the text actually means something.

Comment 13

[Gervase Markham [:gerv]]

> Whats wrong with 1 as a truth argument? It is standard programming practice, 

Well, 1 is the standard value for #define TRUE. But then you use TRUE rather
than 1. We could use "TRUE", but that doesn't give any more information than "1".

> and 'unlock' gives the impression that the text actually means something.

"1" gives the impression that the argument is a number.

(And maybe one day it could mean something - if we decide we want that param to
represent which bit of SQL we execute before dying.)

"unlock" makes it much clearer what's going on. You don't need to read the
function being called to guess what it's doing.

Gerv

Comment 14

[Bradley Baetz (:bbaetz)]

Comment on attachment 77968 [details] [diff] [review]
patch v3: review fixes

gerv: I guess. Althoguh if you do plan to change it, then you should require
'unlock'. I'd prefer
'abort', for aborting trasactions, too, though.

The conversion to the new error stuff should have been a separate bug.

>@@ -948,64 +842,30 @@
>                 SqlQuote($oldhash{'product'}));
>         if ($value eq FetchOneColumn()) {
>             SendSQL("UNLOCK TABLES");
>-            PuntTryAgain("You must determine a target milestone for bug $id " .
>-                         "if you are going to accept it.  (Part of " .
>-                         "accepting a bug is giving an estimate of when it " .
>-                         "will be fixed.)");
>+            ThrowUserError("You must determine a target milestone for bug $id
>+                            if you are going to accept it.  Part of accepting 
>+                            a bug is giving an estimate of when it will be fixed.", 
>+                           undef, 
>+                           "unlock");

if you pass in 'unlock', don't SendSQL("UNLOCK TABLES") above.

Why did we remove punttryagain as a synonym for throwusererror + unlocking
tables, btw?

>+# End the response page.
>+$template->process("show/navigate.html.tmpl", $vars)
>+  || ThrowTemplateError($template->error());
>+$template->process("global/footer", $vars)
>+  || ThrowTemplateError($template->error());



>+<form method="post">
>+
>+[% PROCESS "global/hidden-fields.html.tmpl" exclude="^(Bugzilla|LDAP)_(login|password)$" %]

Does .search do regexp matching?

>+
>+<p>
>+  <input type="radio" name="confirm_add_duplicate" value="1"> 
>+    Yes, add the reporter to CC list on bug [% original_bug_id %]
>+</p>
>+<p>
>+  <input type="radio" name="confirm_add_duplicate" value="0" checked="checked"> 
>+    No, do not add the reporter to CC list on bug [% original_bug_id %]
>+</p>
>+<p>
>+  <a href="show_bug.cgi?id=[% duplicate_bug_id %]">Throw away my changes, 
>+    and revisit bug [% duplicate_bug_id %]</a>
>+<p>
>+  <input type="submit" value="Submit">
>+</p>
>+

Nit: <label> ?

There is no way that we're going to find all the bugs with this. Like the other
ones, we'll need to check it in
and then fix regressions, unfortunately.

Comment 15

[Gervase Markham [:gerv]]

> 'unlock'. I'd prefer
> 'abort', for aborting trasactions, too, though.

"abort" is fine with me. I just want something descriptive.

> The conversion to the new error stuff should have been a separate bug.

<shrug> Well, he's done it now.

> Why did we remove punttryagain as a synonym for throwusererror + unlocking
> tables, btw?

Because it's a silly name :-) Conceptually, it makes sense to have whether to
unlock tables or not being a param to ThrowUserError rather than a separate
function.

Gerv

Comment 16

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v4: review fixes

I fixed the formfields.products problem and the text in which it is embedded. 
The rest of the language corrections should be filed as a new bug (the patch
for which should be small enough to get 2xreview).

I think "abort" makes much less sense than "unlock" (which itself is more
obtuse than the well-known Perl boolean "1"), especially in this context where
the only thing it does right now is unlock tables, but it's not a big deal, so
I changed "unlock" to "abort".

>Does .search do regexp matching?

Yes.

Comment 17

[Gervase Markham [:gerv]]

Comment on attachment 78215 [details] [diff] [review]
patch v4: review fixes

If that's all you've changed, r=gerv.

Gerv

Comment 18

[Bradley Baetz (:bbaetz)]

Comment on attachment 78215 [details] [diff] [review]
patch v4: review fixes

please add yourself to teh contributors section of process_bug.

This will cause feedback-delays when processing bugs with lots of dependancies,
I guess we'll have to live with that until processmail becomes a sub. The lots
of little templates stuff should eventually be fixed too, but not for 2.16 - is
there a bug on that?

r=bbaetz

Comment 19

[Myk Melez [:myk] [@mykmelez]]

Note: the fix I checked in includes a line adding my name to the contributors
list in process_bug.cgi but is otherwise the same as version four of the patch.

Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.154; previous revision: 1.153
done
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.121; previous revision: 1.120
done
RCS file:
/cvsroot/mozilla/webtools/bugzilla/template/default/global/hidden-fields.html.tmpl,v
done
Checking in template/default/global/hidden-fields.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/default/global/hidden-fields.html.tmpl,v
 <--  hidden-fields.html.tmpl
initial revision: 1.1
done
RCS file:
/cvsroot/mozilla/webtools/bugzilla/template/default/process/confirm-dupe.html.tmpl,v
done
Checking in template/default/process/confirm-dupe.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/default/process/confirm-dupe.html.tmpl,v
 <--  confirm-dupe.html.tmpl
initial revision: 1.1
done
RCS file:
/cvsroot/mozilla/webtools/bugzilla/template/default/process/mid-air.html.tmpl,v
done
Checking in template/default/process/mid-air.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/default/process/mid-air.html.tmpl,v
 <--  mid-air.html.tmpl
initial revision: 1.1
done
RCS file:
/cvsroot/mozilla/webtools/bugzilla/template/default/process/next-bug.html.tmpl,v
done
Checking in template/default/process/next-bug.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/default/process/next-bug.html.tmpl,v
 <--  next-bug.html.tmpl
initial revision: 1.1
done
RCS file:
/cvsroot/mozilla/webtools/bugzilla/template/default/process/results.html.tmpl,v
done
Checking in template/default/process/results.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/default/process/results.html.tmpl,v
 <--  results.html.tmpl
initial revision: 1.1
done
RCS file:
/cvsroot/mozilla/webtools/bugzilla/template/default/process/verify-new-product.html.tmpl,v
done
Checking in template/default/process/verify-new-product.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/default/process/verify-new-product.html.tmpl,v
 <--  verify-new-product.html.tmpl
initial revision: 1.1
done