Bug 487508: Allow restricting the visibility of custom fields and values by component

r=dkl, a=mkanat

21 files changed, 139 insertions, 222 deletions

diff --git a/Bugzilla/Component.pm b/Bugzilla/Component.pm
index 194a3957c..5fb911031 100644
--- a/Bugzilla/Component.pm
+++ b/Bugzilla/Component.pm
@@ -17,11 +17,9 @@
 #                 Max Kanat-Alexander <mkanat@bugzilla.org>
 #                 Akamai Technologies <bugzilla-dev@akamai.com>
 
-use strict;
-
 package Bugzilla::Component;
-
-use base qw(Bugzilla::Object);
+use strict;
+use base qw(Bugzilla::Field::ChoiceInterface Bugzilla::Object);
 
 use Bugzilla::Constants;
 use Bugzilla::Util;
@@ -35,6 +33,8 @@ use Bugzilla::Series;
 ###############################
 
 use constant DB_TABLE => 'components';
+# This is mostly for the editfields.cgi case where ->get_all is called.
+use constant LIST_ORDER => 'product_id, name';
 
 use constant DB_COLUMNS => qw(
     id
@@ -80,7 +80,7 @@ sub new {
     my $dbh = Bugzilla->dbh;
 
     my $product;
-    if (ref $param) {
+    if (ref $param and !defined $param->{id}) {
         $product = $param->{product};
         my $name = $param->{name};
         if (!defined $product) {
@@ -156,6 +156,8 @@ sub remove_from_db {
     my $self = shift;
     my $dbh = Bugzilla->dbh;
 
+    $self->_check_if_controller(); # From ChoiceInterface
+
     $dbh->bz_start_transaction();
 
     if ($self->bug_count) {
@@ -418,11 +420,25 @@ sub product {
 ####      Accessors        ####
 ###############################
 
-sub id          { return $_[0]->{'id'};          }
-sub name        { return $_[0]->{'name'};        }
 sub description { return $_[0]->{'description'}; }
 sub product_id  { return $_[0]->{'product_id'};  }
 
+##############################################
+# Implement Bugzilla::Field::ChoiceInterface #
+##############################################
+
+use constant FIELD_NAME => 'component';
+use constant is_default => 0;
+use constant is_active => 1;
+
+sub is_set_on_bug {
+    my ($self, $bug) = @_;
+    # We treat it like a hash always, so that we don't have to check if it's
+    # a hash or an object.
+    return 0 if !defined $bug->{component_id};
+    $bug->{component_id} == $self->id ? 1 : 0;
+}
+
 ###############################
 ####      Subroutines      ####
 ###############################
diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index 94d9f8bed..948ff5337 100644
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -129,6 +129,8 @@ use File::Basename;
     FIELD_TYPE_BUG_ID
     FIELD_TYPE_BUG_URLS
 
+    ABNORMAL_SELECTS
+
     TIMETRACKING_FIELDS
 
     USAGE_MODE_BROWSER
@@ -368,6 +370,13 @@ use constant FIELD_TYPE_DATETIME  => 5;
 use constant FIELD_TYPE_BUG_ID  => 6;
 use constant FIELD_TYPE_BUG_URLS => 7;
 
+# See the POD for Bugzilla::Field/is_abnormal to see why these are listed
+# here.
+use constant ABNORMAL_SELECTS => qw(
+    product
+    component
+);
+
 # The fields from fielddefs that are blocked from non-timetracking users.
 # work_time is sometimes called actual_time.
 use constant TIMETRACKING_FIELDS =>
diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm
index 7b1569c52..2f14037ab 100644
--- a/Bugzilla/Field.pm
+++ b/Bugzilla/Field.pm
@@ -180,7 +180,7 @@ use constant DEFAULT_FIELDS => (
     {name => 'priority',     desc => 'Priority',   in_new_bugmail => 1,
      type => FIELD_TYPE_SINGLE_SELECT, buglist => 1},
     {name => 'component',    desc => 'Component',  in_new_bugmail => 1,
-     buglist => 1},
+     type => FIELD_TYPE_SINGLE_SELECT, buglist => 1},
     {name => 'assigned_to',  desc => 'AssignedTo', in_new_bugmail => 1,
      buglist => 1},
     {name => 'reporter',     desc => 'ReportedBy', in_new_bugmail => 1,
@@ -492,6 +492,28 @@ sub is_select {
             || $_[0]->type == FIELD_TYPE_MULTI_SELECT) ? 1 : 0 
 }
 
+=over
+
+=item C<is_abnormal>
+
+Most fields that have a C<SELECT> L</type> have a certain schema for
+the table that stores their values, the table has the same name as the field,
+and the field's legal values can be edited via F<editvalues.cgi>.
+
+However, some fields do not follow that pattern. Those fields are
+considered "abnormal".
+
+This method returns C<1> if the field is "abnormal", C<0> otherwise.
+
+=back
+
+=cut
+
+sub is_abnormal {
+    my $self = shift;
+    return grep($_ eq $self->name, ABNORMAL_SELECTS) ? 1 : 0;
+}
+
 sub legal_values {
     my $self = shift;
 
diff --git a/Bugzilla/Field/Choice.pm b/Bugzilla/Field/Choice.pm
index 7e07ca1e2..9c2fbdb38 100644
--- a/Bugzilla/Field/Choice.pm
+++ b/Bugzilla/Field/Choice.pm
@@ -23,7 +23,7 @@ use strict;
 
 package Bugzilla::Field::Choice;
 
-use base qw(Bugzilla::Object);
+use base qw(Bugzilla::Field::ChoiceInterface Bugzilla::Object);
 
 use Bugzilla::Config qw(SetParam write_params);
 use Bugzilla::Constants;
@@ -66,6 +66,7 @@ use constant VALIDATORS => {
 
 use constant CLASS_MAP => {
     bug_status => 'Bugzilla::Status',
+    component  => 'Bugzilla::Component',
     product    => 'Bugzilla::Product',
 };
 
@@ -194,115 +195,10 @@ sub remove_from_db {
         ThrowUserError("fieldvalue_still_has_bugs",
                        { field => $self->field, value => $self });
     }
-    $self->_check_if_controller();
+    $self->_check_if_controller(); # From ChoiceInterface.
     $self->SUPER::remove_from_db();
 }
 
-# Factored out to make life easier for subclasses.
-sub _check_if_controller {
-    my $self = shift;
-    my $vis_fields = $self->controls_visibility_of_fields;
-    my $values     = $self->controlled_values;
-    if (@$vis_fields || scalar(keys %$values)) {
-        ThrowUserError('fieldvalue_is_controller',
-            { value => $self, fields => [map($_->name, @$vis_fields)],
-              vals => $values });
-    }
-}
-
-
-#############
-# Accessors #
-#############
-
-sub is_active { return $_[0]->{'isactive'}; }
-sub sortkey   { return $_[0]->{'sortkey'};  }
-
-sub bug_count {
-    my $self = shift;
-    return $self->{bug_count} if defined $self->{bug_count};
-    my $dbh = Bugzilla->dbh;
-    my $fname = $self->field->name;
-    my $count;
-    if ($self->field->type == FIELD_TYPE_MULTI_SELECT) {
-        $count = $dbh->selectrow_array("SELECT COUNT(*) FROM bug_$fname
-                                         WHERE value = ?", undef, $self->name);
-    }
-    else {
-        $count = $dbh->selectrow_array("SELECT COUNT(*) FROM bugs 
-                                         WHERE $fname = ?",
-                                       undef, $self->name);
-    }
-    $self->{bug_count} = $count;
-    return $count;
-}
-
-sub field {
-    my $invocant = shift;
-    my $class = ref $invocant || $invocant;
-    my $cache = Bugzilla->request_cache;
-    # This is just to make life easier for subclasses. Our auto-generated
-    # subclasses from type() already have this set.
-    $cache->{"field_$class"} ||=  
-        new Bugzilla::Field({ name => $class->DB_TABLE });
-    return $cache->{"field_$class"};
-}
-
-sub is_default {
-    my $self = shift;
-    my $name = $self->DEFAULT_MAP->{$self->field->name};
-    # If it doesn't exist in DEFAULT_MAP, then there is no parameter
-    # related to this field.
-    return 0 unless $name;
-    return ($self->name eq Bugzilla->params->{$name}) ? 1 : 0;
-}
-
-sub is_static {
-    my $self = shift;
-    # If we need to special-case Resolution for *anything* else, it should
-    # get its own subclass.
-    if ($self->field->name eq 'resolution') {
-        return grep($_ eq $self->name, ('', 'FIXED', 'MOVED', 'DUPLICATE'))
-               ? 1 : 0;
-    }
-    elsif ($self->field->custom) {
-        return $self->name eq '---' ? 1 : 0;
-    }
-    return 0;
-}
-
-sub controls_visibility_of_fields {
-    my $self = shift;
-    $self->{controls_visibility_of_fields} ||= Bugzilla::Field->match(
-        { visibility_field_id => $self->field->id, 
-          visibility_value_id => $self->id });
-    return $self->{controls_visibility_of_fields};
-}
-
-sub visibility_value {
-    my $self = shift;
-    if ($self->{visibility_value_id}) {
-        $self->{visibility_value} ||=
-            Bugzilla::Field::Choice->type($self->field->value_field)->new(
-                $self->{visibility_value_id});
-    }
-    return $self->{visibility_value};
-}
-
-sub controlled_values {
-    my $self = shift;
-    return $self->{controlled_values} if defined $self->{controlled_values};
-    my $fields = $self->field->controls_values_of;
-    my %controlled_values;
-    foreach my $field (@$fields) {
-        $controlled_values{$field->name} = 
-            Bugzilla::Field::Choice->type($field)
-            ->match({ visibility_value_id => $self->id });
-    }
-    $self->{controlled_values} = \%controlled_values;
-    return $self->{controlled_values};
-}
-
 ############
 # Mutators #
 ############
@@ -402,6 +298,9 @@ each value type needs its own class.
 
 See the L</SYNOPSIS> for examples of how this works.
 
+This class implements L<Bugzilla::Field::ChoiceInterface>, and so all
+methods of that class are also available here.
+
 =head1 METHODS
 
 =head2 Class Factory
@@ -424,28 +323,7 @@ must call C<type> to get a class you can call methods on.
 
 =back
 
-=head2 Accessors
+=head2 Mutators
 
-These are in addition to the standard L<Bugzilla::Object> accessors.
-
-=over
-
-=item C<sortkey>
-
-The key that determines the sort order of this item.
-
-=item C<field>
-
-The L<Bugzilla::Field> object that this field value belongs to.
-
-=item C<controlled_values>
-
-Tells you which values in B<other> fields appear (become visible) when this
-value is set in its field.
-
-Returns a hashref of arrayrefs. The hash keys are the names of fields,
-and the values are arrays of C<Bugzilla::Field::Choice> objects,
-representing values that this value controls the visibility of, for
-that field.
-
-=back
+This class implements mutators for all of the settable accessors in
+L<Bugzilla::Field::ChoiceInterface>.
diff --git a/Bugzilla/Migrate.pm b/Bugzilla/Migrate.pm
index 282279e75..6c353357d 100644
--- a/Bugzilla/Migrate.pm
+++ b/Bugzilla/Migrate.pm
@@ -323,7 +323,7 @@ sub reset_serial_values {
     );
     my @select_fields = grep { $_->is_select } (values %{ $self->bug_fields });
     foreach my $field (@select_fields) {
-        next if $field->name eq 'product';
+        next if $field->is_abnormal;
         $reset{$field->name} = 'id';
     }
     
@@ -709,8 +709,8 @@ sub insert_bugs {
         $self->debug($bug, 3);
 
         foreach my $field (@standard_drop_downs) {
+            next if $field->is_abnormal;
             my $field_name = $field->name;
-            next if $field_name eq 'product';
             if (!defined $bug->{$field_name}) {
                 # If there's a default value for this, then just let create()
                 # pick it.
diff --git a/Bugzilla/Product.pm b/Bugzilla/Product.pm
index c993905db..6b00fcbf6 100644
--- a/Bugzilla/Product.pm
+++ b/Bugzilla/Product.pm
@@ -32,9 +32,7 @@ use Bugzilla::Mailer;
 use Bugzilla::Series;
 use Bugzilla::Hook;
 
-# Currently, we only implement enough of the Bugzilla::Field::Choice
-# interface to control the visibility of other fields.
-use base qw(Bugzilla::Field::Choice);
+use base qw(Bugzilla::Field::ChoiceInterface Bugzilla::Object);
 
 use constant DEFAULT_CLASSIFICATION_ID => 1;
 
@@ -43,10 +41,6 @@ use constant DEFAULT_CLASSIFICATION_ID => 1;
 ###############################
 
 use constant DB_TABLE => 'products';
-# Reset these back to the Bugzilla::Object defaults, instead of the
-# Bugzilla::Field::Choice defaults.
-use constant NAME_FIELD => 'name';
-use constant LIST_ORDER => 'name';
 
 use constant DB_COLUMNS => qw(
    id
@@ -565,14 +559,7 @@ sub _check_votes {
 # Implement Bugzilla::Field::Choice #
 #####################################
 
-sub field {
-    my $invocant = shift;
-    my $class = ref $invocant || $invocant;
-    my $cache = Bugzilla->request_cache;
-    $cache->{"field_$class"} ||= new Bugzilla::Field({ name => 'product' });
-    return $cache->{"field_$class"};
-}
-
+use constant FIELD_NAME => 'product';
 use constant is_default => 0;
 
 ###############################
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index 43b95f293..88e44ac1d 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -219,8 +219,8 @@ sub init {
         type     => [FIELD_TYPE_MULTI_SELECT, FIELD_TYPE_BUG_URLS],
         obsolete => 0 });
     foreach my $field (@select_fields) {
+        next if $field->is_abnormal;
         my $name = $field->name;
-        next if $name eq 'product'; # products don't have sortkeys.
         $special_order{$name} = [ "$name.sortkey", "$name.value" ],
         $special_order_join{$name} =
            "LEFT JOIN $name ON $name.value = bugs.$name";
diff --git a/Bugzilla/Status.pm b/Bugzilla/Status.pm
index 4d1281e7e..0dd17ae39 100644
--- a/Bugzilla/Status.pm
+++ b/Bugzilla/Status.pm
@@ -23,7 +23,10 @@ use strict;
 package Bugzilla::Status;
 
 use Bugzilla::Error;
-
+# This subclasses Bugzilla::Field::Choice instead of implementing 
+# ChoiceInterface, because a bug status literally is a special type
+# of Field::Choice, not just an object that happens to have the same
+# methods.
 use base qw(Bugzilla::Field::Choice Exporter);
 @Bugzilla::Status::EXPORT = qw(
     BUG_STATE_OPEN
diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm
index 53f3255d1..16217bb63 100644
--- a/Bugzilla/WebService/Bug.pm
+++ b/Bugzilla/WebService/Bug.pm
@@ -422,8 +422,8 @@ sub legal_values {
     my $field = Bugzilla::Bug::FIELD_MAP->{$params->{field}} 
                 || $params->{field};
 
-    my @global_selects = Bugzilla->get_fields(
-        {type => [FIELD_TYPE_SINGLE_SELECT, FIELD_TYPE_MULTI_SELECT]});
+    my @global_selects = grep { !$_->is_abnormal }
+                         Bugzilla->get_fields({ is_select => 1 });
 
     my $values;
     if (grep($_->name eq $field, @global_selects)) {
diff --git a/editvalues.cgi b/editvalues.cgi
index 477bf8696..b651b23c2 100755
--- a/editvalues.cgi
+++ b/editvalues.cgi
@@ -71,16 +71,12 @@ Bugzilla->user->in_group('admin') ||
 my $action = trim($cgi->param('action')  || '');
 my $token  = $cgi->param('token');
 
-# Fields listed here must not be edited from this interface.
-my @non_editable_fields = qw(product);
-my %block_list = map { $_ => 1 } @non_editable_fields;
-
 #
 # field = '' -> Show nice list of fields
 #
 if (!$cgi->param('field')) {
-    my @field_list = grep { !$block_list{$_->name} }
-                       Bugzilla->get_fields({ is_select => 1 });
+    my @field_list = grep { !$_->is_abnormal }
+                          Bugzilla->get_fields({ is_select => 1 });
 
     $vars->{'fields'} = \@field_list;
     $template->process("admin/fieldvalues/select-field.html.tmpl", $vars)
@@ -90,7 +86,7 @@ if (!$cgi->param('field')) {
 
 # At this point, the field must be defined.
 my $field = Bugzilla::Field->check($cgi->param('field'));
-if (!$field->is_select || $block_list{$field->name}) {
+if (!$field->is_select || $field->is_abnormal) {
     ThrowUserError('fieldname_invalid', { field => $field });
 }
 $vars->{'field'} = $field;
diff --git a/report.cgi b/report.cgi
index 9a618c013..6612ded2a 100755
--- a/report.cgi
+++ b/report.cgi
@@ -111,7 +111,6 @@ my @columns = qw(
     assigned_to
     reporter
     qa_contact
-    component
     classification
     version
     votes
diff --git a/template/en/default/admin/custom_fields/cf-js.js.tmpl b/template/en/default/admin/custom_fields/cf-js.js.tmpl
index 331d809f6..778dd3373 100644
--- a/template/en/default/admin/custom_fields/cf-js.js.tmpl
+++ b/template/en/default/admin/custom_fields/cf-js.js.tmpl
@@ -28,7 +28,14 @@ var select_values = new Array();
 [% FOREACH sel_field = Bugzilla.get_fields({ is_select => 1 }) %]
   select_values[[% sel_field.id FILTER js %]] = [
   [% FOREACH legal_value = sel_field.legal_values %]
-    [[% legal_value.id FILTER js %], '[% legal_value.name FILTER js %]'][% ',' UNLESS loop.last %]
+    [%# Prefix components with the name of their product so that admins
+        know which component we're talking about. #%]
+    [% IF sel_field.name == 'component' %]
+      [% SET value_name = legal_value.product.name _ ': ' _ legal_value.name %]
+    [% ELSE %]
+      [% SET value_name = legal_value.name %]
+    [% END %]
+    [[% legal_value.id FILTER js %], '[% value_name FILTER js %]'][% ',' UNLESS loop.last %]
   [% END %]
   ];
 [% END %]
diff --git a/template/en/default/admin/custom_fields/edit.html.tmpl b/template/en/default/admin/custom_fields/edit.html.tmpl
index b6a8ae9bd..981668260 100644
--- a/template/en/default/admin/custom_fields/edit.html.tmpl
+++ b/template/en/default/admin/custom_fields/edit.html.tmpl
@@ -100,7 +100,10 @@
             <option value="[% value.id FILTER html %]"
              [% ' selected="selected"' 
                 IF field.visibility_value.id == value.id %]>
-              [% value.name FILTER html %]
+              [% IF field.visibility_field.name == 'component' %]
+                [% value.product.name FILTER html %]:
+              [% END %]
+              [%+ value.name FILTER html %]
             </option>
           [% END %]   
         </select>
diff --git a/template/en/default/admin/fieldvalues/create.html.tmpl b/template/en/default/admin/fieldvalues/create.html.tmpl
index f1eec1a5a..019831489 100644
--- a/template/en/default/admin/fieldvalues/create.html.tmpl
+++ b/template/en/default/admin/fieldvalues/create.html.tmpl
@@ -75,6 +75,9 @@
             [% FOREACH field_value = field.value_field.legal_values %]
               [% NEXT IF field_value.name == '' %]
               <option value="[% field_value.id FILTER none %]">
+                [% IF field.value_field.name == 'component' %]
+                  [% field_value.product.name FILTER html %]:
+                [% END %]
                 [%- field_value.name FILTER html -%]
               </option>
             [% END %]
diff --git a/template/en/default/admin/fieldvalues/edit.html.tmpl b/template/en/default/admin/fieldvalues/edit.html.tmpl
index 5650ee87b..fb58e5784 100644
--- a/template/en/default/admin/fieldvalues/edit.html.tmpl
+++ b/template/en/default/admin/fieldvalues/edit.html.tmpl
@@ -73,7 +73,10 @@
               <option value="[% field_value.id FILTER none %]"
                [% ' selected="selected"' 
                   IF field_value.id == value.visibility_value.id %]>
-                [%- field_value.name FILTER html -%]
+                [% IF field.value_field.name == 'component' %]
+                  [% field_value.product.name FILTER html %]:
+                [% END %]
+                [% field_value.name FILTER html -%]
               </option>
             [% END %]
           </select>
diff --git a/template/en/default/bug/create/create.html.tmpl b/template/en/default/bug/create/create.html.tmpl
index 4b1745757..1f3380a29 100644
--- a/template/en/default/bug/create/create.html.tmpl
+++ b/template/en/default/bug/create/create.html.tmpl
@@ -168,14 +168,6 @@ TUI_hide_default('expert_fields');
 -->
 </script>
 
-[% USE Bugzilla %]
-[% SET select_fields = {} %]
-[% FOREACH field = Bugzilla.get_fields(
-  { type => constants.FIELD_TYPE_SINGLE_SELECT, custom => 0 })
-%]
-  [% select_fields.${field.name} = field %]
-[% END %]
-
 <form name="Create" id="Create" method="post" action="post_bug.cgi"
       enctype="multipart/form-data">
 <input type="hidden" name="product" value="[% product.name FILTER html %]">
@@ -224,8 +216,8 @@ TUI_hide_default('expert_fields');
       Component</a>:
     </th>
     <td>
-      <select name="component" onchange="set_assign_to();" size="7"
-              aria-required="true" class="required">
+      <select name="component" id="component" onchange="set_assign_to();"
+              size="7" aria-required="true" class="required">
         [%# Build the lists of assignees and QA contacts if "usemenuforusers" is enabled. %]
         [% IF Param("usemenuforusers") %]
           [% assignees_list = user.get_userlist.clone %]
@@ -234,7 +226,13 @@ TUI_hide_default('expert_fields');
 
         [%- FOREACH c = product.components %]
           <option value="[% c.name FILTER html %]"
-            [% " selected=\"selected\"" IF c.name == default.component_ %]>
+                  id="v[% c.id FILTER html %]_component"
+            [% IF c.name == default.component_ %]
+              [%# This is for bug/field.html.tmpl, for visibility-related
+                # controls. %]
+              [% default.component_id = c.id %]
+              selected="selected"
+            [% END %]>
             [% c.name FILTER html -%]
           </option>
           [% IF Param("usemenuforusers") %]
@@ -245,6 +243,13 @@ TUI_hide_default('expert_fields');
           [% END %]
         [%- END %]
       </select>
+
+      <script type="text/javascript">
+       <!--
+         [%+ INCLUDE "bug/field-events.js.tmpl" 
+                    field = bug_fields.component %]
+       //-->
+       </script>
     </td>
 
     <td colspan="2">
@@ -276,19 +281,19 @@ TUI_hide_default('expert_fields');
     </td>
 
     [% INCLUDE bug/field.html.tmpl
-      bug = default, field = select_fields.bug_severity, editable = 1, 
+      bug = default, field = bug_fields.bug_severity, editable = 1, 
       value = default.bug_severity %]
   </tr>
 
   <tr>
     [% INCLUDE bug/field.html.tmpl
-      bug = default, field = select_fields.rep_platform, editable = 1,
+      bug = default, field = bug_fields.rep_platform, editable = 1,
       value = default.rep_platform %]
   </tr>
 
   <tr>
     [% INCLUDE bug/field.html.tmpl 
-       bug = default, field = select_fields.op_sys, editable = 1, 
+       bug = default, field = bug_fields.op_sys, editable = 1, 
        value = default.op_sys %]
   </tr>
 </tbody>
@@ -304,7 +309,7 @@ TUI_hide_default('expert_fields');
 
     [% IF Param('letsubmitterchoosepriority') %]
       [% INCLUDE bug/field.html.tmpl
-        bug = default, field = select_fields.priority, editable = 1, 
+        bug = default, field = bug_fields.priority, editable = 1, 
         value = default.priority %]
     [% ELSE %]
       <td colspan="2">&nbsp;</td>
@@ -679,7 +684,7 @@ TUI_hide_default('expert_fields');
        <script type="text/javascript">
         <!--
           [%+ INCLUDE "bug/field-events.js.tmpl" 
-                     field = select_fields.bug_status %]
+                     field = bug_fields.bug_status %]
         //-->
         </script>
     [% END %]
diff --git a/template/en/default/bug/edit.html.tmpl b/template/en/default/bug/edit.html.tmpl
index 379370689..63b81d733 100644
--- a/template/en/default/bug/edit.html.tmpl
+++ b/template/en/default/bug/edit.html.tmpl
@@ -30,14 +30,6 @@
 
 [% PROCESS bug/time.html.tmpl %]
 
-[% USE Bugzilla %]
-[% SET select_fields = {} %]
-[% FOREACH field = Bugzilla.get_fields(
-  { type => constants.FIELD_TYPE_SINGLE_SELECT, custom => 0 })
-%]
-  [% select_fields.${field.name} = field %]
-[% END %]
-
   <script type="text/javascript">
   <!--
   
@@ -323,7 +315,7 @@
     
     <tr>
        [% INCLUDE bug/field.html.tmpl
-            bug = bug, field = select_fields.product,
+            bug = bug, field = bug_fields.product,
             override_legal_values = bug.choices.product
             desc_url = 'describecomponents.cgi', value = bug.product
             editable = bug.check_can_change_field('product', 0, 1) %]
@@ -332,14 +324,12 @@
     [%#  Component  #%]
     [%###############%]
     <tr>
-      <td class="field_label">
-        <label for="component" accesskey="m">
-          <b><a href="describecomponents.cgi?product=[% bug.product FILTER url_quote %]">
-            Co<u>m</u>ponent</a>:
-          </b>
-        </label>
-      </td>
-      [% PROCESS select selname => "component" %]
+      [% INCLUDE bug/field.html.tmpl
+          bug = bug, field = bug_fields.component, value = bug.component
+          override_legal_values = bug.choices.component
+          desc_url = "describecomponents.cgi?product=$bug.product"
+          editable = bug.check_can_change_field('component', 0, 1)
+      %]
     </tr>
     <tr>
       <td class="field_label">
@@ -357,11 +347,11 @@
       </td>
       <td class="field_value">
        [% INCLUDE bug/field.html.tmpl
-            bug = bug, field = select_fields.rep_platform,
+            bug = bug, field = bug_fields.rep_platform,
             no_tds = 1, value = bug.rep_platform
             editable = bug.check_can_change_field('rep_platform', 0, 1) %]
        [%+ INCLUDE bug/field.html.tmpl 
-            bug = bug, field = select_fields.op_sys, 
+            bug = bug, field = bug_fields.op_sys, 
             no_tds = 1, value = bug.op_sys
             editable = bug.check_can_change_field('op_sys', 0, 1) %]
        <script type="text/javascript">
@@ -417,11 +407,11 @@
       </td>
       <td>
        [% INCLUDE bug/field.html.tmpl
-            bug = bug, field = select_fields.priority,
+            bug = bug, field = bug_fields.priority,
             no_tds = 1, value = bug.priority
             editable = bug.check_can_change_field('priority', 0, 1) %]
        [%+ INCLUDE bug/field.html.tmpl
-            bug = bug, field = select_fields.bug_severity,
+            bug = bug, field = bug_fields.bug_severity,
             no_tds = 1, value = bug.bug_severity
             editable = bug.check_can_change_field('bug_severity', 0, 1) %]
         [% IF bug.use_votes %]
@@ -895,7 +885,7 @@
 
 [% BLOCK section_customfields %]
 [%# *** Custom Fields *** %]
-
+  [% USE Bugzilla %]
   [% FOREACH field = Bugzilla.active_custom_fields %]
     <tr>
       [% PROCESS bug/field.html.tmpl value=bug.${field.name}
diff --git a/template/en/default/bug/field-events.js.tmpl b/template/en/default/bug/field-events.js.tmpl
index 06fba1245..763687e06 100644
--- a/template/en/default/bug/field-events.js.tmpl
+++ b/template/en/default/bug/field-events.js.tmpl
@@ -32,6 +32,7 @@
     [% FOREACH val = legal_value.controlled_values.$controlled_field %]
       [% cont_ids.push(val.id) %]
     [% END %]
+    [% NEXT IF !cont_ids.size %]
     showValueWhen('[% controlled_field FILTER js %]',
                   [[% cont_ids.join(',') FILTER js %]],
                   '[% field.name FILTER js %]',
diff --git a/template/en/default/bug/field.html.tmpl b/template/en/default/bug/field.html.tmpl
index bb678d79d..ac62bf7ba 100644
--- a/template/en/default/bug/field.html.tmpl
+++ b/template/en/default/bug/field.html.tmpl
@@ -40,12 +40,10 @@
   #%]
 
 [% SET hidden = 0 %]
-[% IF field.visibility_field.defined %]
-  [% IF !bug.${field.visibility_field.name}
-         .contains(field.visibility_value.name) 
-  %]
-    [% SET hidden = 1 %]
-  [% END %]
+[% IF field.visibility_field.defined AND bug
+      AND !field.visibility_value.is_set_on_bug(bug) 
+%]
+  [% SET hidden = 1 %]
 [% END %]
 
 [% IF NOT no_tds %]
@@ -138,8 +136,6 @@
             [% legal_values = field.legal_values %]
           [% END %]
           [% FOREACH legal_value = legal_values %]
-            [% SET control_value = legal_value.visibility_value %]
-            [% SET control_field = field.value_field %]
             <option value="[% legal_value.name FILTER html %]"
                     id="v[% legal_value.id FILTER html %]_
                         [%- field.name FILTER html %]"
@@ -147,10 +143,7 @@
                 # hidden %]
               [% IF value.contains(legal_value.name).size %]
                 selected="selected"
-              [% ELSIF (control_field && control_value
-                        && !bug.${control_field.name}.contains(control_value.name))
-                       || !legal_value.is_active
-              %]
+              [% ELSIF bug AND !legal_value.is_visible_on_bug(bug) %]
                 class="bz_hidden_option" disabled="disabled"
               [% END %]>
               [%- display_value(field.name, legal_value.name) FILTER html ~%]
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 79faabbfd..37429a57d 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -504,7 +504,7 @@
       it controls the visibility of the following fields:
       [%+ fields.join(', ') FILTER html %].
     [% END %]
-    [% ' and ' IF fields.size AND vals.size %]
+    [% ' Also, ' IF fields.size AND vals.size %]
     [% IF vals.size %]
       it controls the visibility of the following field values:
       <ul>
diff --git a/template/en/default/list/edit-multiple.html.tmpl b/template/en/default/list/edit-multiple.html.tmpl
index 75f2f833b..91c86cc64 100644
--- a/template/en/default/list/edit-multiple.html.tmpl
+++ b/template/en/default/list/edit-multiple.html.tmpl
@@ -276,6 +276,8 @@
   [% END %]
 
   [% USE Bugzilla %]
+  [%# Show all legal values and all fields, ignoring visibility controls. %]
+  [% bug = 0 %]
   [% FOREACH field = Bugzilla.active_custom_fields %]
     <tr>
       [% PROCESS bug/field.html.tmpl value = dontchange