Per a long thread in the dev list http://lists.drupal.org/archives/development/2006-10/msg00238.html, there seems to be agreement that there should be a separation between the UI and data layer, and that as a part of that the validation should be done by the fields rather than the widgets, so a task that needs to be done is to move validation from the widgets to the fields in all the core modules. Also, there are differences between the widgets and the fields and between 4.7 and 5.x on the terminology for the array of field data, in some places it's called $items and in others $node_field. Both parameters refer to identical arrays, so it would reduce confusion to always use the same name. I prefer $items because it doesn't get confused with $node, and it's a shorter name.

Some modules have already had these changes made (number.module and the date.module), but several others in core still need updating.

I'll be ready to make this change shortly.

Comments

karens’s picture

karens commented
Status: Active » Fixed

These changes have been committed. This will also pave the way for the standarization of the default value.

yched’s picture

yched commented

Karen, just to be sure : is our goal here to remove the 'validate' op completely for widgets ?

I'm not trying to revive the ML thread your're quoting, but it seems that _some_ level of validation belongs to the widget. (the same way you can assign a '#validate' callback to a form element in FAPI, and then have a global validation callback for your form - or at least I _think_ you can, I might have to check this...)

Widgets handle form 'elements', so they should provide some validation for the specific form of input they collect. Then the field performs 'data'-level validation. I'm mainly thinking about noderef here : having the field 'validate' op perform autocomplete-specific stuff ([nid:nn] pattern...) feels weird.
Or maybe this can be managed in widget's 'process form values' ? I'll try to investigate further...

karens’s picture

karens commented

I don't think there is a 'right' or 'wrong' answer, although the tone on the dev list was pretty strongly in favor of keeping the UI and data layer separate. Here's my thinking:

We have this whole big debate about what is the difference between fields and widgets. To the extent they do the same things they confuse everyone even more. I'd like to draw nice clear lines between them -- widgets pull the value out of the database and massage it if necessary to present in the UI, then massage the results back into the format that goes in the database. Fields are responsible for storing the data, hence they must determine if the data is valid. So widgets handle 'prepare form values', 'form', and 'process form values' (and fields do none of those), and fields do 'validate', 'insert', and 'delete' (and widgets do none of those).

Plus, we already know that limiting validation to the fields will work better when we add in our standardized methods of handling default values, which is just one more reason to do it.

I don't know if I am suggesting we get rid of the validation op for widgets, although I think that might make sense. For now at least we could try not using it and see if there are any problems that result.

For whatever it's worth, there's one more operation they both perform -- 'submit'. I don't know what, if anything, should be done with that one.

karens’s picture

karens commented

I wonder if the nodereference autocomplete stuff is really form processing rather than validation?? I guess the big problem is form processing doesn't necessarily inject into the right point in the processing to throw up a form validation error.

Maybe it will have to go back as it was, but let's see what works...

yched’s picture

yched commented

I updated noderef and userref so that they work OK with php default values.

I think the bottomline is this :
field-level validation can only operate on data in the final form. It cannot rely on any knowledge of the widget's 'shape' and format (this is clear concept separation IMO, plus the widget might be provided by an external module anyway)
For most widgets, no widget-level validation should be required, only data massaging, and as a general rule we should try to avoid it.
But I do think there are cases where it is needed, and where the field is not the proper place to do it, precisely according to the 'keep UI and data levels separate'. Some widgets require validation steps that others won't require.

For instance, I kept one validation step in noderef autocomplete widget :
This widgets accepts entries in the form 'node title [nid:n]'. The [nid:n] part is added by the autocomplete stuff, or could be manually entered (or altered) by the user.
What if the title does not correspond to the nid ?
So far we issued an error : 'check your input'. This check is necessarily performed in the widget's 'validate' op.
(we might decide to accept the input anyway and favor the [nid:n] part, but this is a matter of choice and does not invalidate the above IMO)

karens’s picture

karens commented

Well I think you could still make a case that it is the field which should say that the data is not valid, but I don't feel strongly enough to get too excited about it. So I guess we'll leave it that in general validation should be done by the field, but there may be a few exceptions. Which means we still need 'validate' op for the widget.

If anyone wants to debate this further in the future, they can open a new issue :-)

yched’s picture

yched commented

Which means we still need 'validate' op for the widget.
Not necessarily. Dopry mentioned the idea of having the widgets validate through the '#validate' FAPI tag.
Why not, it could alleviate the confusion between field and widgets 'validate' op.

dopry’s picture

dopry commented

I agree with yched on this one, widgets do need some level of validation, especially if you're planning on creating relatively intelligent or service interface widgets. However that said, it can all be handled through the form processing pipeline. #validate, #process... I'm not sure that there is a FormAPI equivalent of prepare, yet. I'm hoping to see a slow merging of the concept of widget and form elements. Maybe one day we'll have a simple hook_widget_settings which specifies which form elements are valid widgets(op == info) for a particular field type and provides a settings form for the widget(op == form) and content.module handles the rest. I'll try to have some working code soon.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)