Download & Extend

Don't save junk as variables during settings save

Project:Drupal core
Version:5.x-dev
Component:system.module
Category:bug report
Priority:normal
Assigned:moshe weitzman
Status:closed (fixed)

Issue Summary

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.

AttachmentSizeStatusTest resultOperations
no_crap_in_variables_table.patch2.18 KBIgnored: Check issue status.NoneNone

Comments

#1

same patch, without unrelated upload.module fix.

AttachmentSizeStatusTest resultOperations
no_crap_in_variables_table_0.patch1.27 KBIgnored: Check issue status.NoneNone

#2

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)?

#3

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

#4

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.

#5

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.

#6

+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?)

AttachmentSizeStatusTest resultOperations
no_crap_in_variables_table_1.patch1.28 KBIgnored: Check issue status.NoneNone

#7

Version:4.7.0-beta1» x.y.z
Status:needs work» needs review

#8

Status:needs review» reviewed & tested by the community

#9

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

#10

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

#11

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

#12

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.

#13

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

#14

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

#15

rerolled to fix a minor offset

AttachmentSizeStatusTest resultOperations
no_crap_in_variables_table_2.patch1.38 KBIgnored: Check issue status.NoneNone

#16

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

#17

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

#18

Version:x.y.z» 5.x-dev
Status:needs work» closed (fixed)

This is fixed in Drupal 5. Closing.

nobody click here