Download & Extend

#access of date_combo fields prevents validation

Project:Date
Version:7.x-2.x-dev
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed (fixed)

Issue Summary

The symptom is that when a date_combo element has #access set to FALSE, the form never validates because "The To date must be greater than the From date."

I think that Drupal core could do a better job here, however I believe that there is an issue with the way date handles validation using #process and #post. The Drupal core problem is that when a FAPI element has #access set to FALSE, FAPI still calls all of the #process hooks. I chatted with chx about it, and his response was "why not". But the answer to "why not" is that form element implementers (such as date) rely on the process callbacks to setup lots of work, and if the element isn't accessibly then why are they doing that work?

Since we're not going to fix this in Drupal core today (and 5.x ever) there's still a problem with the date module. I suggest that in date_combo_process you just check for #access and return if it's 0. Patch is attached.

AttachmentSizeStatusTest resultOperations
access.patch464 bytesIgnored: Check issue status.NoneNone

Comments

#1

Here's an updated patch the doesn't catch a form element without #access at all.

AttachmentSizeStatusTest resultOperations
408770.patch494 bytesIgnored: Check issue status.NoneNone

#2

Hmm... true, this is an issue. This is probably needed in D6, too, I imagine.

#3

Status:needs review» fixed

OK, fixed. I'm trying think if there are other places this will be needed, but I can't come up with any right now. Thanks!

#4

Status:fixed» closed (fixed)

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

#5

Status:closed (fixed)» active

I'm not getting good results with this patch. When I set the #access attribute to TRUE, the value of the field is erased upon submission and I get the following notices:

# notice: Undefined index: #value in /home/kratib/www/d6/includes/form.inc on line 1052.
# notice: Undefined index: #parents in /home/kratib/www/d6/includes/form.inc on line 1312.
# warning: array_shift() [function.array-shift]: The argument should be an array in /home/kratib/www/d6/includes/form.inc on line 1323.
# notice: Undefined offset: 0 in /home/kratib/www/d6/sites/all/modules/cck/content.module on line 1025.
# notice: Undefined offset: 0 in /home/kratib/www/d6/sites/all/modules/cck/content.module on line 1025.

However, if I move the code snippet to date_combo_validate() instead of date_combo_process(), these notices don't appear and the value is not lost. What I get instead is the behaviour of #403404: Default value set to 1000 years back when #access is FALSE.

#6

Version:5.x-2.5» 6.x-2.2

Also applies to latest D6 release. In my test scenario, moving the #access check to date_combo_validate resolves the problem and I don't get the 1000 years problem either.

#7

I have the same issue as kratib. If I set #access = FALSE, a previously existing value should be retained. Instead, it's clearing it. A scenario would be a user with lesser permissions editing a node that was created with someone with access to that particular field. In my case, I'm doing multistep forms in which the #access is set to FALSE for steps that do not contain that particular field, in which case the value is being dropped after submitting a different step. See #503158: array_shift() warning when using Date fields.

What would the implications be of returning $element instead of just returning nothing? The idea is to return $element without processing it.

<?php
if (isset($element['#access']) && empty($element['#access'])) {
  return
$element;
}
?>

This is what I'm doing to solve the issue with the Multistep module. The problem in #403404: Default value set to 1000 years back when #access is FALSE was also fixed for me, although I'm not sure if this is the correct approach.

#8

Status:active» needs review

Here's the patch to make date_combo_process() return an unprocessed $element when #access is FALSE.

AttachmentSizeStatusTest resultOperations
date_combo_process.patch503 bytesIgnored: Check issue status.NoneNone

#9

Version:6.x-2.2» 6.x-2.x-dev

The last patch did not apply successfully for the latest code. I'm uploading a new patch that works with the current development snapshot.

AttachmentSizeStatusTest resultOperations
date-408770.patch623 bytesIgnored: Check issue status.NoneNone

#10

Thanks vkareh - #9 is just what I needed :) Works perfectly.

#11

Status:needs review» fixed

Yep, I think this is the right solution. Thanks!

#12

Status:fixed» closed (fixed)

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

#13

Version:6.x-2.x-dev» 7.x-2.x-dev
Status:closed (fixed)» patch (to be ported)

this oneliner is not yet in 7.x

#14

sub

#15

Status:patch (to be ported)» fixed

Added to 7.x. Thanks for pointing this out.

#16

Status:fixed» closed (fixed)

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

#17

Could this patch be causing this problem?
#1267434: Dates fields set to #access=FALSE are emptied on save

I've removed this code as it leads to #default_value not being set, thus the values are never getting set in $form_state['values'], which is what's used to update the node.

UPDATE #1

I believe this is the case when using Date Popup field formatter. Still looking into it. I believe it's because of this line of code:

-  $input = drupal_array_get_nested_value($form_state['input'], $element['#parents'], $input_exists);
+  $input = drupal_array_get_nested_value($form_state['values'], $element['#parents'], $input_exists);

Getting some other warnings on submission now related to timezone & value2 not being set. I believe this is because these fields are added in the validate function unfortunately which is not called I assume when #access = FALSE.

#18

That bit of code has nothing to do with the patch in this issue. And this is a closed issue.

nobody click here