The problem I encountered first is that the TT's use clause in verify-new-product.html.tmpl seems to freeze in my Win32/IIS5 installation, causing a CGI timeout. That's a problem in itself, one which has to be examined closer if the following one isn't solved. The more interesting question: why is cgi.pm used here, and if this is intentional, should we require it in checksetup.pl?

I used CGI.pm in that template because it's an elegant way of generating HTML. It doesn't need to be required in checksetup.pl because it comes standard with all recent Perl distributions. We should be using CGI.pm methods more and more to replace our homegrown equivalents in CGI.pl (and in lieu of TT snippets), but we could excise this code on the 2.16 branch (putting a TT snippet or one of our homegrown functions in its place) for the 2.16 release if the code is causing problems.

I think it would be wisest to use the old approach in 2.16. The USE statement in TT definitely seems to have some off problems on my three different Windows test platforms - all of them seem to freeze. I agree about migrating to CGI.pm, but I think whole Bugzilla should aim to do that in one step (hopefully not one patch, but within the span of one release - 2.18 or 2.20 then?).

What version of CGI.pm do you have? (and what perl version?) Are you using IIS or apache/win32?

IIS5, W2k, ActiveState Perl 5.6.1. I can't remember the CGI.pm version right now, but I can check it out later. The other two configurations are close to the same, but don't have the specs here offhand. All are W2k/IIS boxes anyway.

what happens if you change the USE CGI to USE CGI (-no_debug) I'm not sure if that works in TT - you may have to play arround a bit.

A good guess there, bbaetz. The proper syntax is [% USE mycgi = CGI (no_debug) %] and that does it, at least on my home test system. The CGI module version number is 2.74. Ok, since this apparently can be fixed this way, I'll attach a patch to add the no_debug switch to the use clause. I'm still not happy with one part of Bugzilla using CGI.pm when a lot of work has been done elsewhere to avoid using it, but if Myk feels it's better to this way, then I just propose adding the no_debug flag. Should IMO be resolved before 2.16, one way or another.

Add no_debug to verify-new-product's use cgi call.

Are you sure its not -no_debug, rather than no_debug? Thats what the docs say to do. If you upgrade CGI.pm, does that fix it? I know that I didn't need the -no_debug stuff we added to the testing suite, somehow.

I wouldn't say we avoid using CGI.pm, I think the reasons we don't use it are historical. Converting to templates doesn't really mean we shouldn't use it, I think it's a good idea personally. We should document its usage if it starts spreading through the code, however.

Even documenting the use isn't enough IMO. The shift from a custom CGI module to the common cgi.pm should be done with some coordination (a single patch or at least a group of patches reviewed together). It's not good for just some templates to start using cgi.pm whenever their authors so wish. It leads to obscure bugs such as this, where one seemingly innocent part of the system fails because there are some unknown incompatibilities. Bbaetz, re comment 8: I'm sure it's no_debug without the hyphen, no matter how illogical it sounds. I should try upgrading cgi.pm, but haven't had the time to test this. I'll try to do this tonight.

Jouni: Other places in the code use -no_debug, though. Can you look at CGI.pm to check? My version has: $DEBUG=0, next if /^[:-]no_?[Dd]ebug$/; which seems to require the -.

Comment on attachment 84055 [details] [diff] [review] Patch Oh dear. I tried "USE mycgi = CGI (abcdefghijklmnop)" and it works fine. Actually, the relevant thing seems to be SOME parameters for the CGI. It's not relevant which ones they are. I upgraded CGI.pm to 2.81 but with no apparent effect. BUT: if I tap in "USE mycgi = CGI (-no_debug)", note the hyphen, the I get the following: "file error - parse error: bug/process/verify-new-product.html.tmpl line 36: unexpected token (-) [% USE mycgi = CGI (-no_debug) %]" That makes me think it would be a TT problem. Any ideas on this? Obsoleting my patch proposal because it has become misleading in the light of last discoveries.

Try taking it to the TT list (www.tt2.org).

So are we doing the quick hack here for 2.16?

OK, executive decision here... get rid of the CGI.pm calls for 2.16. Let's open a new bug about transitioning our code to CGI.pm and eliminating CGI.pl later. This likely ain't going to make 2.16rc2 because it's probably too big a job, but we should have this in by the 2.16 GM (or rc3 if we end up doing one).

Ok, moving towards CGI.pm is now bug 147833. I'll mail the TT list about the problems with USE cgi and parameters on Win32. But that will have to wait until I get the time to test that a bit more thoroughly.

Shouldn't this include a minor version number bump?

Comment on attachment 85525 [details] [diff] [review] patch v1: select-menu.html.tmpl replaces CGI.pm calls r=gerv. Does TT not already provide some way of determining the type of variables? Gerv

>Shouldn't this include a minor version number bump? No, since the interface for the template isn't changing. >Does TT not already provide some way of determining the type of variables? Not that I can find. Using array and hash references in scalar context returns a scalar representation of the reference starting with the words ARRAY and HASH, respectively, but I know no good clean way of extracting that reference.

This patch grabs the variable type of the "values" variable without adding additional pseudo-methods.

Comment on attachment 85643 [details] [diff] [review] patch v2: same fix, but without adding "type" pseudo-methods >Index: template/en/default/global/select-menu.html.tmpl >=================================================================== >+ [% FOREACH value = values %] >+ <option value="[% value.value FILTER html %]" >+ [% " selected" IF value.value == default %]> >+ [% value.key FILTER html %] >+ </option> >+ [% END %] This "value.value" thing is quite horrible. Could that "values" be "choices" or "options" (per the HTML terminology)? A select element is not given values but choices or options IMO. Choosing between those two terms is a matter of taste - I'm slightly in favor of choices but would be fine with options as well. :-) Anyway, this works fine on Linux and W2k. Fix that values thing above, and r=jouni.

Comment on attachment 85643 [details] [diff] [review] patch v2: same fix, but without adding "type" pseudo-methods Makes more sense :-) Gerv

Regarding issue mentioned in comment 12: I believe there is some TT-internal problem with using the CGI.pm. I wrote about this to the TT list and Randal Schwartz replied saying he had a similar problem. See his post: http://www.template-toolkit.org/pipermail/templates/2002-May/003161.html We should consider this when moving on to using CGI plugin from TT. (I'll copy this over to bug 147833 as well)

This looks like the email you meant: http://www.template-toolkit.org/pipermail/templates/2002-May/003242.html

"values" changed to "options", commented the funky block that lets me determine whether the reference is to an array or a hash, and consistentized the formatting.

Comment on attachment 85658 [details] [diff] [review] patch v3: options instead of values; more comments; formatting fixes r=jouni

Comment on attachment 85658 [details] [diff] [review] patch v3: options instead of values; more comments; formatting fixes r=gerv. Gerv

patch is Myk's... looks ready for checkin.

Checked into the trunk and 2.16 branch. Resolving fixed. RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/select-menu.html.tmpl,v done Checking in template/en/default/global/select-menu.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/select-menu.html.tmpl,v <-- select-menu.html.tmpl initial revision: 1.1 done Checking in template/en/default/bug/process/verify-new-product.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/process/verify-new-product.html.tmpl,v <-- verify-new-product.html.tmpl new revision: 1.5; previous revision: 1.4 done Checking in template/en/default/global/select-menu.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/select-menu.html.tmpl,v <-- select-menu.html.tmpl new revision: 1.1.2.1; previous revision: 1.1 done Checking in template/en/default/bug/process/verify-new-product.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/process/verify-new-product.html.tmpl,v <-- verify-new-product.html.tmpl new revision: 1.3.2.2; previous revision: 1.3.2.1 done