Problem

If a Date field is added to a node and is hidden with hide(), the module will try to validate the content of the field.

This will lead to validation errors in some cases, but not others. A Date field with a Text widget will always throw validation errors on new content as well as existing content. Other widgets for other field types like Unix or ISO timestamps will only seem to cause problems with new content (sometimes even this seems inconsistent).

The text widget for a Date field will cause this error to appear...

There are errors in FIELD_LABEL value #1:

The dates are invalid.

This is reliably caused when trying to create a new node with a field widget obscured by hide() in a module's hook_form_alter().

Workaround

For now a simple workaround is to add a 'display: none;' style to the form element.

CommentFileSizeAuthor
#13 date-hide-validation-test-1419286-13.patch5.6 KBwwedding
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

KarenS’s picture

KarenS CreditAttribution: KarenS commented
Status: Active » Postponed (maintainer needs more info)

Using hide() as it is normally used, in templates, works fine. I have never seen it used any other way, nor do I see any example in core of using it any other way.

I have no idea how you're trying to use this in a form, nor any idea what errors you are seeing.

Nothing I can do without more information.

wwedding’s picture

wwedding CreditAttribution: wwedding commented

I haven't encountered any other field that has different behavior depending on where hide() is called, so that aspect had not occurred to me.

This is specific to the hook_form_BASE_FORM_ID_alter() hook, at the module level.

As specific as I can get, this is being called on the 'commerce_order_ui_order_form' presented by the Drupal Commerce module for a 'commerce_order' entity.

Error message is...

There are errors in FIELD_LABEL value #1:

The dates are invalid.

FIELD_LABEL is a placeholder, the actual error message correctly displays the field label. This message is technically correct as far as $form_state is concerned. The issue is what gets placed into $form_state.

KarenS’s picture

KarenS CreditAttribution: KarenS commented
Project: Date » Commerce Core
Version: 7.x-2.x-dev » 7.x-1.x-dev
Component: Date Field » Order
Status: Postponed (maintainer needs more info) » Active

Moving this to Drupal Commerce. I have no idea how that module works or what it may be trying to do.

wwedding’s picture

wwedding CreditAttribution: wwedding commented

It's just a regular form, so I'm pretty sure this isn't a Commerce problem. Particularly considering the Date field's other numerous issues with form manipulation (like #access changes in other now-resolved issues).

amateescu’s picture

amateescu CreditAttribution: amateescu commented
Category: bug » support
Status: Active » Postponed (maintainer needs more info)

I don't think hide() is supposed to be used on forms, but on render arrays. Any reason for you to not use the #access property instead?

Also, can you try to reproduce this in a regular (not provided by Commerce) form?

wwedding’s picture

wwedding CreditAttribution: wwedding commented
Project: Commerce Core » Date
Issue summary: View changes

Added a workaround

wwedding’s picture

wwedding CreditAttribution: wwedding commented
Project: Date » Commerce Core

A test install I've done here at home makes it look like it isn't Commerce specific. Although I do see slightly different behavior (the form doesn't rebuild with an error anymore, but only because the validation callback fails to call the form_error() correctly, and only when new content is created), which might be related to using a fresher dev release of Date.

There's no particular reason I don't use #access beyond not knowing it was there, or was about hiding form elements, specifically. All Google searches do is return recommendations to use unset(), but I learned a long time ago that you only unset() a field widget if you want to fill your error log with warnings. Without experimenting I'm not even sure if #access will let me do what I want to do.

The use case is hiding the $form element that represents the Field's widget, but still wanting to occasionally change the field's value when the form is submitted by the user, by inserting a value into the $form_state['values'].

Form arrays are render arrays, and all Core fields behave just fine when it's used (preexisting values are kept when the form is submitted, form elements are not displayed, and I can set $form_state['values'] to change field values). If there's a specification somewhere that recommends one approach over the other, I'd be happy to adjust my behavior (but I'd still call this buggy behavior).

I'm also updating the issue text to make it clearer.

amateescu’s picture

amateescu CreditAttribution: amateescu commented
Project: Commerce Core » Date
Version: 7.x-1.x-dev » 7.x-2.x-dev
Component: Order » Date Field
Status: Postponed (maintainer needs more info) » Fixed

Ah, your reply reminded me that #access won't work on fields. This is probably what you're looking for:

http://api.drupal.org/api/drupal/modules!field!field.api.php/function/ho...

wwedding’s picture

wwedding CreditAttribution: wwedding commented

Nope. What I'm looking for is Date module to behave in an expected fashion consistent with Core field behavior, which is to not break when a field widget is hidden with hide().

This isn't a support request. it's a bug report for the Dates module that got chucked into Drupal Commerce for no particular reason by the maintainer of that module.

KarenS’s picture

KarenS CreditAttribution: KarenS commented

It got chuncked into Drupal Commerce because core does not use hide() on forms anywhere that I know of and I don't have any idea what Drupal Commerce is doing. I can't fix what I don't understand and I don't understand why that is being done, nor have I ever seen any example of how or where it was implemented.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

mkadin’s picture

mkadin CreditAttribution: mkadin commented
Status: Closed (fixed) » Postponed (maintainer needs more info)

If its alright, I've set this to postponed, needs more info. I too have run into this problem. I've posted an issue on the core queue about whether or not hide() should work for form API render arrays. hide() is generally used in the theming layer, but that is not specified in its documentation. I also tested hide() against the checkbox, textfield, and date core form elements and it worked flawlessly, so I think that contrib modules should support its use.

http://drupal.org/node/1452200 is the new issue in the core queue. When it is resolved I'll come back and post the results here.

mkadin’s picture

mkadin CreditAttribution: mkadin commented
Status: Postponed (maintainer needs more info) » Active

According to @jhogdgon, core function hide() should work for all render arrays, including form elements. I think we should open this issue back up and see if it can't be solved. I'll look into what the internals of hide() actually does and see if I can't provide a patch.

Closed core issue that discusses hide() and form arrays: http://drupal.org/node/1452200

mkadin’s picture

mkadin CreditAttribution: mkadin commented
Issue summary: View changes

Making issue text a little clearer, with specific error message.

wwedding’s picture

wwedding CreditAttribution: wwedding commented
Title: Using hide() on the Date (Unix Timesamp) field form element causes empty $form_state values » Using hide() on Date field form elements causes validation errors
Category: support » bug
FileSize
date-hide-validation-test-1419286-13.patch5.6 KB

Updated issue description.

I've been trying to make progress on this one so I can finally check this off my list but it has turned into a quagmire really quickly. Quick fixes that work for some widgets and only require touching a single file don't work for other widgets.

Attached is a patch that will add a new test to illustrate the problem since KarenS couldn't reproduce when I reported this a year ago. It's a bit rough and not intended to be committed yet (it also includes a tweak to DateFieldBasic class, which is a separate issue).

wwedding’s picture

wwedding CreditAttribution: wwedding commented
Issue summary: View changes

Fixing up the issue description to be more accurate before changing the title.

Shevchuk’s picture

Shevchuk CreditAttribution: Shevchuk commented
Issue summary: View changes

Experiencing this on node edit with hidden Date type field (auto filled in presave hook). Unix timestamp type fields and empty fields seem to be not affected.

CSS hide workaround is not secure, so a proper fix is needed.

wwedding’s picture

wwedding CreditAttribution: wwedding commented

In the case where security is important, the best option is to use #access properties in a form render array. A form element that is hidden by the use of hide() can still be used successfully by a user if they know it is there and know how to spoof form input.

jweirather’s picture

jweirather CreditAttribution: jweirather commented

This is happening to me too, as of Drupal 7.43, using Date 7.x-2.9. Using the #access method worked well for me. For others' reference, in a custom module, I added:


function MY_MODULE_form_alter(&$form, &$form_state, $form_id) { 
  if($form_id == 'CONTENT_TYPE_node_form') {
    $form['field_MY_FIELD']['#access'] = false;
  }
}

(the CAPS indicate changes you need to make for your own environment)

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Active » Needs review

Thanks for the patch, we'll try to review it soon.

BTW when you upload a patch it helps to set the status to "needs review", that triggers the testbots and lets others know that there's something review.

Status: Needs review » Needs work

The last submitted patch, 13: date-hide-validation-test-1419286-13.patch, failed testing. View results