WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#16117 closed defect (bug) (fixed)

Deleting theme in multisite throws errors

Reported by: talus Owned by:
Milestone: 3.1 Priority: normal
Severity: major Version: 3.1
Component: Multisite Keywords: has-patch
Focuses: multisite Cc:

Description

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.

Attachments (6)

16117.patch (645 bytes) - added by SergeyBiryukov 3 years ago.
greuben-16117.diff (494 bytes) - added by greuben 3 years ago.
remove unnecessary query args before redirecting
greuben-16117-rev2.diff (566 bytes) - added by greuben 3 years ago.
I think this looks better than my previous patch
16117-3.diff (574 bytes) - added by greuben 3 years ago.
16117-4.diff (690 bytes) - added by greuben 3 years ago.
as nacin suggested
16117.diff (1.6 KB) - added by ryan 3 years ago.
Fix redirect after connection information

Download all attachments as: .zip

Change History (30)

SergeyBiryukov3 years ago

comment:1 SergeyBiryukov3 years ago

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).

comment:2 nacin3 years ago

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

greuben3 years ago

remove unnecessary query args before redirecting

comment:3 greuben3 years ago

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

comment:4 nacin3 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 3.1

greuben3 years ago

I think this looks better than my previous patch

comment:5 ryan3 years ago

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

comment:6 ryan3 years ago

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

comment:7 talus3 years ago

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.

comment:8 follow-up: greuben3 years ago

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.

comment:9 greuben3 years ago

This is a caused by r17130

Version 0, edited 3 years ago by greuben (next)

greuben3 years ago

comment:10 nacin3 years ago

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.

comment:11 nacin3 years ago

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

comment:12 greuben3 years ago

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.

greuben3 years ago

as nacin suggested

comment:13 in reply to: ↑ 8 ryan3 years ago

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.

comment:14 ryan3 years ago

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.

comment:15 ryan3 years ago

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

comment:16 ryan3 years ago

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.

ryan3 years ago

Fix redirect after connection information

comment:17 PeteMall3 years ago

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

comment:18 ryan3 years ago

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

comment:19 ryan3 years ago

The other patches will break deletion over FTP.

comment:20 ryan3 years ago

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

comment:21 ryan3 years ago

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

comment:22 ryan3 years ago

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.

comment:23 ryan3 years ago

  • Resolution set to fixed
  • Status changed from new to closed

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

Note: See TracTickets for help on using tickets.