How many ways are there to cache a form? [#583730]

"Follow-up" on #579366: How many ways are there to redirect after form submission? ;)

$form['#cache']

$element['#cache']

$form['#no_cache']

$form_state['cache']

Additionally, #cache now clashes with the new drupal_render() cache.

Note:

This patch will probably not work, because we still don't have a $form_state['build_info']. $form['#cache'] is cached with the form and also retrieved from cache, so drupal_rebuild_form() is able to check the value again. But as of now, only $form_state['storage'] is cached from the $form_state, so the info in $form_state['cache'] is lost on subsequent rebuilds from cache. See #367567: Use AJAX framework for "Add more" / load include file containing form / introduce $form_state['build_info'] for a patch to commit ;)

Comment	File	Size	Author
#8	drupal.form-cache.8.patch	10.7 KB	sun
#8
#5	drupal.form-cache.5.patch	11.31 KB	sun
#5
	drupal.form-cache.patch	11.33 KB	sun

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

sun

German

Karlsruhe

CreditAttribution: sun commented 24 September 2009 at 16:53

From #367567-106: Use AJAX framework for "Add more" / load include file containing form / introduce $form_state['build_info']: The following are missing in above list:

- $form_state['rebuild']

- $form_state['storage']

Those also trigger caching of a form.

I need help/input here. I have absolutely no idea how all of these are supposed to be used or not used (except $form_state['no_cache'], which follows the same logic as 'no_redirect').

Comment #2

Frando CreditAttribution: Frando commented 25 September 2009 at 02:47

Oh, great to see this happening. I wanted to do this since I finished #483778: The great form.inc cleanup tour, part 1 but didn't come around to it. I totally agree on killing $form['#cache'], $form['#no_cache'] and $element['#cache'] and using $form_state['cache|no_cache'] everywhere.

I think the logic is basically simple with form and element properties removed, $form_state['no_cache'] has precedence, and if that is not set to FALSE we cache if either $form_state['cache'] or $form_state['storage'] is !empty().

I'm not really sure OTOH why $form_state['rebuild'] always triggers caching the form if we're checking for $form_state['storage'] already. Maybe it was the assumption that $form_state['rebuild'] means multi-step form means caching is required.

Comment #3

mattyoung CreditAttribution: mattyoung commented 25 September 2009 at 06:24

Comment #4

2 October 2009 at 21:20

Status:

Needs review

» Needs work

The last submitted patch failed testing.

Comment #5

sun

German

Karlsruhe

CreditAttribution: sun commented 6 October 2009 at 22:24

Status:

Needs work

» Needs review

File	Size
drupal.form-cache.5.patch	11.31 KB

It seems we all don't really know.

Hence, I recommend moving forward with the provided clean-up. Anything else will have to wait for D8.

Same patch, testbot seemed to have a problem.

Comment #6

14 October 2009 at 01:20

Status:

Needs review

» Needs work

The last submitted patch failed testing.

Comment #7

fago

German

Vienna

CreditAttribution: fago commented 17 November 2009 at 09:33

Yep, indeed this is really confusing. +1 for using $form_state['cache'] only.

Well for rebuild and storage caching makes sense, so it's turned on automatically. But it's also turned on automatically once #ajax is used. So people build there forms without #cache and it's working fine, but afterwards if some form_alters() it and #ajax gets enabled suddenly the form breaks - as caching got enabled and some includes are missing. So turning on cache on any form would help here avoiding unexpected behaviour.
But then for file including it would make sense to have the possibility to specify more than one include file in $form_state as sometimes you'll need more e.g. when you want to reuse submit/validate handlers sitting in another foreign include. Manually including those is possible with some tricks, but that's quite hard to do and hacky.

Comment #8

sun

German

Karlsruhe

CreditAttribution: sun commented 17 November 2009 at 12:46

Priority:	Normal	» Critical
Status:	Needs work	» Needs review

File	Size
drupal.form-cache.8.patch	10.7 KB

Since #cache now clashes with the drupal_render() #cache property, I'm bumping this to critical.

Since we remove the auto-caching and auto-rebuilding in #634440: Remove auto-rebuilding magic for $form_state['storage'], I think this patch is ready to fly.

Comment #9

sun

German

Karlsruhe

CreditAttribution: sun commented 17 November 2009 at 15:24

Assigned:	Unassigned	» sun
Issue tags:		+D7 API clean-up

Tests passed.

Comment #10

effulgentsia CreditAttribution: effulgentsia commented 18 November 2009 at 01:15

subscribing

Comment #11

moshe weitzman CreditAttribution: moshe weitzman commented 18 November 2009 at 14:32

Status:

Needs review

» Reviewed & tested by the community

Nice cleanup and bug fix. Bots happy.

Comment #12

effulgentsia CreditAttribution: effulgentsia commented 18 November 2009 at 17:43

+1 from me as well. We have a bunch of critical bugs related to unexpected behaviors in FAPI: #635552: [meta issue] Major Form API/Field API problems, mostly surfacing now due to the more advanced capabilities of the AJAX framework and FieldAPI. Things like this that remove those weirdnesses either directly help with those other issues, or indirectly help by allowing developers to make sense of what's going on in order to come up with non-hacky solutions. And as per #8, this issue is critical in and of itself.