FormAPI: $_POST['edit'] is harmful.

This is terribly broken right now, but in progress. ;)

Slightly updated, still needs work, but things aren't obviously broken. ;)

This actually eliminates the $_POST['edit'] construct entirely in favor of $_POST.

And here's a better version, with form.inc actually included in the patch.

This version of the patch changes all references to $_POST['edit'] and ['#post']['edit'] to $_POST and ['#post'], respectively. Why? A lot of investigating and a conversaion with UnConeD revealed that the only reason $_POST['edit'] exists is the legacy requirement that incoming form edit values be castable straight into a valid node object, to be passed into node_save().

The use of $_POST['edit'] causes major problems integrating submit buttons into the legitimate form values collection, among other things. By rolling everything back into $_POST, we gain the ability to work with the full range of form data (including the clicked button) in our submit and validate handlers.

Reviews appreciated.

Changing the title to reflect the direness of $_POST['edit'].

New patch that fixes the issues with checkboxes (and, in fact, any form elements deeply nested). #name was being built incorrectly. Two lines of array-wiggling in form_builder correct this problem.

mfredrickson, you bring up an excellent point. previous, all buttons were hard-coded with the name 'op'. It didn't matter much, because those values weren't really available to most form processing code. Now, we not only get op, we can name separate sets of buttons differently to accomplish different things and switch based on which one was pressed. It's valuable, but I think we're a bit late in the development cycle to try to add complex function-firing based on it at this point. It's definitely a direction for future improvement, though.

I'll reiterate quickly why this patch is valuable and important.

1. It allows us to take our hands off of $_POST, and deal exclusively with $form_values, for our logic switching.
2. It makes a broader range of behaviours possible for multipart forms (where $form_values can be preserved, but $_POST is transient)
3. It eliminates unecessary legacy cruft inherited from years old Drupal code
4. It makes $_POST more self-explanitory, with less room for strange 'hidden' properties that exist outside the edit array

Re-rolled against the current HEAD.

And, ironically, $_POST['op'] wasn't making it into $form_values['op']. Fixed.

Good point. Here's the rationale for the chat, minus hype. :)

1. With the addition of multi-step form handling, and the possible addition of drupal_execute() for programmatically submitting forms, we're starting to rely more and more on the contents of a $form_values array. However, some form submission handlers (and even some recursive building functions) rely on the button operation. To switch to a confirm form, to execute different logic, etc.
2. Because we currently keep the contents of our form in $_POST['edit'] while the buttons reside in $_POST, $op is never available in $form_values. That means that forms with different button actions can never be part of Drupal's multistep form processes. In addition, it means that many locations peek directly at $_POST when they shouldn't really have to. That's brittle.
3. The reason that we keep some values in $_POST['edit'], but buttons in $_POST, is purely historical. According to UnConeD, it was necessary because the old pre-4.7 node form turned the $_POST['edit'] values into a node object directly, then validated the node object. Adding the button values would've polluted it. We no longer use this method, however -- everything goes through Form API validation and submit functions.
4. This first step to eliminating unecessary uses of $_POST is moving ALL form values into $_POST proper, and eliminating the unecessary ['edit']. This patch does that.
6. The second step is eliminating references to $_POST['op'] wherever possible. For example, the patch changes node.module's node_form function:
   

   -  $op = isset($_POST['op']) ? $_POST['op'] : '';
   -  if ($op == t('Preview')) {
   +  $op = isset($form_values['op']) ? $form_values['op'] : '';
   +  if ($op == $form_values['preview']) {
   

In some cases, $_POST is examined when forms are redirecting to each other, or when our API would not be handing off the $form_values at all. These cases still remain in the code, as they need more individual attention and (potentially) more complex logic conversion. This patch, though, lays the groundwork for them and makes almost all 'normal' cases of $_POST peeking unecessary.

7. As others have noted, it points out an unrelated but potentially cool direction for future expansion: the ability to have different buttons trigger different formAPI hooks. Right now, we have one flag: #executes_submit_callback. We use p[ switching to determine the button that was pressed. Instead, individual buttons could have a single #callback property, which FormAPI would use to determine what function to call when processing. I haven't tried to add that to this patch; the freeze is close enough I'd rather get the essential change in before tinkering with 'cool stuff.' If you think it's worth pursuing, Dries, I can give it a shot -- it wouldn't be too difficult to try making *_submit an overridable default #callback for buttons.

I like ordered lists.

Re-rolled against HEAD

Dries, I'm going to let this patch sit as it is at RTBC, and pursue an add-on this evening that implements the button mechanism discussed above.

Just tested on a fresh HEAD; uploads are in fact working again and all is well in JavaScript land. Changing the handling of the files array to prevent attempts at nesting is also a good idea; such things can only end in tears.