Remove reference magic from form API

This is nothing new. module_implements has the explanation.

I have grepped for \$form_values.*= and found quite a number:

comment.module:1408:    $form_values['date'] = 'now';
comment.module:1410:  $form_values['timestamp'] = strtotime($form_values['date']);
comment.module:1413:    $form_values['uid'] = $account->uid;
comment.module:1414:    $form_values['name'] = $form_values['author'];
comment.module:1424:    $form_values['subject'] = truncate_utf8(decode_entities(strip_tags(check_markup($form_values['comment'], $form_values['format']))), 29, TRUE);
comment.module:1430:  $form_values = _comment_form_submit($form_values);
contact.module:227:  $form_values['recipients'] = implode(',', $recipients);
node.module:2174:    $form_values['type'] = array_filter($form_values['type']);
node.module:2196:    $form_values['processed_keys']['#ref'] = trim($keys);
taxonomy.module:286:  $form_values['nodes'] = array_filter($form_values['nodes']);
user.module:260:  $form_values['picture'] = $user->picture;
user.module:282:      $form_values['picture'] = $file->filepath;

but comment.module is OK, it's a submit and it _comment_form_submit has a return $form_values , taxonomy.module is again a submit and it's used immediately, and user_validate_picture has global $form_values which is not elegant but works.

So if Zen confirms that this indeed makes $form_values immutable then we are good to go.

and node.module is also OK as Zen has just patched those two lines in http://drupal.org/node/56911

*SIGH*

Commented.

Shorter version.

FYI: this will break the project module, which attempts to modify stuff in $form_values during validate in quite a few places. so, if we're going to go this route, we'll need to fix project asap...

FYI: i just submitted #56959 as an issue against the project module to fix this (since, according to gatsby in IRC, project should never have been doing that in the first place, regardless of this fix)...

applied

Someone please enlighten me what's going on! This is way over Goba's head, too.

What I have found is that removing the $form = from $form = form_builder($form_id, $form) then it works. If I assign that to ANYTHING then validate breaks -- I tried using $form_copy = form_builder($form_id, $form) and $form_copy downwards. So, I have changed form_builder to use &$form instead.

Here is the problem in an absolutely condensed form:

function t1($x) {
global $a;
$a = array('a' => &$x);
return $a;
}
$x = t1('x');
var_dump($a);
t1('x');
var_dump($a);

output is:

array(1) {
  ["a"]=>
  &string(1) "x"
}
array(1) {
  ["a"]=>
  string(1) "x"
}

tested the latest patch on our multipage example module, and it doesn't seem to break anything.

I already mailed php internals and I hope will understand what's up. But this patch is good to go.

For the record: http://marc.theaimsgroup.com/?l=php-dev&m=114400300015496&w=2

Looks like this was comitted to HEAD.

Ah, missed the second half of the updates.

Comitted to HEAD.

I am told that using references here can lead to a php segfault so I am better to turn back to the first version with #ref as an exception.

Ops, this is not good, #ref is in $form.. let me think.

OK, I give up. They are modifiable and that won't be fixed. One should know where are his limits. This reverts all involved parties to their RC1 state.

It seems impossible to make $form_values changeable via #ref and not modifiable via itself.

Why is it impossible? If I set $form to contain references to $form_values then all values in the latter will become references and therefore they will be changeable, too.

The patch that hit core broke views.module's ui as well. This revert fixes it. +one kajillion.

ok, applied

Let this be closed, buried six feet under and forgot...

Does this mean that any modules that were changed to accomodate the original change (where $form = form_builder() became just form_builder()) will not work with this patch unless they too are reverted back?

I could not let this rest. This was eating me day and night. I am not sure this is 4.7 -- but it has the advantage to remove the $ref black magic from form_builder which was really a) elegant b) non-drupalish because it was black magic.

Minor correction,

hm, some old patch got into this.

abusing the new 'replies to issues show up in my issues' feature so that I can remember to test this tomorrow. ;)

form_set_value() takes the place of #ref.

Killes tells me he is not again some last minute cleanup. Tested password editing and advanced search both.

Tested password/search as well, and just clicked around generally to a bunch of different forms to see what happened. Couldn't find any errors, so marking this RTBC.

Note that there are three contrib modules that will be broken by this change:

backport:
D:\Development\Drupal\contributions HEAD\modules\backport\form.inc (308) $form['#ref'] = &$ref;
D:\Development\Drupal\contributions HEAD\modules\backport\form.inc (646) $form['pass1']['#ref'] = NULL;
D:\Development\Drupal\contributions HEAD\modules\backport\form.inc (647) $form['pass2']['#ref'] = NULL;
D:\Development\Drupal\contributions HEAD\modules\backport\form.inc (653) $form['#ref'] = $pass1;

(looks like it's a copy/paste job from the old pass code in HEAD)

HTMLTidy:
D:\Development\Drupal\contributions HEAD\modules\htmltidy\forms_api_backport.module (394) $form['#ref'] = &$ref;
D:\Development\Drupal\contributions HEAD\modules\htmltidy\forms_api_backport.module (716) $form['pass1']['#ref'] = NULL;
D:\Development\Drupal\contributions HEAD\modules\htmltidy\forms_api_backport.module (717) $form['pass2']['#ref'] = NULL;
D:\Development\Drupal\contributions HEAD\modules\htmltidy\forms_api_backport.module (723) $form['#ref'] = $pass1;

(looks like it's just taking the code from backport)

OG:
D:\Development\Drupal\contributions HEAD\modules\og\og.module (439) $form['valid_emails']['#ref'] = $valid_emails;

(my voodoo magic patch, but should be remediable by form_set_value())

I repent my sins -- step by step we strayed further and further from the one and true Drupal Way with every piece of reference magic we added. So please give me redemption by committing this.

Renamed function, Steven polished the Doxygen.

Committed to HEAD. Good job.

Give it a minute :P.