Closed
Bug 121747
Opened 23 years ago
Closed 23 years ago
query.cgi doesn't check for shutdownhtml
Categories
(Bugzilla :: Query/Bug List, defect, P1)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.16
People
(Reporter: justdave, Assigned: myk)
References
Details
(Keywords: regression)
Attachments
(1 file, 2 obsolete files)
6.82 KB,
patch
|
gerv
:
review+
myk
:
review+
|
Details | Diff | Splinter Review |
Is this a regression because of the template conversion? Are any other templatized pages affected?
Reporter | ||
Updated•23 years ago
|
Severity: normal → blocker
Priority: -- → P1
Target Milestone: --- → Bugzilla 2.16
Reporter | ||
Updated•23 years ago
|
Keywords: regression
Comment 1•23 years ago
|
||
Probably, because all the template stuff reads before anything happens. We need a global func: checkForShutdown() which handles this for us.
Comment 2•23 years ago
|
||
IIRC, we check for shutdownhtml in PutHeader(). It was done there because then it only required a few lines of code change rather than changing *.cgi. This wasn't a really big deal originally, as all the .cgi's called PutHeader() [most of them before doing anything critical, see bug 121741 for an exception]. As we move to templates, this line of thinking changes in that you want to gather all your data before you output anything. Also, I'm pretty sure that the templates don't use PutHeader() anymore. I know they all include global/header, but I highly doubt that checks for shutdownhtml and does the right thing. So, it appears that Bradley is correct and we need to new global routine [such as checkForShutdown()] that gets called at the top of every .cgi... As we start moving toward having Bugzilla be able to output data formats other than HTML (such as xml.cgi and the patch in bug 103778) we need to be careful that we output errors appropriate to the format in question, not just generically spit out some HTML that says Bugzilla is down.
Assignee | ||
Comment 3•23 years ago
|
||
Why not do this in CGI.pl? That script gets required by every CGI script, so we could put a check at the top of it and stop every script dead in its tracks if Bugzilla is currently shut down.
Assignee | ||
Comment 4•23 years ago
|
||
Assignee | ||
Updated•23 years ago
|
Reporter | ||
Comment 7•23 years ago
|
||
Comment on attachment 66925 [details] [diff] [review] patch v1: moves shut down message display to beginning of CGI.pl Looks like you have the attachment tracker patch mixed in with this...
Assignee | ||
Comment 8•23 years ago
|
||
Attachment #66925 -
Attachment is obsolete: true
Assignee | ||
Comment 9•23 years ago
|
||
This version removes the "our" variable declarations that Perl 5.005 doesn't support and substitutes good ole "$::varname"s.
Attachment #66936 -
Attachment is obsolete: true
Comment 10•23 years ago
|
||
Comment on attachment 66959 [details] [diff] [review] patch v3: uses global variables instead of "our" declaration r=gerv. This would be very good to have :-) Gerv
Attachment #66959 -
Flags: review+
Comment 11•23 years ago
|
||
Comment on attachment 66959 [details] [diff] [review] patch v3: uses global variables instead of "our" declaration r=kiko except for the Stash functions (they look ok, I just don't understand what they are for) The rest of the code should simplify work. You can mark 2r= if you think it's ok to land (I do)
Assignee | ||
Comment 12•23 years ago
|
||
Comment on attachment 66959 [details] [diff] [review] patch v3: uses global variables instead of "our" declaration Apparently I kiko's r= if I agree with him that he should r= it. I do agree with about that, so checking it in. Checking in CGI.pl; /cvsroot/mozilla/webtools/bugzilla/CGI.pl,v <-- CGI.pl new revision: 1.130; previous revision: 1.129 done Checking in globals.pl; /cvsroot/mozilla/webtools/bugzilla/globals.pl,v <-- globals.pl new revision: 1.135; previous revision: 1.134 done Checking in template/default/global/message.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/default/global/message.html.tmpl,v <-- message.html.tmpl initial revision: 1.1 done Checking in template/default/global/header; /cvsroot/mozilla/webtools/bugzilla/template/default/global/header,v <-- header new revision: 1.5; previous revision: 1.4 done
Attachment #66959 -
Flags: review+
Assignee | ||
Comment 13•23 years ago
|
||
Fix checked in. Resolving fixed.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 14•23 years ago
|
||
So, what's the officially-sanctioned way of using templates in a file now, then? Here's my guess: use vars qw($template $vars); ... $vars->{'foo'} = \@bar; Is that right? I'd very much like to avoid having to do a global s/$vars/$::vars/, if possible. Gerv
Comment 15•23 years ago
|
||
Clarifying my drunken comment: I just didn't figure out if the Stash part did what was expected; the code looked correct and good, so if Stash works r=kiko. :)
Assignee | ||
Comment 16•23 years ago
|
||
>I'd very much like to avoid having to do a global s/$vars/$::vars/, if possible.
Sorry, but we had to revert to $::vars for compatibility with Perl 5.005 ("our"
was introduced in 5.6). This nightware will all go away in modularization. In
the meantime, squeeze your eyes shut and repeat after me: there's no place like
home, there's no place like home. ;-)
Comment 17•23 years ago
|
||
Will my $vars = $::vars; my $template = $::template; work? That would solve a lot of problems. Gerv
Assignee | ||
Comment 18•23 years ago
|
||
>Will > >my $vars = $::vars; >my $template = $::template; > >work? That would solve a lot of problems. This probably won't work if you need to process multiple templates without the variables from earlier templates polluting the variable space of later templates, since $::vars (and $::template) are references to a hash and an object, respectively. Otherwise, they probably will work, but what problems do $::vars and $::template have that creating local references will solve?
Comment 19•23 years ago
|
||
> Otherwise, they probably will work, but what problems do
> $::vars and $::template have that creating local references will solve?
a) They prevent a global search and replace
b) They look far, far nicer :-)
Gerv
Updated•12 years ago
|
QA Contact: matty_is_a_geek → default-qa
You need to log in
before you can comment on or make changes to this bug.
Description
•