Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-11537

Allow to set form element 'checkbox' value to something different than 1

    Details

    • Type: Improvement
    • Status: Reopened
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: 1.9, 3.0
    • Fix Version/s: None
    • Component/s: Forms Library
    • Labels:
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_30_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
          bazaar 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
          bazaar 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
          jamiesensei 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
          jamiesensei 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
          bazaar 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
          bazaar 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
          bazaar Mawuli Kuivi added a comment - - edited

          Any news on this request?

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

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

          Show
          skodak Petr Skoda added a comment - lowering priority - this does not break anything, developers may work around this - sorry
          Hide
          cfulton 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
          cfulton 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
          timhunt 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
          timhunt 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
          stronk7 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
          stronk7 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
          stronk7 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
          stronk7 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
          Hide
          danielneis Daniel Neis added a comment - - edited

          Hello,

          this is very annoying.
          Why people must have to use "advcheckbox" to only pass a value?
          A common "checkbox" should be enough... Also, how do you imagine people would know that?
          I've googled and found this bug after some minutes thinking about it ...
          Another annoying thing is the "id" being tied to "name", so i can't use names like "myfield[]" to have arrays... maybe shuold use addGroup() to this, but common checkbox should acomplish that too.

          Kind regards,
          Daniel

          Show
          danielneis Daniel Neis added a comment - - edited Hello, this is very annoying. Why people must have to use "advcheckbox" to only pass a value? A common "checkbox" should be enough... Also, how do you imagine people would know that? I've googled and found this bug after some minutes thinking about it ... Another annoying thing is the "id" being tied to "name", so i can't use names like "myfield[]" to have arrays... maybe shuold use addGroup() to this, but common checkbox should acomplish that too. Kind regards, Daniel
          Hide
          marina Marina Glancy added a comment -

          Hello, this will be a nice improvement to the forms library. The patch from the community is welcome. We can not consider this a bug and integrate in released versions because it can affect plugins expectations

          Show
          marina Marina Glancy added a comment - Hello, this will be a nice improvement to the forms library. The patch from the community is welcome. We can not consider this a bug and integrate in released versions because it can affect plugins expectations

            People

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

              Dates

              • Created:
                Updated: