We need to exclude certain variables from being saved when submitting a settings form. For example, without this patch submit the admin/settings/content-types/story page while your devel.module query log is enabled. you will queries like

DELETE FROM variable WHERE name = 'submit'
0.561INSERT INTO variable (name, value) VALUES ('submit', 's:18:\"Save configuration\";')

This patch avoids these meaningless variables.

Comments

moshe weitzman’s picture

StatusFileSize
new1.27 KB

same patch, without unrelated upload.module fix.

chx’s picture

Status: Needs review » Needs work

'type' is used only by node settings and there is the problem: other modules may want to exclude other stuff. what about adding a $form['#exclude'] = array(...) to various hook_settings implementations, merging it with the default ones (reset, submit, form_id, array_filter)?

dries’s picture

Isn't the forms API supposed to know exactly what should be saved, and what not?

chx’s picture

How could I know which hidden values are to be saved and which are not?

And also, here we have another problem, too: _submit does not get the form only form values. Can be fixed easily though...

This may even lead to somewhere, we add $form as thrid parameter to submit , and save only those that do not have #save => FALSE. Might even set this as a default on submit type.

moshe weitzman’s picture

my patch prevents most of the crap. preventing all of the crap requires each hook_settings() to implement something and i'm not inclined to write that now. chx - if you think this patch needs work, i hope you or one of friends will resubmit. otherwise, please set to ready to commit.

Bèr Kessels’s picture

StatusFileSize
new1.28 KB

+1 from me.

I was not aware of the bug, but when I just looked at my variables table, i indeed encuotered al lot of these ugly variables. The patch seems to fix this.

This is a rerolled patch (ready to commit?)

moshe weitzman’s picture

Version: 4.7.0-beta1 » x.y.z
Status: Needs work » Needs review
moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community
dries’s picture

I wonder why the forms api uses that many hidden variables.

chx’s picture

So many? Hidden? form API only adds form_id and nothing else. 'submit', and 'reset' are buttons which the form always sets. 'type' and 'array_filter' is used with node options. 'array_filter' is a kludge because we did not want to rewrite node options (I did rewrite it once but then run away in horror -- see the checkboxes thread). 'type' I would

chx’s picture

delete 'type' I would from my comment's end.

Bèr Kessels’s picture

So, after we ran away in horror, what should happen to this simple patch?
It does fix the issues addressed, without running away in horror.

moshe weitzman’s picture

lets commit this one. it performs exactly as the title describes. and thats good.

moshe weitzman’s picture

any reason why this one has stalled? if so, please speak up so we can fix and resubmit.

Bèr Kessels’s picture

StatusFileSize
new1.38 KB

rerolled to fix a minor offset

dries’s picture

Is this patch still needed? The forms API has been modified to filter out some common variables.

markus_petrux’s picture

Status: Reviewed & tested by the community » Needs work

Yes, I believe something is needed. Actually, before finding this issue I opened another one, because in fact, there's already code in the forms api to filter some of that 'junk', but it is incomplete. Here's the issue:
http://drupal.org/node/48209

Jaza’s picture

Version: x.y.z » 5.x-dev
Status: Needs work » Closed (fixed)

This is fixed in Drupal 5. Closing.