Merge branch 'MDL-30168_tim2_22' of git://github.com/timhunt/moodle into MOODLE_22_STABLE

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()
     {

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
      *

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.
      *

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
      *

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
      *

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
      *

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
      *

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
      *

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
+        // Make sure the element has an id.
-        if (!is_null($element->getAttribute('id'))) {
+        $element->_generateId();
-            $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));
-        }
 
         //adding stuff to place holders in template
         //check if this is a group element first

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
     
     // }}}

lib/pear/HTML/QuickForm/element.php

@@ -415,14 +415,16 @@ class HTML_QuickForm_element extends HTML_Common
     * @access private
     * @return void 
     */
-    function _generateId()
+    function _generateId() {
-    {
+        if ($this->getAttribute('id')) {
-        static $idx = 1;
+            return;
-
-        if (!$this->getAttribute('id')) {
-            $this->updateAttributes(array('id' => 'qf_' . substr(md5(microtime() . $idx++), 0, 6)));
         }
-    } // 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()

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()
 
     /**

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');
+    }
 }