diff --git a/lib/form/advcheckbox.php b/lib/form/advcheckbox.php index 349fb8c02fd..4fe4650d6e1 100644 --- a/lib/form/advcheckbox.php +++ b/lib/form/advcheckbox.php @@ -87,24 +87,6 @@ class MoodleQuickForm_advcheckbox extends HTML_QuickForm_advcheckbox{ function getHelpButton(){ return $this->_helpbutton; } - /** - * Automatically generates and assigns an 'id' attribute for the element. - * - * Currently used to ensure that labels work on radio buttons and - * advcheckboxes. Per idea of Alexander Radivanovich. - * Overriden in moodleforms to remove qf_ prefix. - * - * @access private - * @return void - */ - function _generateId() - { - static $idx = 1; - - if (!$this->getAttribute('id')) { - $this->updateAttributes(array('id' => 'id_'.substr(md5(microtime() . $idx++), 0, 6))); - } - } // end func _generateId function toHtml() { diff --git a/lib/form/checkbox.php b/lib/form/checkbox.php index 237c796a149..bbccbc08600 100644 --- a/lib/form/checkbox.php +++ b/lib/form/checkbox.php @@ -36,24 +36,7 @@ class MoodleQuickForm_checkbox extends HTML_QuickForm_checkbox{ function getHelpButton(){ return $this->_helpbutton; } - /** - * Automatically generates and assigns an 'id' attribute for the element. - * - * Currently used to ensure that labels work on radio buttons and - * checkboxes. Per idea of Alexander Radivanovich. - * Overriden in moodleforms to remove qf_ prefix. - * - * @access private - * @return void - */ - function _generateId() - { - static $idx = 1; - if (!$this->getAttribute('id')) { - $this->updateAttributes(array('id' => 'id_'.substr(md5(microtime() . $idx++), 0, 6))); - } - } // end func _generateId /** * Called by HTML_QuickForm whenever form event is made on this element * diff --git a/lib/form/radio.php b/lib/form/radio.php index 3dc03a4a7dd..0287b0ff486 100644 --- a/lib/form/radio.php +++ b/lib/form/radio.php @@ -36,24 +36,7 @@ class MoodleQuickForm_radio extends HTML_QuickForm_radio{ function getHelpButton(){ return $this->_helpbutton; } - /** - * Automatically generates and assigns an 'id' attribute for the element. - * - * Currently used to ensure that labels work on radio buttons and - * checkboxes. Per idea of Alexander Radivanovich. - * Overriden in moodleforms to remove qf_ prefix. - * - * @access private - * @return void - */ - function _generateId() - { - static $idx = 1; - if (!$this->getAttribute('id')) { - $this->updateAttributes(array('id' => 'id_'.substr(md5(microtime() . $idx++), 0, 6))); - } - } // end func _generateId /** * Slightly different container template when frozen. * diff --git a/lib/form/select.php b/lib/form/select.php index f6bcfbd10e6..e76cd5718bf 100644 --- a/lib/form/select.php +++ b/lib/form/select.php @@ -31,24 +31,7 @@ class MoodleQuickForm_select extends HTML_QuickForm_select{ return parent::toHtml(); } } - /** - * Automatically generates and assigns an 'id' attribute for the element. - * - * Currently used to ensure that labels work on radio buttons and - * checkboxes. Per idea of Alexander Radivanovich. - * Overriden in moodleforms to remove qf_ prefix. - * - * @access private - * @return void - */ - function _generateId() - { - static $idx = 1; - if (!$this->getAttribute('id')) { - $this->updateAttributes(array('id' => 'id_'. substr(md5(microtime() . $idx++), 0, 6))); - } - } // end func _generateId /** * set html for help button * diff --git a/lib/form/selectgroups.php b/lib/form/selectgroups.php index 01ee9ed3336..92b5ea05450 100644 --- a/lib/form/selectgroups.php +++ b/lib/form/selectgroups.php @@ -548,24 +548,6 @@ class MoodleQuickForm_selectgroups extends HTML_QuickForm_element { function setHiddenLabel($hiddenLabel){ $this->_hiddenLabel = $hiddenLabel; } - /** - * Automatically generates and assigns an 'id' attribute for the element. - * - * Currently used to ensure that labels work on radio buttons and - * checkboxes. Per idea of Alexander Radivanovich. - * Overriden in moodleforms to remove qf_ prefix. - * - * @access private - * @return void - */ - function _generateId() - { - static $idx = 1; - - if (!$this->getAttribute('id')) { - $this->updateAttributes(array('id' => 'id_'. substr(md5(microtime() . $idx++), 0, 6))); - } - } // end func _generateId /** * set html for help button * diff --git a/lib/form/selectwithlink.php b/lib/form/selectwithlink.php index ccdd55a2599..637ce6a28cb 100644 --- a/lib/form/selectwithlink.php +++ b/lib/form/selectwithlink.php @@ -64,24 +64,7 @@ class MoodleQuickForm_selectwithlink extends HTML_QuickForm_select{ return $retval; } - /** - * Automatically generates and assigns an 'id' attribute for the element. - * - * Currently used to ensure that labels work on radio buttons and - * checkboxes. Per idea of Alexander Radivanovich. - * Overriden in moodleforms to remove qf_ prefix. - * - * @access private - * @return void - */ - function _generateId() - { - static $idx = 1; - if (!$this->getAttribute('id')) { - $this->updateAttributes(array('id' => 'id_'. substr(md5(microtime() . $idx++), 0, 6))); - } - } // end func _generateId /** * set html for help button * diff --git a/lib/form/text.php b/lib/form/text.php index 2e119b0abef..7d36b944e4b 100644 --- a/lib/form/text.php +++ b/lib/form/text.php @@ -32,24 +32,7 @@ class MoodleQuickForm_text extends HTML_QuickForm_text{ return parent::toHtml(); } } - /** - * Automatically generates and assigns an 'id' attribute for the element. - * - * Currently used to ensure that labels work on radio buttons and - * checkboxes. Per idea of Alexander Radivanovich. - * Overriden in moodleforms to remove qf_ prefix. - * - * @access private - * @return void - */ - function _generateId() - { - static $idx = 1; - if (!$this->getAttribute('id')) { - $this->updateAttributes(array('id' => 'id_'. substr(md5(microtime() . $idx++), 0, 6))); - } - } // end func _generateId /** * set html for help button * diff --git a/lib/form/url.php b/lib/form/url.php index 8822aa8ec46..dd38f92cfe7 100644 --- a/lib/form/url.php +++ b/lib/form/url.php @@ -80,24 +80,7 @@ EOD; return $str; } - /** - * Automatically generates and assigns an 'id' attribute for the element. - * - * Currently used to ensure that labels work on radio buttons and - * checkboxes. Per idea of Alexander Radivanovich. - * Overriden in moodleforms to remove qf_ prefix. - * - * @access private - * @return void - */ - function _generateId() - { - static $idx = 1; - if (!$this->getAttribute('id')) { - $this->updateAttributes(array('id' => 'id_'. substr(md5(microtime() . $idx++), 0, 6))); - } - } // end func _generateId /** * set html for help button * diff --git a/lib/formslib.php b/lib/formslib.php index ca83c7c6001..c69b6147750 100644 --- a/lib/formslib.php +++ b/lib/formslib.php @@ -2305,17 +2305,8 @@ class MoodleQuickForm_Renderer extends HTML_QuickForm_Renderer_Tableless{ * @param mixed $error */ function renderElement(&$element, $required, $error){ - //manipulate id of all elements before rendering - if (!is_null($element->getAttribute('id'))) { - $id = $element->getAttribute('id'); - } else { - $id = $element->getName(); - } - //strip qf_ prefix and replace '[' with '_' and strip ']' - $id = preg_replace(array('/^qf_|\]/', '/\[/'), array('', '_'), $id); - if (strpos($id, 'id_') !== 0){ - $element->updateAttributes(array('id'=>'id_'.$id)); - } + // Make sure the element has an id. + $element->_generateId(); //adding stuff to place holders in template //check if this is a group element first diff --git a/lib/pear/HTML/QuickForm/checkbox.php b/lib/pear/HTML/QuickForm/checkbox.php index 55c7574c57e..8b79c123fa3 100644 --- a/lib/pear/HTML/QuickForm/checkbox.php +++ b/lib/pear/HTML/QuickForm/checkbox.php @@ -64,7 +64,6 @@ class HTML_QuickForm_checkbox extends HTML_QuickForm_input $this->_text = $text; $this->setType('checkbox'); $this->updateAttributes(array('value'=>1)); - $this->_generateId(); } //end constructor // }}} diff --git a/lib/pear/HTML/QuickForm/element.php b/lib/pear/HTML/QuickForm/element.php index fccad5935cb..a0a600c45f5 100644 --- a/lib/pear/HTML/QuickForm/element.php +++ b/lib/pear/HTML/QuickForm/element.php @@ -415,14 +415,16 @@ class HTML_QuickForm_element extends HTML_Common * @access private * @return void */ - function _generateId() - { - static $idx = 1; - - if (!$this->getAttribute('id')) { - $this->updateAttributes(array('id' => 'qf_' . substr(md5(microtime() . $idx++), 0, 6))); + function _generateId() { + if ($this->getAttribute('id')) { + return; } - } // end func _generateId + + $id = $this->getName(); + $id = 'id_' . str_replace(array('qf_', '[', ']'), array('', '_', ''), $id); + $id = clean_param($id, PARAM_ALPHANUMEXT); + $this->updateAttributes(array('id' => $id)); + } // }}} // {{{ exportValue() diff --git a/lib/pear/HTML/QuickForm/radio.php b/lib/pear/HTML/QuickForm/radio.php index 963168b06ac..26320267668 100644 --- a/lib/pear/HTML/QuickForm/radio.php +++ b/lib/pear/HTML/QuickForm/radio.php @@ -66,10 +66,24 @@ class HTML_QuickForm_radio extends HTML_QuickForm_input $this->_persistantFreeze = true; $this->setType('radio'); $this->_text = $text; - $this->_generateId(); } //end constructor - + // }}} + + function _generateId() { + // Override the standard implementation, since you can have multiple + // check-boxes with the same name on a form. Therefore, add the + // (cleaned up) value to the id. + + if ($this->getAttribute('id')) { + return; + } + + parent::_generateId(); + $id = $this->getAttribute('id') . '_' . clean_param($this->getValue(), PARAM_ALPHANUMEXT); + $this->updateAttributes(array('id' => $id)); + } + // {{{ setChecked() /** diff --git a/lib/simpletest/testformslib.php b/lib/simpletest/testformslib.php index 03308956cf8..2d8d788e2cf 100644 --- a/lib/simpletest/testformslib.php +++ b/lib/simpletest/testformslib.php @@ -28,6 +28,9 @@ if (!defined('MOODLE_INTERNAL')) { die('Direct access to this script is forbidden.'); /// It must be included from a Moodle page } require_once($CFG->libdir . '/formslib.php'); +require_once($CFG->libdir . '/form/radio.php'); +require_once($CFG->libdir . '/form/select.php'); +require_once($CFG->libdir . '/form/text.php'); class formslib_test extends UnitTestCase { @@ -116,4 +119,104 @@ class formslib_test extends UnitTestCase { $CFG->strictformsrequired = $strictformsrequired; } + public function test_generate_id_select() { + $el = new MoodleQuickForm_select('choose_one', 'Choose one', + array(1 => 'One', '2' => 'Two')); + $el->_generateId(); + $this->assertEqual('id_choose_one', $el->getAttribute('id')); + } + + public function test_generate_id_like_repeat() { + $el = new MoodleQuickForm_text('text[7]', 'Type something'); + $el->_generateId(); + $this->assertEqual('id_text_7', $el->getAttribute('id')); + } + + public function test_can_manually_set_id() { + $el = new MoodleQuickForm_text('elementname', 'Type something', + array('id' => 'customelementid')); + $el->_generateId(); + $this->assertEqual('customelementid', $el->getAttribute('id')); + } + + public function test_generate_id_radio() { + $el = new MoodleQuickForm_radio('radio', 'Label', 'Choice label', 'choice_value'); + $el->_generateId(); + $this->assertEqual('id_radio_choice_value', $el->getAttribute('id')); + } + + public function test_radio_can_manually_set_id() { + $el = new MoodleQuickForm_radio('radio2', 'Label', 'Choice label', 'choice_value', + array('id' => 'customelementid2')); + $el->_generateId(); + $this->assertEqual('customelementid2', $el->getAttribute('id')); + } + + public function test_generate_id_radio_like_repeat() { + $el = new MoodleQuickForm_radio('repeatradio[2]', 'Label', 'Choice label', 'val'); + $el->_generateId(); + $this->assertEqual('id_repeatradio_2_val', $el->getAttribute('id')); + } + + public function test_rendering() { + $form = new formslib_test_form(); + ob_start(); + $form->display(); + $html = ob_get_clean(); + + $this->assert(new ContainsTagWithAttributes('select', array( + 'id' => 'id_choose_one', 'name' => 'choose_one')), $html); + + $this->assert(new ContainsTagWithAttributes('input', array( + 'type' => 'text', 'id' => 'id_text_0', 'name' => 'text[0]')), $html); + + $this->assert(new ContainsTagWithAttributes('input', array( + 'type' => 'text', 'id' => 'id_text_1', 'name' => 'text[1]')), $html); + + $this->assert(new ContainsTagWithAttributes('input', array( + 'type' => 'radio', 'id' => 'id_radio_choice_value', + 'name' => 'radio', 'value' => 'choice_value')), $html); + + $this->assert(new ContainsTagWithAttributes('input', array( + 'type' => 'radio', 'id' => 'customelementid2', 'name' => 'radio2')), $html); + + $this->assert(new ContainsTagWithAttributes('input', array( + 'type' => 'radio', 'id' => 'id_repeatradio_0_2', + 'name' => 'repeatradio[0]', 'value' => '2')), $html); + + $this->assert(new ContainsTagWithAttributes('input', array( + 'type' => 'radio', 'id' => 'id_repeatradio_2_1', + 'name' => 'repeatradio[2]', 'value' => '1')), $html); + + $this->assert(new ContainsTagWithAttributes('input', array( + 'type' => 'radio', 'id' => 'id_repeatradio_2_2', + 'name' => 'repeatradio[2]', 'value' => '2')), $html); + } +} + + +/** + * Test form to be used by {@link formslib_test::test_rendering()}. + */ +class formslib_test_form extends moodleform { + public function definition() { + $this->_form->addElement('select', 'choose_one', 'Choose one', + array(1 => 'One', '2' => 'Two')); + + $repeatels = array( + $this->_form->createElement('text', 'text', 'Type something') + ); + $this->repeat_elements($repeatels, 2, array(), 'numtexts', 'addtexts'); + + $this->_form->addElement('radio', 'radio', 'Label', 'Choice label', 'choice_value'); + + $this->_form->addElement('radio', 'radio2', 'Label', 'Choice label', 'choice_value', + array('id' => 'customelementid2')); + + $repeatels = array( + $this->_form->createElement('radio', 'repeatradio', 'Choose {no}', 'One', 1), + $this->_form->createElement('radio', 'repeatradio', 'Choose {no}', 'Two', 2), + ); + $this->repeat_elements($repeatels, 3, array(), 'numradios', 'addradios'); + } } \ No newline at end of file