Deleting theme in multisite throws errors

[talus]

Deleting a theme in multisite (3.1-RC2-17229) threw the following error:
(Attempting to delete constructor theme below)

Warning: fopen(/home/~~username~~/public_html/~~sitename~~.com/wp-content/themes/constructor/style.css) [function.fopen]: failed to open stream: No such file or directory in /home/~~username~~/public_html/~~sitename~~.com/wp-includes/functions.php on line 4273

Warning: fread(): supplied argument is not a valid stream resource in /home/~~username~~/public_html/~~sitename~~.com/wp-includes/functions.php on line 4276

Warning: fclose(): supplied argument is not a valid stream resource in /home/~~username~~/public_html/~~sitename~~.com/wp-includes/functions.php on line 4279

Then asks to confirm delete of Theme (blank space) by Anonymous.

I'm only able to reproduce this if I delete style.css manually before clicking the Delete link. Added check for file existence (just in case).

This is really edge -- if the theme doesn't have a style.css then it won't be listed.

remove unnecessary query args before redirecting

It displays the delete form again because action='delete-selected' is set.

I think this looks better than my previous patch

It'd be nice to delete what files there are rather than skipping the theme.

This seems so edge case that I think it can wait until 3.2.

Please note: I did not delete any files manually. In fact, to repro the test, I installed Constructor from the theme install, then deleted it immediately. Constructor is a "Featured" theme from WordPress.org, and this error occurred with every theme I tried.

You can't delete a theme if you don't have style.css because it wont show up in list.

@ryan
3.2? releasing 3.1 with errors is bad.

This is caused by r17130

If we're going to avoid remove_query_arg(), can we use add_query_arg() instead? Looks better.

For what it's worth though, I think remove_query_arg() is proper here.

Should this be part of $temp_args higher up? Does it apply to other redirects?

It does not apply to other redirects because query arg action in $referer is only set in the delete-selected verify form. Yes, it can move to $temp_args.

as nacin suggested

Replying to greuben:

You can't delete a theme if you don't have style.css because it wont show up in list.

I know. We have a broken themes list elsewhere, perhaps that should be included here.

@ryan
3.2? releasing 3.1 with errors is bad.

There are always errors. :-)

The latest patch seems good though.

Are any of you testing with FTP? I can't get any themes to delete, with or without these patches. Tons of notices are thrown on the Connection Information screen. It looks like all of the things admin.php sets up are not being set up.

Deleting a plugin throws the same notices but actually works in the end.

The redirect url in delete_theme() needs to be updated for network admin. Either pass a redirect url or add an is_network_admin() check inside the function.

Fix redirect after connection information

Attached patch by ryan works for the theme deletion over ftp issue. Need another patch for the notices.

(In [17237]) Fix net admin theme deletion over FTP. see #16117

The other patches will break deletion over FTP.

Actually, -3 works with FTP. -4 does not.

(In [17238]) Fix post theme delete redirect. Props greuben. see #16117

The notices when showing the Connection Information form during a delete via FTP are also present in 3.0. I think we can ignore those for 3.1. If someone wants to try to track the root cause down, however, that would be great.

See #16143 for the notices. Closing this ticket as fixed.

Related: #16171