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

          Attachments

            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 Araujo 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 Araujo 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: