Overlay does a bunch of unnecessary form redirect handling in JavaScript

Seems like drupal.org is eating attached files again...

No, that fallback code in overlay-parent.js (I think you are talking about the code in Drupal.overlay.loadChild?) is not what does it.

With this patch applied, the code that actually causes it to break out of the overlay is in overlay_init():

      // If this page shouldn't be rendered here, redirect to the parent.
      if (!path_is_admin($current_path)) {
        overlay_close_dialog($current_path);
      }

I think you are right that this patch leads to an extra request on form submission, although I'm not sure "extra" is really the right word since it's what the form API does normally... overlay module was just adding a bunch of code to bypass that :)

I looked a little more closely and did some logging, and if what I am seeing is correct, the icing on the cake here is that overlay_form_submit() is ALREADY NOT BEING CALLED :) At least on the node form, which is the main place I've ever really tested this from, and the main place where it needs to work.

The form API is always hard to follow, but it has this code:

    // If the triggering element specifies "button-level" validation and submit
    // handlers to run instead of the default form-level ones, then add those to
    // the form state.
    foreach (array('validate', 'submit') as $type) {
      if (isset($form_state['triggering_element']['#' . $type])) {
        $form_state[$type . '_handlers'] = $form_state['triggering_element']['#' . $type];
      }
    }

These button-specific handlers are then the only ones that get used to process the form, in form_execute_handlers():

  // If there was a button pressed, use its handlers.
  if (isset($form_state[$type . '_handlers'])) {
    $handlers = $form_state[$type . '_handlers'];
  }

The node form submit button has a button-level submit handler, so I think what happens is that this gets copied into $form_state['submit_handers'] before the overlay module's #after_build callback runs and adds the extra submit handler to it.

If this is correct, it must be a relatively recent change (and possibly a form API bug) - I'm positive that overlay_form_submit() used to get called.

But if my analysis is correct and it currently isn't getting called on the node form and Drupal is still working fine without it, then that's all the more reason to go ahead and commit this patch :)

In case of the modules form and the overlay module being disabled it seems to work fine either. In that case overlay_init() is not being called no more, but overlay-parent.js' fallback handles that nicely; no visual differences.

Ah yes, good point. I had noticed that worked too, but hadn't thought about why. In that particular case, it must indeed be due to the JavaScript fallback because it's difficult for a module's hooks to get called after the module is already disabled :)

I couldn't find an existing issue so I just created #877210: Validation and submit handlers added during #after_build don't always get called for the (possible) form API bug that led to the code removed here not getting called.