238873 - remove %FORM from buglist.cgi

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect, P2)

Description

[Dave Miller [:justdave]]

This one is medium difficulty.

72:my $dotweak = $::FORM{'tweak'} ? 1 : 0;
85:if ($::FORM{'format'} && $::FORM{'format'} eq "rdf" && !$::FORM{'ctype'}) { 
86:    $::FORM{'ctype'} = "rdf";
87:    delete($::FORM{'format'});
96:if ((exists $::FORM{'ctype'}) && ($::FORM{'ctype'} eq "js")) {
103:my $format = GetFormat("list/list", $::FORM{'format'}, $::FORM{'ctype'});
120:            && !defined($::FORM{'serverpush'})
121:              || $::FORM{'serverpush'};
123:my $order = $::FORM{'order'} || "";
131:if ($::FORM{'regetlastlist'}) {
258:$::FORM{'cmdtype'} ||= "";
259:$::FORM{'remaction'} ||= "";
263:if ($::FORM{'cmdtype'} eq "runnamed") {  
264:    $::FORM{'cmdtype'} = "dorem"; 
265:    $::FORM{'remaction'} = "run";
283:if ($::FORM{'cmdtype'} eq "dorem" && $::FORM{'remaction'} =~ /^run/) {
284:    $filename = "$::FORM{'namedcmd'}-$date.$format->{extension}";
291:if ($::FORM{'cmdtype'} eq "dorem") {  
292:    if ($::FORM{'remaction'} eq "run") {
293:        $::buffer = LookupNamedQuery($::FORM{"namedcmd"});
294:        $vars->{'searchname'} = $::FORM{'namedcmd'};
299:    elsif ($::FORM{'remaction'} eq "runseries") {
300:        $::buffer = LookupSeries($::FORM{"series_id"});
301:        $vars->{'searchname'} = $::FORM{'namedcmd'};
306:    elsif ($::FORM{'remaction'} eq "forget") {
309:        my $qname = SqlQuote($::FORM{'namedcmd'});
318:        $vars->{'namedcmd'} = $::FORM{'namedcmd'};
325:elsif (($::FORM{'cmdtype'} eq "doit") && $::FORM{'remtype'}) {
326:    if ($::FORM{'remtype'} eq "asdefault") {
336:    elsif ($::FORM{'remtype'} eq "asnamed") {
340:        my $name = trim($::FORM{'newqueryname'});
345:        $::FORM{'newquery'} || ThrowUserError("buglist_parameters_required", 
347:        my $qbuffer = SqlQuote($::FORM{'newquery'});
665:if ($::FORM{'debug'}) {

124:my $order_from_cookie = 0;  # True if $order set using $::COOKIE{'LASTORDER'}
132:    $::COOKIE{'BUGLIST'} || ThrowUserError("missing_cookie");
138:                                 bug_id => [split(/:/, $::COOKIE{'BUGLIST'})],
185:    my $userid = DBNameToIdAndCheck($::COOKIE{"Bugzilla_login"});
308:        my $userid = DBNameToIdAndCheck($::COOKIE{"Bugzilla_login"});
328:        my $userid = DBNameToIdAndCheck($::COOKIE{"Bugzilla_login"});
338:        my $userid = DBNameToIdAndCheck($::COOKIE{"Bugzilla_login"});
457:elsif (defined $::COOKIE{'COLUMNLIST'}) {
459:    my $columnlist = $::COOKIE{'COLUMNLIST'};
555:if ($::COOKIE{'LASTORDER'} && (!$order || $order =~ /^reuse/i)) {
556:    $order = $::COOKIE{'LASTORDER'};
802:my $login = $::COOKIE{'Bugzilla_login'};
822:$vars->{'splitheader'} = $::COOKIE{'SPLITHEADER'} ? 1 : 0;

Comment 1

[Christian Reis]

COOKIE being done in bug 251937.

Comment 2

[Max Kanat-Alexander]

I suppose I can do this one.

Comment 3

[Teemu Mannermaa (:wicked)]

Attachment: Remove FORMs from buglist.cgi, V1

This is a self contained patch to change FORM hash to CGI object in buglist.cgi
script. Passes tests and I tested buglist functions briefly with this patch
applied.

Comment 4

[Jouni Heikniemi]

Comment on attachment 179576 [details] [diff] [review]
Remove FORMs from buglist.cgi, V1

>-if ($::FORM{'content'}) { $fulltext = 1 }
>+if (defined $cgi->param('content')) { $fulltext = 1 }

Note that changes like this actually change the interface; while "fulltext=0"
didn't previously enable full text search, they now do. Similarly for several
other spots such as regetlastlist. I don't actually consider this to be a
problem since we never linked to other than the =1 forms anyway. In a sense, I
find just checking for defined/not-defined more clear than the content-oriented
comparison... So I'm inclined to go your way here.

r=jouni, but of course leaving the approver the chance to reject the patch
because of this change.

Comment 5

[Frédéric Buclin]

Comment on attachment 179576 [details] [diff] [review]
Remove FORMs from buglist.cgi, V1

r=LpSolit by inspection. I did not test this patch, so this is not an
"official" review.

I agree with jouni, |if ($::FORM{foo})| is ambiguous and replacing this by |if
(defined $cgi->param(foo))| looks like the right substitution to me.

Comment 6

[Frédéric Buclin]

Checking in buglist.cgi;
/cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v  <--  buglist.cgi
new revision: 1.290; previous revision: 1.289
done