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
    • Rank:
      3877

      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.

        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 Škoda added a comment -

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

        Show
        Petr Škoda 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: