Cusotom field label should not be associated to the first input (#25760).

git-svn-id: http://svn.redmine.org/redmine/trunk@16574 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/helpers/custom_fields_helper.rb

@@ -96,16 +96,23 @@ module CustomFieldsHelper
   # Return custom field label tag
   def custom_field_label_tag(name, custom_value, options={})
     required = options[:required] || custom_value.custom_field.is_required?
+    for_tag_id = options.fetch(:for_tag_id, "#{name}_custom_field_values_#{custom_value.custom_field.id}")
     content = custom_field_name_tag custom_value.custom_field
 
     content_tag "label", content +
       (required ? " <span class=\"required\">*</span>".html_safe : ""),
-      :for => "#{name}_custom_field_values_#{custom_value.custom_field.id}"
+      :for => for_tag_id
   end
 
   # Return custom field tag with its label tag
   def custom_field_tag_with_label(name, custom_value, options={})
-    custom_field_label_tag(name, custom_value, options) + custom_field_tag(name, custom_value)
+    tag = custom_field_tag(name, custom_value)
+    tag_id = nil
+    ids = tag.scan(/ id="(.+?)"/)
+    if ids.size == 1
+      tag_id = ids.first.first
+    end
+    custom_field_label_tag(name, custom_value, options.merge(:for_tag_id => tag_id)) + tag
   end
 
   # Returns the custom field tag for when bulk editing objects

lib/redmine/field_format.rb

@@ -620,13 +620,11 @@ module Redmine
         opts.each do |label, value|
           value ||= label
           checked = (custom_value.value.is_a?(Array) && custom_value.value.include?(value)) || custom_value.value.to_s == value
-          tag = view.send(tag_method, tag_name, value, checked, :id => tag_id)
-          # set the id on the first tag only
-          tag_id = nil
+          tag = view.send(tag_method, tag_name, value, checked, :id => nil)
           s << view.content_tag('label', tag + ' ' + label) 
         end
         if custom_value.custom_field.multiple?
-          s << view.hidden_field_tag(tag_name, '')
+          s << view.hidden_field_tag(tag_name, '', :id => nil)
         end
         css = "#{options[:class]} check_box_group"
         view.content_tag('span', s, options.merge(:class => css))

test/unit/helpers/custom_fields_helper_test.rb

@@ -40,6 +40,36 @@ class CustomFieldsHelperTest < Redmine::HelperTest
     assert_select_in tag, 'label span[title]', 0
   end
 
+  def test_label_tag_should_include_for_attribute_for_select_tag
+    field = CustomField.new(:name => 'Foo', :field_format => 'list')
+    s = custom_field_tag_with_label('foo', CustomValue.new(:custom_field => field))
+    assert_select_in s, 'label[for]'
+  end
+
+  def test_label_tag_should_not_include_for_attribute_for_checkboxes
+    field = CustomField.new(:name => 'Foo', :field_format => 'list', :edit_tag_style => 'check_box')
+    s = custom_field_tag_with_label('foo', CustomValue.new(:custom_field => field))
+    assert_select_in s, 'label:not([for])'
+  end
+
+  def test_label_tag_should_include_for_attribute_for_bool_as_select_tag
+    field = CustomField.new(:name => 'Foo', :field_format => 'bool')
+    s = custom_field_tag_with_label('foo', CustomValue.new(:custom_field => field))
+    assert_select_in s, 'label[for]'
+  end
+
+  def test_label_tag_should_include_for_attribute_for_bool_as_checkbox
+    field = CustomField.new(:name => 'Foo', :field_format => 'bool', :edit_tag_style => 'check_box')
+    s = custom_field_tag_with_label('foo', CustomValue.new(:custom_field => field))
+    assert_select_in s, 'label[for]'
+  end
+
+  def test_label_tag_should_not_include_for_attribute_for_bool_as_radio
+    field = CustomField.new(:name => 'Foo', :field_format => 'bool', :edit_tag_style => 'radio')
+    s = custom_field_tag_with_label('foo', CustomValue.new(:custom_field => field))
+    assert_select_in s, 'label:not([for])'
+  end
+
   def test_unknow_field_format_should_be_edited_as_string
     field = CustomField.new(:field_format => 'foo')
     value = CustomValue.new(:value => 'bar', :custom_field => field)