Moodle

Sorry for the typo in the last line. I meant that the purpose of a checkbox is to be on or off. As it is, the checkbox field behaves now more like a radio button, requiring at least one of the options to be checked. It means it is not possible to have a checkbox field with a single checkbox.

The underlying problem is that a group of radio buttons (i.e. which all have the the same name) do not return anything at all if none of the boxes are ticked. The database code therefore "forgets" that it exists.

Aren't the field-specific behaviors supposed to be coded for each field-type independently? Could you mean checkbox group, like in checkboxes belonging to a given checkbox field, and just mistyped "radio"? Checkbox should not behave like radio buttons. It is possible to reset the text field to be empty and that works, so passing empty seems possible.

Yes - sorry I meant checkbox (not radio). I think I've fixed it anyway. What I now do is to send the ids of the fields in hidden fields. This is then used to process the fields rather than the names of the fields that happen to be in the form (or not in the case of empty checkboxes). This seems to work... just needs some more testing and I need to check the security of the thing too.

Too quick on the save button...

The underlying problem was that checkboxes with nothing checked don't exist (they are not returned at all in the $_POST) array. My solution is therefore to send the ids of the fields being edited in a hidden "array". An empty (i.e. all unchecked) set of check boxes is then immediately evident.

Reviewed code and tested. It works. Thanks.

Sorry, reopening this and raising to critical!

The patch makes the assumption that all parameters have this format:

$name = "field_$fieldid";
$value = optional_param( $name,'' );

And that's 100% a wrong assumption! Some fields like dates have:

• field_XX_day
• field_XX_month
• field_XX_year

Or text fields have:

• field_XX, for the text
• field_XX_content, for the format of the content.

So the modification isn't correct at all. Just create one database having one date field, add/edit one record and you'll see the problem in action!

Please fix ASAP, this is in stable and causing problems in any DB using dates. Ciao

ok.... working on this asap

Ok... I'm reasonably confident that this is ok now. I added another method to the field class that adds the name of the field to a list (to be displayed in the hidden fields). The standard method just adds the id as before but now things like the date field can override and add their three different field names to the list.

Phew.... panic!!!

Mmm... textarea not quite right. Just a minute

Hi Howard, I've been playing a bit with date / textarea fields and your changes look perfect! Thanks for your quick fix! B-)

Ciao

File fields have stopped working after the changes you made.
Playing with the new code, I found that class data_field_file needs this method:

function list_add_field(&$fields) {
$fields[] = $this->field->id.'_filename';
$fields[] = $this->field->id.'_file';
return true;
}

Thanks Albert.... missed that one. Onto it.

Fixed and tested. Thanks.

Howard, may you should put a few comments in the class definition about those dependencies for future reference.

Yet another regression: MDL-18628

Re-opening due to regression in field type picture - see MDL-18628.

just a question - IIUC the only problem was that un-checked checkboxes do not send any value via POST. This can be easily hacked using an empty hidden field with value "0" and with the same name as the checkbox has. Google for "checkbox hidden same name" - it is quite common solution.

So I propose to revert all patches of this, not introducing the new array, methods etc. and have things clear and simple. Feel free to re-assign to me.

hmm, looks like too many regressions happened here, I do not understand why we should break backwards compatibility in 1.9.5 when simple hidden input filed trick should solve this, right?
If somebody has custom element type it will be broken too, right?

David's right... I caused all this so I'll sort it in the morning. I thought I was simplifying the code but hadn't noticed the multiple part fields. It was all downhill after that.

These are two patches. The first one reverts the previous Howard's fix on MOODLE_19_STABLE (I am going to prepare a reverting patch for HEAD soon). The second one is very simple fix of this issue. I was trying to prepare the reverting patch carefully, keeping other not-related commits.

Why this should be re-patched:
+ small and simple fix (one-liner)
+ the trick is already being used in mform::advcheckbox and elsewhere
+ does not influence other field types, module API, custom 3rd party types etc.
+ does not introduce regressions

The patch was prepared off the git branch. To apply it, go to the dirroot and use
patch -p1 < file.patch
(note the -p1 to ignore the first path elements a/ and b/)

QA to Petr as agreed via jabber

grrrrrrrrrrrrrrrrr, watch out for Tim's and Dongsheng's commits!

Should be reverted in 1.9 branch... will get HEAD tomorrow.

Assigning back to Howard. Cross fingers man!

While not denying a need to reverse the changes for 1.9 branch, after looking at the earlier solution and the approach that Howard implemented, I see a lot of merit to what he did. Too bad that the previous implementation overloaded some elements, which he initially overlooked. I would suggest, however, to review handling of the database elements for Moodle 2 and possibly base that on his approach. Of course, a suitable solution has to be found for handling elements that have multiple components.

MY +1 for revisiting this in 2.0

Cautiously resolving this... My fixes reverted, David's one-line fix included.

Hi,

just guessing the change in API commented here:

http://docs.moodle.org/en/Moodle_1.9.5_release_notes#Changes_in_Moodle_API

Is not valid, anymore, correct? Feel free to delete it from there, plz. Ciao