Remove VarHash experiment

Bugzilla/CGI.pm

@@ -496,34 +496,6 @@ sub cookie
     return new CGI::Cookie(@param);
 }
 
-# Request variables in PHP-like format:
-# - parameters without [] are always treated as scalars, except listed as keys of %$force_array
-# - parameters with [] are always treated as arrays
-sub VarHash
-{
-    my $self = shift;
-    return $self->{_VarHash} if $self->{_VarHash};
-    my ($force_array) = @_;
-    my $args = Bugzilla->input_params;
-    my $filtered = {};
-    for my $key (keys %$args)
-    {
-        if ($key =~ /\[\]$/so)
-        {
-            $filtered->{substr $key, 0, -2} = ref $args->{$key} eq 'ARRAY' ? $args->{$key} : [ $args->{$key} ];
-        }
-        elsif ($force_array->{$key})
-        {
-            $filtered->{$key} = ref $args->{$key} eq 'ARRAY' ? $args->{$key} : [ $args->{$key} ];
-        }
-        else
-        {
-            $filtered->{$key} = ref $args->{$key} eq 'ARRAY' ? $args->{$key}->[-1] : $args->{$key};
-        }
-    }
-    return $self->{_VarHash} = $filtered;
-}
-
 1;
 
 __END__

Bugzilla/Product.pm

@@ -1118,9 +1118,10 @@ sub choose_product
     ThrowUserError('no_products') unless @$products;
     return $products->[0] if @$products == 1;
 
-    delete $query_params->{classification};
-    $query_params = http_build_query($query_params);
-    $query_params .= '&' if length $query_params;
+    my $qp = { %$query_params };
+    delete $qp->{classification};
+    $qp = http_build_query($qp);
+    $qp .= '&' if length $qp;
     if (!$target)
     {
         $target = $ENV{REQUEST_URI};
@@ -1129,7 +1130,7 @@ sub choose_product
     }
     my $vars = {
         target => $target,
-        query_params => $query_params,
+        query_params => $qp,
     };
     if (Bugzilla->get_field('classification')->enabled)
     {

Bugzilla/Util.pm

@@ -225,7 +225,7 @@ sub url_quote_noslash
     return $toencode;
 }
 
-# http_build_query($hashref), like PHP's one
+# http_build_query($hashref) - transforms a parameter hash into URL-encoded query string
 sub http_build_query($)
 {
     my ($query) = @_;

editusersingroup.cgi

@@ -14,7 +14,7 @@ use Bugzilla::User;
 use Bugzilla::Util;
 use Bugzilla::Token;
 
-my $ARGS = Bugzilla->cgi->VarHash;
+my $ARGS = Bugzilla->input_params;
 my $user = Bugzilla->login(LOGIN_REQUIRED);
 my $vars;
 
@@ -41,8 +41,8 @@ Bugzilla::User::match_field({
 
 my @add_members = split /[\s,]+/, $ARGS->{add_members};
 my @add_bless = $vars->{allow_bless} ? (split /[\s,]+/, $ARGS->{add_bless}) : ();
-my @rm_members = @{$ARGS->{remove} || []};
-my @rm_bless = $vars->{allow_bless} ? @{$ARGS->{unbless} || []} : ();
+my @rm_members = list $ARGS->{remove};
+my @rm_bless = $vars->{allow_bless} ? list $ARGS->{unbless} : ();
 
 if (@add_members || @add_bless || @rm_members || @rm_bless)
 {

editvalues.cgi

@@ -29,9 +29,8 @@ use Bugzilla::Field::Choice;
 Bugzilla->login(LOGIN_REQUIRED);
 
 my $dbh      = Bugzilla->dbh;
-my $cgi      = Bugzilla->cgi;
 my $template = Bugzilla->template;
-my $ARGS     = $cgi->VarHash;
+my $ARGS     = Bugzilla->input_params;
 my $vars     = {};
 
 Bugzilla->user->in_group('editvalues')
@@ -101,7 +100,7 @@ if ($action eq 'new')
         map { $_ => $ARGS->{$_} }
         grep { defined $ARGS->{$_} } ($type->DB_COLUMNS, $type->REQUIRED_CREATE_FIELDS)
     });
-    $created_value->set_visibility_values($ARGS->{visibility_value_id});
+    $created_value->set_visibility_values([ list $ARGS->{visibility_value_id} ]);
 
     delete_token($token);
 
@@ -166,7 +165,7 @@ if ($action eq 'update')
     }
     if ($value->field->value_field)
     {
-        $vars->{changes}->{visibility_values} = $value->set_visibility_values($ARGS->{visibility_value_id});
+        $vars->{changes}->{visibility_values} = $value->set_visibility_values([ list $ARGS->{visibility_value_id} ]);
     }
     $vars->{changes}->{control_lists} = 1 if $field->update_control_lists($value->id, $ARGS);
     delete_token($token);

enter_bug.cgi

@@ -57,13 +57,10 @@ my $user = Bugzilla->login(LOGIN_REQUIRED);
 my $cloned_bug;
 my $cloned_bug_id;
 
-my $cgi = Bugzilla->cgi;
 my $dbh = Bugzilla->dbh;
 my $template = Bugzilla->template;
 my $vars = {};
-my $ARGS = $cgi->VarHash({
-    (map { ($_->name => 1) } grep { $_->type == FIELD_TYPE_MULTI_SELECT } Bugzilla->active_custom_fields),
-});
+my $ARGS = Bugzilla->input_params;
 
 # All pages point to the same part of the documentation.
 $vars->{doc_section} = 'bugreports.html';
@@ -191,7 +188,7 @@ foreach my $field (Bugzilla->active_custom_fields)
     my $cf_value = $ARGS->{$cf_name};
     if (defined $cf_value)
     {
-        $default{$cf_name} = $cf_value;
+        $default{$cf_name} = $field->type == FIELD_TYPE_MULTI_SELECT ? [ list $cf_value ] : $cf_value;
     }
     elsif ($field->default_value && !$field->is_select)
     {
@@ -343,7 +340,7 @@ else
 #
 # Eventually maybe each product should have a "current version"
 # parameter.
-my $vercookie = $cgi->cookie('VERSION-' . $product->name);
+my $vercookie = Bugzilla->cookies->{'VERSION-' . $product->name};
 if ($cloned_bug_id && $product->name eq $cloned_bug->product)
 {
     $default{version} = $cloned_bug->version && $cloned_bug->version_obj->name;
@@ -457,7 +454,7 @@ $vars->{default} = \%default;
 
 my $format = $template->get_format('bug/create/create', $ARGS->{format}, $ARGS->{ctype});
 
-$cgi->send_header($format->{ctype});
+Bugzilla->cgi->send_header($format->{ctype});
 $template->process($format->{template}, $vars)
     || ThrowTemplateError($template->error());
 exit;

post_bug.cgi

@@ -46,22 +46,24 @@ use Bugzilla::Flag;
 
 my $user = Bugzilla->login(LOGIN_REQUIRED);
 
-my $cgi = Bugzilla->cgi;
 my $dbh = Bugzilla->dbh;
 my $template = Bugzilla->template;
 my $vars = {};
-my $ARGS = $cgi->VarHash({
-    cc => 1,
-    (map { ($_->name => 1) } Bugzilla->get_fields({ type => FIELD_TYPE_MULTI_SELECT })),
-});
-$ARGS->{cc} = join(', ', @{$ARGS->{cc}}) if $ARGS->{cc};
+my $ARGS = Bugzilla->input_params;
+
+$ARGS->{cc} = join(', ', list $ARGS->{cc}) if $ARGS->{cc};
+$ARGS->{$_->name} = [ list $ARGS->{$_->name} ] for Bugzilla->get_fields({ type => FIELD_TYPE_MULTI_SELECT });
 
 ######################################################################
 # Main Script
 ######################################################################
 
 # redirect to enter_bug if no field is passed.
-print $cgi->redirect(correct_urlbase() . 'enter_bug.cgi') unless $cgi->param;
+unless (keys %$ARGS)
+{
+    print Bugzilla->cgi->redirect(correct_urlbase() . 'enter_bug.cgi');
+    exit;
+}
 
 # Detect if the user already used the same form to submit a bug
 my $token = trim($ARGS->{token});
@@ -89,7 +91,12 @@ Bugzilla::User::match_field({
 
 if (defined $ARGS->{maketemplate})
 {
-    $vars->{url} = $cgi->canonicalise_query('token', 'cloned_bug_id', 'cloned_comment');
+    delete $ARGS->{$_} for qw(token cloned_bug_id cloned_comment);
+    for (keys %$ARGS)
+    {
+        delete $ARGS->{$_} if $ARGS->{$_} eq '';
+    }
+    $vars->{url} = http_build_query($ARGS);
     $vars->{short_desc} = $ARGS->{short_desc};
 
     $template->process("bug/create/make-template.html.tmpl", $vars)
@@ -193,7 +200,7 @@ my $timestamp = $bug->creation_ts;
 # a version on the page.
 if (defined $ARGS->{version} && $ARGS->{version} ne '' && $bug->version)
 {
-    $cgi->send_cookie(
+    Bugzilla->cgi->send_cookie(
         -name => "VERSION-" . $bug->product,
         -value => $bug->version_obj->name,
         -expires => "Fri, 01-Jan-2038 00:00:00 GMT"
@@ -210,7 +217,7 @@ for (keys %$ARGS)
 {
     if (/^attachmulti_(.*)_([^_]*)$/so)
     {
-        if ($1 eq 'data' && $cgi->upload($_))
+        if ($1 eq 'data' && Bugzilla->cgi->upload($_))
         {
             $is_multiple = 1;
         }
@@ -221,7 +228,7 @@ if ($is_multiple)
 {
     Bugzilla::Attachment::add_multiple($bug);
 }
-elsif (defined($cgi->upload('data')) || $ARGS->{attachurl} ||
+elsif (defined(Bugzilla->cgi->upload('data')) || $ARGS->{attachurl} ||
     $ARGS->{text_attachment} || $ARGS->{base64_content})
 {
     $ARGS->{isprivate} = $ARGS->{commentprivacy};
@@ -238,7 +245,7 @@ elsif (defined($cgi->upload('data')) || $ARGS->{attachurl} ||
     {
         my $data = $ARGS->{data};
         my $filename = '';
-        $filename = scalar($cgi->upload('data')) || $ARGS->{filename};
+        $filename = scalar(Bugzilla->cgi->upload('data')) || $ARGS->{filename};
         if ($ARGS->{text_attachment} !~ /^\s*$/so)
         {
             $data = $ARGS->{text_attachment};
@@ -299,7 +306,7 @@ if (Bugzilla->usage_mode != USAGE_MODE_EMAIL)
         title => Bugzilla->messages->{terms}->{Bug}.' '.$bug->id.' Submitted – '.$bug->short_desc,
         header => Bugzilla->messages->{terms}->{Bug}.' '.$bug->id.' Submitted',
     });
-    print $cgi->redirect(-location => 'show_bug.cgi?id='.$bug->id);
+    print Bugzilla->cgi->redirect(-location => 'show_bug.cgi?id='.$bug->id);
 }
 
 $vars;

process_bug.cgi

@@ -70,7 +70,6 @@ my $dbh = Bugzilla->dbh;
 my $template = Bugzilla->template;
 my $vars = {};
 my $ARGS = Bugzilla->input_params;
-#my $ARGS = $cgi->VarHash; # FIXME (see lines with "FIXME array[]")
 
 ######################################################################
 # Begin Data/Security Validation
@@ -83,7 +82,7 @@ $dbh->bz_start_transaction();
 my @bug_objects;
 if ($ARGS->{id})
 {
-    ($ARGS->{id}) = @{$ARGS->{id}} if ref $ARGS->{id}; # FIXME array[]
+    ($ARGS->{id}) = @{$ARGS->{id}} if ref $ARGS->{id};
     my $bug = Bugzilla::Bug->check({ id => $ARGS->{id}, for_update => 1 });
     $ARGS->{id} = $bug->id;
     push @bug_objects, $bug;
@@ -128,8 +127,8 @@ if ($ARGS->{dontchange})
 # do a match on the fields if applicable
 Bugzilla::User::match_field({
     'qa_contact'  => { type => 'single' },
-    'newcc'       => { type => 'multi'  }, # FIXME array[]
-    'masscc'      => { type => 'multi'  }, # FIXME array[]
+    'newcc'       => { type => 'multi'  },
+    'masscc'      => { type => 'multi'  },
     'assigned_to' => { type => 'single' },
 });
 
@@ -436,7 +435,7 @@ foreach my $b (@bug_objects)
         my @see_also = split ',', $ARGS->{see_also};
         $b->add_see_also($_) foreach @see_also;
     }
-    if (defined $ARGS->{remove_see_also}) # FIXME array[]
+    if (defined $ARGS->{remove_see_also})
     {
         $b->remove_see_also($_) foreach ref $ARGS->{remove_see_also} ? @{$ARGS->{remove_see_also}} : $ARGS->{remove_see_also};
     }
@@ -445,7 +444,7 @@ foreach my $b (@bug_objects)
     foreach my $field (@custom_fields)
     {
         my $fname = $field->name;
-        if (defined $ARGS->{$fname} || defined $ARGS->{"defined_$fname"}) # FIXME array[] for multiselects
+        if (defined $ARGS->{$fname} || defined $ARGS->{"defined_$fname"})
         {
             $b->set($fname, $ARGS->{$fname});
         }
@@ -526,10 +525,10 @@ if ($ARGS->{newcc} || $ARGS->{addselfcc} || $ARGS->{removecc} || $ARGS->{masscc}
     }
     else
     {
-        $cc_add = ref $ARGS->{newcc} ? join(', ', @{$ARGS->{newcc}}) : $ARGS->{newcc}; # FIXME array[]
+        $cc_add = ref $ARGS->{newcc} ? join(', ', @{$ARGS->{newcc}}) : $ARGS->{newcc};
         # We came from bug_form which uses a select box to determine what cc's
         # need to be removed...
-        if (defined $ARGS->{removecc} && $ARGS->{cc}) # FIXME array[]
+        if (defined $ARGS->{removecc} && $ARGS->{cc})
         {
             $cc_remove = ref $ARGS->{cc} ? join(', ', @{$ARGS->{cc}}) : $ARGS->{cc};
         }

show_bug.cgi

@@ -31,7 +31,7 @@ use Bugzilla::Bug;
 
 my $template = Bugzilla->template;
 my $vars = {};
-my $ARGS = Bugzilla->cgi->VarHash({ id => 1, excludefield => 1, field => 1, includefield => 1 });
+my $ARGS = Bugzilla->input_params;
 my $user = Bugzilla->login;
 
 # Editable, 'single' HTML bugs are treated slightly specially in a few places
@@ -58,7 +58,7 @@ Bugzilla->switch_to_shadow_db unless $user->id;
 
 if ($single)
 {
-    my ($id) = @{$ARGS->{id}};
+    my ($id) = list $ARGS->{id};
     push @bugs, Bugzilla::Bug->check($id);
     if (defined $ARGS->{mark})
     {
@@ -81,7 +81,7 @@ if ($single)
 else
 {
     my @errors;
-    foreach my $id (@{$ARGS->{id}})
+    foreach my $id (list $ARGS->{id})
     {
         # Be kind enough and accept URLs of the form: id=1,2,3.
         my @ids = split /,/, $id;
@@ -134,7 +134,7 @@ foreach (@fieldlist)
     $displayfields{$_} = 1;
 }
 
-foreach (@{$ARGS->{excludefield} || []})
+foreach (list $ARGS->{excludefield})
 {
     $displayfields{$_} = undef;
 }
@@ -142,7 +142,7 @@ foreach (@{$ARGS->{excludefield} || []})
 # CustIS Bug 70168
 if ($ARGS->{includefield} || $ARGS->{field})
 {
-    %displayfields = map { $_ => 1 } grep { $displayfields{$_} } @{$ARGS->{includefield} || $ARGS->{field}};
+    %displayfields = map { $_ => 1 } grep { $displayfields{$_} } list($ARGS->{includefield} || $ARGS->{field});
 }
 
 $vars->{displayfields} = \%displayfields;

template/en/default/admin/fieldvalues/control-list-common.html.tmpl

@@ -115,7 +115,7 @@
       [% IF f.is_select %]
         [% SET cur_default = f.default_value_hash_for(this_value.id) %]
         <select name="default_[% f.name | html %]" id="default_[% f.name | html %]"
-          [% IF f.type == constants.FIELD_TYPE_MULTI_SELECT %] multiple size="5"[% END %]>[%# FIXME array[] %]
+          [% IF f.type == constants.FIELD_TYPE_MULTI_SELECT %] multiple size="5"[% END %]>
           <option value="">---</option>
           [% SET vs = f.value_field_id == f.default_field_id ? f.restricted_legal_values(this_value.id) : f.legal_values %]
           [%# FIXME: component/target_milestone/version product_id is taken via hardcode here %]

template/en/default/admin/fieldvalues/edit.html.tmpl

@@ -107,7 +107,7 @@
       </tr>
       <tr>
         <td colspan="2">
-          <select name="visibility_value_id[]" id="visibility_value_id" multiple="multiple" size="15" style="width: 400px">
+          <select name="visibility_value_id" id="visibility_value_id" multiple="multiple" size="15" style="width: 400px">
             [% IF field.value_field.nullable %]
             <option value="0"[% IF field.is_value_enabled(value.id, 0) %] selected[% END %]>---</option>
             [% END %]

template/en/default/admin/groups/usersingroup.html.tmpl

@@ -73,7 +73,7 @@
       <td>[% u.user.realname | html %]</td>
       <td align="center">
         [% IF u.member_direct %]
-          explicit, <input type="checkbox" id="remove_[% u.user.id %]" name="remove[]"
+          explicit, <input type="checkbox" id="remove_[% u.user.id %]" name="remove"
             value="[% u.user.id %]" /><label title="remove [% u.user.login | html %] from [% group.name | html %]"
             for="remove_[% u.user.id %]">remove</label>
         [% ELSIF u.member_regexp %]
@@ -85,7 +85,7 @@
       <td align="center">
         [% IF u.bless_direct %]
           explicit
-          [%- IF allow_bless %], <input type="checkbox" id="unbless_[% u.user.id %]" name="unbless[]"
+          [%- IF allow_bless %], <input type="checkbox" id="unbless_[% u.user.id %]" name="unbless"
             value="[% u.user.id %]" /><label title="revoke [% group.name | html %] grant permission from [% u.user.login | html %]"
             for="unbless_[% u.user.id %]">revoke</label>
           [% END %]

template/en/default/bug/field.html.tmpl

@@ -150,7 +150,6 @@
         [% IF tabindex %] tabindex="[% tabindex | html %]"[% END %]
         name="[% field.name | html %]"
         [% IF field.type == constants.FIELD_TYPE_MULTI_SELECT %]
-          [% # FIXME array[] when $cgi->VarHash will be used everywhere %]
           [% SET field_size = 5 %]
           [% IF field.legal_values.size < 5 %]
             [% SET field_size = field.legal_values.size %]
@@ -219,7 +218,7 @@
           [% IF editable %]
             <label><input type="checkbox" value="[% url | html %]"
               [% IF tabindex %] tabindex="[% tabindex | html %]"[% END %]
-              name="remove_[% field.name | html %]" />Remove</label> [% # FIXME array[] when $cgi->VarHash will be used everywhere %]
+              name="remove_[% field.name | html %]" />Remove</label>
           [% END %]
         </li>
       [% END %]