die_with_dignity is useless inside of Bugzilla::CGI

RESOLVED FIXED in Bugzilla 3.0

Status

()

Bugzilla
Bugzilla-General
--
minor
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: Max Kanat-Alexander, Assigned: Max Kanat-Alexander)

Tracking

2.23
Bugzilla 3.0
Bug Flags:
approval +

Details

Attachments

(1 attachment)

v1
1.86 KB, patch
cso
: review+
Frédéric Buclin
: review+
Details | Diff | Splinter Review
(Assignee)

Description

12 years ago
We have our die_with_dignity debugging code in Bugzilla::CGI. But guess what? It doesn't do anything if it's inside of Bugzilla::CGI.

Probably better to put it in Bugzilla.pm, where it would actually work.
(Assignee)

Comment 1

12 years ago
Created attachment 227842 [details] [diff] [review]
v1

Now, this one's complex. Moving commented code... :-D
Assignee: general → mkanat
Status: NEW → ASSIGNED
Attachment #227842 - Flags: review?(colin.ogilvie)

Comment 2

12 years ago
Comment on attachment 227842 [details] [diff] [review]
v1

r=me on inspection - I don' think a lot can go wrong with a commented out code block ;)
Attachment #227842 - Flags: review?(colin.ogilvie) → review+

Comment 3

12 years ago
(In reply to comment #2)
> I don' think a lot can go wrong with a commented out code block ;)

This is not the right approach to review this patch. What you should do is to uncomment this block and make sure die_with_dignity() works correctly when being in Bugzilla.pm.
(Assignee)

Updated

12 years ago
Attachment #227842 - Flags: review?(LpSolit)

Comment 4

12 years ago
Comment on attachment 227842 [details] [diff] [review]
v1

Note that if $cgi->header hasn't been called previously for .cgi scripts, a "malformed header" message is displayed in your browser instead of the expected error message (typically enter_bug.cgi which calls $cgi->header very late). The web server error log records both errors though, i.e. the "malformed header" one and the "real" error message.

I checked, 2.22, where die_with_dignity() is still in globals.pl, has the same problem; so this is not a regression. That's the reason I r+ this patch, and also because leaving it in CGI.pm doesn't work.

r=LpSolit
Attachment #227842 - Flags: review?(LpSolit) → review+

Updated

12 years ago
Severity: normal → minor
Flags: approval?
Flags: approval? → approval+
(Assignee)

Comment 5

12 years ago
Checking in Bugzilla.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla.pm,v  <--  Bugzilla.pm
new revision: 1.40; previous revision: 1.39
done
Checking in Bugzilla/CGI.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/CGI.pm,v  <--  CGI.pm
new revision: 1.25; previous revision: 1.24
done
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.