There is some logic in place to avoid running the validation on a date popup more than once because validating the widget also changes its data. The logic does not work because it is looking for the data in the wrong place.

Details: At the end of date_popup a call to form_set_value changes the data in to a string inside the form state. From the comments of form_set_value:

It does not change the value in $element['#value'], only in $form_state['values'], which is where submitted values are always stored.

At the beginning of date_popup_validate this string is checked for in $element["#value"]. Since it wasn't set there, the rest of the validation will run and it will produce an incorrect form error because it can not handle the modified data.

I will post a patch for it.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jmuzz’s picture

jmuzz’s picture

This is part of a solution for #1289062: Required fields in collection not validated when empty. I tried to add it in the related issues field but it was ignored when I submitted.

vijaycs85’s picture

@jmuzz, Thanks for reporting issue and your patch. Setting this to review to testbot pick up this patch. However it would be great if:

1. Formal steps in issue summary to reproduce/manual test patch.
2. Test only that proves current implementation is wrong and this patch fixes it.

jmuzz’s picture

Status: Active » Needs review

Oh yeah I meant to set it to needs review.

I don't know a way to test it other than by using the patch I posted in that field collections issue. In what circumstances would a field 'normally' need to be validated twice during one submission?

dafeder’s picture

#1 worked for me. I was getting the "second validation" error when I submitted a custom form with a "date_popup" element. Node forms were working fine.

cafuego’s picture

This also fixes a double validation problem with the node_expire module. I'm tempted to apply the patch as-is.

oadaeh’s picture

What I'm seeing is this: when submitting a multistep form with a programmatically created date field, the field's values are validated twice. When a date field successfully passes validation, the validated date is saved back to the $form_state['values'] array. The first time through, everything is okay, except that the date goes from being an array to a string. The second time through, the date string no longer represents a "valid" value and fails validation.

Probably the reason the problem doesn't manifest itself so much is that a Field API created date field (or one created in the Drupal UI) is a multi-nested array, where as a programmatically created date field is only(usually?) a single-nested array. The multi-nested arrays pass through the dual validation check fine, because they are still arrays the second time through.

While it may work to fix this problem, I believe the previously proposed patch reverts a fix for other functionality.

I've attached a patch that works for me and basically resaves the new string date as an array to $form_state['values'].

Status: Needs review » Needs work

The last submitted patch, 7: date-validation-error-2199587-7.patch, failed testing.

Status: Needs work » Needs review
Kristen Pol’s picture

Patch #7 fixes the issue for me but it isn't passing tests so I've resubmitted it to try one more time.

Status: Needs review » Needs work

The last submitted patch, 7: date-validation-error-2199587-7.patch, failed testing.

oadaeh’s picture

Okay, so more testing in a live situation proved that my fixed caused some unwanted side effects.

@jmuzz's recommended fix does work, but I still think removing the other code reverts a fix for another bug (that I was unable to locate, but is mentioned in the code comments), and it's removal is unnecessary for this issue.

I've attached a patch that adds his fix (w/o the code removal) and a coment explaining why it necessary.

Locally, there are test failures with the Date Migration and Timezone & Granularity tests, but they also fail without the attached patch.

oadaeh’s picture

Status: Needs work » Needs review

Setting the status.

Kristen Pol’s picture

Status: Needs review » Reviewed & tested by the community

This fixed the problems I was seeing with other date fields as well as the original multi-step form issue. Thanks!

cafuego’s picture

Ok, I'll apply this patch to date on an unrelated project that was fixed by #1 and see if behat is happy with it. If so, it's going in :-)

cafuego’s picture

Status: Reviewed & tested by the community » Fixed

Aaaand applied. Nice work!

  • cafuego committed ede6c62 on 7.x-2.x authored by oadaeh
    Issue #2199587 by oadaeh, jmuzz: Second validation of date popup causes...

Status: Fixed » Closed (fixed)

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