I am opening this issue in order to be able to submit a patch that fixes the following issues. I'm sorry for submitting a single patch for all those issues, but it turns out that just a few changes allowed all those improvements.
1) Do not overwrite $_REQUEST variable
This simply can cause problems to other modules using it, therefore it is better to introduce a new variable rather than overwriting it.
#1496310: Do not overwrite $_REQUEST variable
2) Allow default values set through the URL to actually be modified by users during form editing.
Before this patch users were able to edit the default values, but those values were not saved, which was quite a serious issue.
#1473856: Overzealous prepopulation
#1309934: After prepoulating fields (new) content can not be saved if it was edited before
3) Allow to prepopulate 'hidden' and 'value' field types.
I see no reasons to prevent setting default values those field types.
#1275274: Allow use with #type = 'hidden'
3) Add administration options (and a related permission) to lock default values and overwrite the ones already present for the field.
#926094: Able to lock the field after populating?
#672400: Working around field defaults?
4) Remove prepopulate.install and replace it with hook_module_implements_alter implementation in prepopulate.module.
I am not sure this is really needed, but it is surely better implemented as a hook.
5) Add usage information in the module UI help page.
Just to make the usage more clear to new users.
Comments
Comment #1
Norberto Ostallo CreditAttribution: Norberto Ostallo commentedHere it is the patch.
Comment #2
jbrauer CreditAttribution: jbrauer commentedThanks for these suggestions.
I think this needs to be broken up into smaller issues. In part some of this looks like a good start and could be included sooner than later. Others like the hidden field values are more significant and will have several other dependencies. Marking as needs work but for most items there is a separate issue where a specific patch would be appreciated.
Comment #3
Norberto Ostallo CreditAttribution: Norberto Ostallo commentedI'm glad to hear you find this interesting and I will try to break this up into smaller issues, providing a patch for each of them.
In this way it should be easier to check and validate each of them.
Comment #4
Norberto Ostallo CreditAttribution: Norberto Ostallo commentedI have reduced the scope of this patch, opening other issues for minor tasks.
The following are the major changes to the module.
Comment #5
Norberto Ostallo CreditAttribution: Norberto Ostallo commentedI'm sorry, patch in #4 was broken because it did not include a new file.
This patch should work and I have also added a first way to allow admins to restrict prepopulation on selected forms.
Comment #6
jbrauer CreditAttribution: jbrauer commented1. cannot be done because it does not adequately address security concerns. Other modules could act after Prepopulate and change access to the field so the only place this can be done securely is in #after_build. Any further work on that should be in #1309934: After prepoulating fields (new) content can not be saved if it was edited before.
2. The #default_value does not work in all cases which lead to having to use both #default_value and #value which is not as much an issue because item 1 precludes editing fields that have been pre-populated.
The only way I've been able to see around these issues which are in #1309934: After prepoulating fields (new) content can not be saved if it was edited before would be to implement an administration system where site administrators can choose what forms and ideally can be prepopulated. Absent this level of administrative control site would be open to CSRF.
These issues have already been fixed once and can't be casually made insecure again. See http://drupal.org/node/1182968 for related information.
Comment #7
thekevinday CreditAttribution: thekevinday commentedI was thinking entityreference_prepopulate has a better approach all around.
In particular, entityreference_prepopulate makes the arguments cleaner and easier to figure out.
The downside I think would be that each field type would have to have support written for it, so custom modules would each need their own prepopulate handlers.
All of 1->5 are solved by the strategy used by entityreference_prepopulate.
The only problem is its explicitly for entityreference.
Perhaps refactoring and using a more generalized version of it would be a better start than the current 7.x version of prepopulate, which has not seen any changes in a year (and never really was written/released for D7).
Edit:
see also: #1612548: Prepopulate integration
Comment #7.0
Robin Millette CreditAttribution: Robin Millette commentedUpdated issue summary. Linked to the "Do not overwrite $_REQUEST variable" issue.
Comment #8
jbrauer CreditAttribution: jbrauer commentedThese issues have evolved in other directions leaving this to be outdated.
Comment #9
manoloka CreditAttribution: manoloka commentedjbrauer ... for what you say in #6 patch in 5# should not be used as it would make the module unsafe, is that right?
Thanks
Comment #10
manoloka CreditAttribution: manoloka commentedNorberto Ostallo if 1. and 2. from patch in #5 are not safe (as says jbrauer in 6#).
Would it be possible for you to make a patch to implement the admin and lock function?
That'd be a great addition to the module
Thanks for your work
Manolo