Cannot run certain batch processes (tests, update manager, etc) with the overlay enabled

This should take care of it. It defines /batch as an administrative path, which I suppose is not always technically true, but it won't keep non-administrative batch operations from opening in the parent window, because you get to /batch via a redirect anyway, not by clicking a link to /batch.

Except we don't want to define /batch as an admin page :-(.
Nothing prevents batches from being triggered from non-admin pages. We don't want the admin theme to flash in on the progress page.
That's exactly what #539022: Batch API should use the current theme to run the batches was about...

hook_admin_paths() by itself won't trigger the admin theme. (Actually, David_Rothstein wants to rename it hook_overlay_paths(), which for D7 makes sense, since all we're doing with the information is deciding which pages appear in the overlay.) The only way a batch will appear in the admin theme is if it runs inside the overlay, and the overlay already forces everything inside it to be in the admin theme. But AFAICT, this patch does not make the admin theme appear on non-admin batch pages appearing outside the overlay.

Got it - then fine by me, but hook_admin_paths() is indeed really misnamed ;-)

This is actually a case where hook_overlay_paths() might be worse - how do you use that to say that a path is sometimes in the overlay but sometimes isn't? With hook_admin_paths() there is at least a plan to explain the inherent ambiguity for paths like these via better documentation... see #651586: Improve the API documentation for hook_admin_paths()-related functions

However, this patch doesn't seem to work correctly:
1. It breaks the ability to do a batch outside of the overlay (for example, canceling a user account).
2. It's not clear to me what the overlay_close_dialog() changes mean and how they are related to this - and in the case where $redirect is NULL the behavior still seems broken; we define a $path variable that never gets used?

I wonder if we have to look at conditionally including 'batch' in hook_admin_paths() based on the current batch state, almost like we do for the theme in http://api.drupal.org/api/function/_system_batch_theme/7? - I hope not :)

subscribing

From #658720: Clean up overlay_close_dialog() and related code it seems that the batch process that starts when a manual module upgrade check is done also doesn't work correctly in the overlay. I assume that's due to the same issue as this one.

Yeah, aside from just checking for updates, you also can't install missing updates with overlay enabled (the new "Update manager" functionality). I was going to submit an "Overlay completely breaks the Update manager" issue, but I think that'd just be duplicate with this one. ;)

This seems to be the minimal fix that works for me. It is based directly on what @ksenzee did above, but for some reason, the issue I reported in #6 no longer seems to be a problem (a lot of fixes were made to the overlay javascript redirect code since then, so not sure if that's the reason).

There is more cleanup that could be done in overlay_close_dialog(), but we can leave that for a followup patch after this one gets in.

@dww, I don't seem to have the ability to test the update manager functionality either with or without the overlay. Can you check if it works with this patch? I expect there will be some weird-looking behavior since going to authorize.php takes you out of the overlay, but hopefully the functionality will at least work correctly.

@David_Rothstein: Yeah, that's a lot closer. However, when the batch runs in the overlay, I get this:

Notice: Undefined index: css in _batch_page() (line 65 of /.../includes/batch.inc).
Warning: Invalid argument supplied for foreach() in _batch_page() (line 65 of /.../includes/batch.inc).

With overlay disabled, the batch runs without errors.

Also, out of scope for this patch (and perhaps a reason to open that "Overlay and Update manager don't play nicely together" issue after all), but if you start the update manager process in the overlay, once you're redirected to authorize.php (which of course isn't in the overlay at all, by design), if you click on the "Administration pages" link on the final results page, you end up back at /admin without an overlay...

Notices around $batch_set['css'] are one of the side fixes in #629794: Scaling issues with batch API (first lines in the patch in, say, #44) - a fix there was needed to let the added tests run.

Not sure how the overlay interacts with the fact that the notices are triggered or not, though.

Ack! You can't run one of the most awesome improvements in D7 in the default configuration! Is there a button for "OMG FLAMING CRITICAL?" :)

I have all day tomorrow to work on core, so I'll put this at the top of the list.

Help us, ksenzee... you're our only hope...!

For extra added pressure, I'm going to be showing Drupal 7 to a room full of übergeeks at LinuxConf Australia next week for a hands-on tutorial session. I would *really* like to be able to tell them to download 7.x-alpha 1 and not "the latest dev release which worked this morning but aw crap Dries just broke it" ;)

(j/k Dries. <3 u)

David's patch applies with a bit of offset, and it works fine as far as I can tell now that the Batch API stuff has landed. No notices. I tested batch operations using SimpleTest, user cancellation, and update manager, and all is well. (Update manager is a bit rough as dww noted in #12, but that's out of scope here.) I know this is originally based on my idea, so it's maybe a little cheeky to RTBC it, but, well, RTBC.

Well, okay, dww says update manager is still borked for him. Trying to reproduce.

Sorry, there was a mixup with my local workspace. Now that I actually have the latest HEAD and this patch applied, I can confirm that update manager works in the overlay. Yee haw!

The admin link on the final landing page of authorize.php still brings you to the real admin page, not the overlay, as I explain in comment #12, but that's out of scope for this issue (and probably not going to be fixed for alpha1, if ever).

The only other bug I saw was multiple (+) icons, but ksenzee pointed me to #683128: Multiple 'add to shortcut' links appearing when adding fields which indeed fixes that problem.

Given other patches landing in the last few weeks, #10 is indeed RTBC, confirmed by the bot, ksenzee, and myself. Fire at will! ;)

YES!!!!!!!!!!!

Thank you SO much!! :D Confirmed it's working here too.

Committed to HEAD!

I'm still seeing unhappy results trying to use the update manager in the overlay, but it doesn't seem to get stuck on the batch API, so I think it's a separate issue (similar to what @dww is saying above, but I'm experiencing a more serious problem where authorize.php gets shown inside the overlay but doesn't work correctly in it at all).

See: #687594: Overlay and authorize.php don't play nicely together