Moodle
  1. Moodle
  2. MDL-11537

Checkbox value always set to 1 no matter what you set in the addElement function

    Details

    • Type: Bug Bug
    • Status: Reopened
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 1.9
    • Fix Version/s: None
    • Component/s: Forms Library
    • Labels:
      None
    • Affected Branches:
      MOODLE_19_STABLE

      Description

      In the file

      {moodesite}

      /lib/pear/HTML/QuickForm/checkbox.php there is a problem in the definition.

      function HTML_QuickForm_checkbox($elementName=null, $elementLabel=null, $text='', $attributes=null)

      { HTML_QuickForm_input::HTML_QuickForm_input($elementName, $elementLabel, $attributes); $this->_persistantFreeze = true; $this->_text = $text; $this->setType('checkbox'); $this->updateAttributes(array('value'=>1)); $this->_generateId(); }

      //end constructor

      As you see, the value is always set to 1 no matter what developer sets in the addElement function.

      I think you need to check to see if a value if given. If one is given, no need to set the value =1.
      For example,
      function HTML_QuickForm_checkbox($elementName=null, $elementLabel=null, $text='', $attributes=null)
      {
      HTML_QuickForm_input::HTML_QuickForm_input($elementName, $elementLabel, $attributes);
      $this->_persistantFreeze = true;
      $this->_text = $text;
      $this->setType('checkbox');
      if (!isset($this->_attributes['value']) )

      { $this->updateAttributes(array('value'=>1)); }

      $this->_generateId();
      } //end constructor

      This way, the value is only set to 1 if no value is given.

        Gliffy Diagrams

          Activity

          Hide
          Mawuli Kuivi added a comment -

          In doing more research on this, l found that the forms element has an array called [_elementIndex] => Array

          This array has the location of the items in the array.

          The getElementType function in QuickForm.php depends on this.

          But when you had items to a group, the items are not part of this index hence getElementType fails.

          It would be nice if getElementType can recurse or somehow find item types in a Groups. Or just add the items in the Groups to this index array.

          Show
          Mawuli Kuivi added a comment - In doing more research on this, l found that the forms element has an array called [_elementIndex] => Array This array has the location of the items in the array. The getElementType function in QuickForm.php depends on this. But when you had items to a group, the items are not part of this index hence getElementType fails. It would be nice if getElementType can recurse or somehow find item types in a Groups. Or just add the items in the Groups to this index array.
          Hide
          Jamie Pratt added a comment -

          You can use advcheckbox that also allows you to set a value to be passed by get_data when the checkbox is not checked. You are right checkbox only allows you to set a value of 1. You could process the data from the form and test for the value 1 been returned by the form and set it to another value if necessary.

          Show
          Jamie Pratt added a comment - You can use advcheckbox that also allows you to set a value to be passed by get_data when the checkbox is not checked. You are right checkbox only allows you to set a value of 1. You could process the data from the form and test for the value 1 been returned by the form and set it to another value if necessary.
          Hide
          Mawuli Kuivi added a comment -

          You are right. I could do that by checking for the value equal to 1 but would be better if you can set the value to whatever you want.

          If someone does not provide a value, then you create a default. The way it is no is not the best or not very good.

          Show
          Mawuli Kuivi added a comment - You are right. I could do that by checking for the value equal to 1 but would be better if you can set the value to whatever you want. If someone does not provide a value, then you create a default. The way it is no is not the best or not very good.
          Hide
          Mawuli Kuivi added a comment - - edited

          Any news on this request?

          Show
          Mawuli Kuivi added a comment - - edited Any news on this request?
          Hide
          Petr Skoda added a comment -

          lowering priority - this does not break anything, developers may work around this - sorry

          Show
          Petr Skoda added a comment - lowering priority - this does not break anything, developers may work around this - sorry
          Hide
          Charles Fulton added a comment -

          I beg to differ, but this is clearly a bug: there are valid reasons to pass a value other than 1 to a checkbox, especially if you're using an array of checkboxes. Having a value of 1 as a default makes sense, yes, but not if the developer is explicitly passing a different value.

          Show
          Charles Fulton added a comment - I beg to differ, but this is clearly a bug: there are valid reasons to pass a value other than 1 to a checkbox, especially if you're using an array of checkboxes. Having a value of 1 as a default makes sense, yes, but not if the developer is explicitly passing a different value.
          Hide
          Tim Hunt added a comment -

          I agree this should be closed WONTFIX.

          First, you almost always want to use advcheckbox anyway.
          Second, this bug is in the third-party HTMLQuickForms code, and we don't like changing the code of third-party libraries unless it is really necessary.

          Show
          Tim Hunt added a comment - I agree this should be closed WONTFIX. First, you almost always want to use advcheckbox anyway. Second, this bug is in the third-party HTMLQuickForms code, and we don't like changing the code of third-party libraries unless it is really necessary.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          U P S T R E A M I Z E D !

          Many thanks, this is now available in all the repos (git & cvs).

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - U P S T R E A M I Z E D ! Many thanks, this is now available in all the repos (git & cvs). Closing, ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Doh,

          somehow this issue was closed incorrectly when processing all the integrated issues this week. (sort of most voted and current in integration filters mix). Apologies for the confusion, reseting to previous status!

          Ciao, Eloy

          Show
          Eloy Lafuente (stronk7) added a comment - Doh, somehow this issue was closed incorrectly when processing all the integrated issues this week. (sort of most voted and current in integration filters mix). Apologies for the confusion, reseting to previous status! Ciao, Eloy

            People

            • Votes:
              19 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated: