Merge branch 'MDL-30168_tim2_21' of git://github.com/timhunt/moodle i…

…nto MOODLE_21_STABLE
moodle · Jan 2, 2012 · 9b82d1b · 9b82d1b
2 parents 19bfd15 + 515ff4d
commit 9b82d1b
Show file tree

Hide file tree

Showing 13 changed files with 130 additions and 159 deletions.
diff --git a/lib/form/advcheckbox.php b/lib/form/advcheckbox.php
@@ -87,24 +87,6 @@ function setHelpButton($helpbuttonargs, $function='helpbutton'){
     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
@@ -36,24 +36,7 @@ function setHelpButton($helpbuttonargs, $function='helpbutton'){
     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
@@ -36,24 +36,7 @@ function setHelpButton($helpbuttonargs, $function='helpbutton'){
     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
@@ -31,24 +31,7 @@ function toHtml(){
              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
@@ -548,24 +548,6 @@ function onQuickFormEvent($event, $arg, &$caller)
     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
@@ -64,24 +64,7 @@ function toHtml(){
 
         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
@@ -32,24 +32,7 @@ function toHtml(){
              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
@@ -80,24 +80,7 @@ function toHtml(){
 
         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
@@ -2280,17 +2280,8 @@ function startGroup(&$group, $required, $error){
      * @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

diff --git a/lib/pear/HTML/QuickForm/checkbox.php b/lib/pear/HTML/QuickForm/checkbox.php
@@ -64,7 +64,6 @@ function HTML_QuickForm_checkbox($elementName=null, $elementLabel=null, $text=''
         $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
@@ -415,14 +415,16 @@ function accept(&$renderer, $required=false, $error=null)
     * @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()

diff --git a/lib/pear/HTML/QuickForm/radio.php b/lib/pear/HTML/QuickForm/radio.php
@@ -66,10 +66,24 @@ function HTML_QuickForm_radio($elementName=null, $elementLabel=null, $text=null,
         $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
@@ -28,6 +28,9 @@
     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 @@ public function test_require_rule() {
         $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');
+    }
 }