WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 3 months ago

#11282 reopened defect (bug)

Bizarre Behavior When wp-content Missing

Reported by: miqrogroove Owned by: westi
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 2.8.4
Component: Themes Keywords: has-patch
Focuses: Cc:

Description

Steps to reproduce:

  1. Begin and complete a normal installation, but skip or remove the wp-content directory.
  1. Try to view the Dashboard and the Visit Site link.

Expected Result: WP did not install, wp-content is missing.

Actual Result: Dashboard is visible, site's front page is not. In /wp-admin/error_log

PHP Warning: array_keys() [<a href='function.array-keys'>function.array-keys</a>]: The first argument should be an array in /wp-includes/theme.php on line 481

Attachments (2)

11282.diff (4.4 KB) - added by westi 4 years ago.
A first pass patch ugly and wide ranging
11282-check-content-dir-at-setup.diff (715 bytes) - added by ifrins 3 years ago.

Download all attachments as: .zip

Change History (35)

comment:1 dd324 years ago

I'm not too sure of the need for this..

Then again, a simple addition could "fix" it:

if ( ! is_dir(WP_CONTENT_DIR) )
   wp_die( sprintf('Your wp-content directory(%s) is missing.', WP_CONTENT_DIR) );

It comes down to performance perhaps, Is an extra IO check needed when this would only occur under a incorrect/incomplete install?

It could alternativly only be added to the installer.. rather than wp-settings.php

comment:2 scribu4 years ago

  • Milestone changed from 2.9 to Future Release

+1 on adding it to the installer and not wp-settings.php

Moving to Future Release until a patch is available.

comment:3 in reply to: ↑ description lloydbudd4 years ago

miqrogroove, it might help if you describe how you encountered this issue?

NOTE: if done for run-time, any check should likely only be done on the wp-admin side, we've had issues previously when we used to check for theme, see Ticket #3907 "WP reverts to default theme on file access collision"

comment:4 follow-up: miqrogroove4 years ago

lloydbudd, I forgot to upload the wp-content folder before running the install script. That's not really the point though. The arguments to array_keys aren't handled correctly.

dd32, the IO check only needs to happen when the theme list is empty (but preferably also during install). It should be trivial to do is_dir() right before array_keys() craps out.

comment:5 miqrogroove4 years ago

p.s. Wouldn't there be other obvious checkpoints, like when no template is found for the front page?

comment:6 in reply to: ↑ 4 lloydbudd4 years ago

Replying to miqrogroove:

lloydbudd, I forgot to upload the wp-content folder before running the install script. That's not really the point though.

The likelihood of others encountering an issue is always relevant.

comment:7 miqrogroove4 years ago

I didn't want to focus on the scenario. Imagine if a folder was accidentally deleted or renamed on a production site, and all of a sudden the front page just goes blank. Or what if it's just the style.css file?

comment:8 follow-up: miqrogroove4 years ago

  • Milestone changed from Future Release to 3.0

I also confirmed in 2.9.2 the front end throws a WSOD with no errors reported when the theme is missing for any reason. This needs consideration in 3.0.

comment:9 dd324 years ago

  • Milestone changed from 3.0 to Future Release

No traction in recent times. No patch. Moving to Future Release.

comment:10 in reply to: ↑ 8 westi4 years ago

Replying to miqrogroove:

I also confirmed in 2.9.2 the front end throws a WSOD with no errors reported when the theme is missing for any reason. This needs consideration in 3.0.

This has been there for a while ever since we removed the validate_current_theme check on the frontend due to it not being reliable on some servers to do this.

As recently discussed on wp-hackers I think a better error page is needed here and I will add this soon.

westi4 years ago

A first pass patch ugly and wide ranging

comment:11 westi4 years ago

  • Keywords has-patch added
  • Status changed from new to accepted

That is a first pass - error is ugly for now and I had to touch a lot of code to get it working.

Untested in multisite mode at present.

comment:12 miqrogroove4 years ago

Hi westi (and dd32), I noticed your patch says @since 3.0.0. Do we need to reset the milestone for this ticket?

comment:13 nacin4 years ago

  • Milestone changed from Future Release to 3.0

westi has indicated he wants to do this soon, so future -> 3.0.

comment:14 westi4 years ago

Yeah. Musing on getting this right.

The patch has since 3.0.0 in it as I wrote it for now.

Need to unuglify and test for multisite first.

comment:15 westi4 years ago

  • Component changed from Warnings/Notices to Themes
  • Keywords needs-patch added; has-patch removed
  • Milestone changed from 3.0 to 3.1

I'm still in the camp for a better error page but I would like to leave this till 3.1 as it is not a major issue and doesn't occur frequently.

I want to see if we can maybe find a good solution to this - maybe we can come up with a way of using the default theme but displaying an error in the backend when we can't find the theme files.

comment:16 nacin3 years ago

  • Milestone changed from Awaiting Triage to Future Release

comment:17 ifrins3 years ago

  • Cc ifrins added

Added a patch to check if the wp-content directory exists before starting the installation in wp-admin/setup-config.php

comment:18 ifrins3 years ago

  • Keywords has-patch added; needs-patch removed

comment:19 thee172 years ago

As of 3.4 there is an error on the dashboard "ERROR: The themes directory is either empty or doesn’t exist. Please check your installation."

The plugins page looks like you have no plugins with no warning.

The themes page looks funny but no warning.

Media when uploaded creates a wp-contents/uploads folder.

If you try to install a theme or plugin you get an "ask for credentials" screen.

comment:20 thee172 years ago

  • Cc charles@… added

comment:21 MikeHansenMe20 months ago

  • Keywords close added

I agree with thee17. The red error message in the dashboard seems sufficient. Proposing we close the ticket.

comment:22 scribu20 months ago

  • Milestone Future Release deleted
  • Resolution set to worksforme
  • Status changed from accepted to closed

comment:23 miqrogroove19 months ago

#21931 was marked as a duplicate.

comment:24 miqrogroove19 months ago

  • Resolution worksforme deleted
  • Status changed from closed to reopened

More discussion is needed before closing this ticket.

comment:25 scribu19 months ago

Don't just reopen a ticket with a blanket statement like that. Bring some new information to the table.

comment:26 MikeHansenMe19 months ago

After looking into the issue again, I still think the red error message is noticeable. The only situation I could still see a problem is if the user has turned off the "Right Now" from their Dashboard.

comment:27 bpetty18 months ago

  • Keywords close removed
  • Milestone set to Awaiting Review

So, it seems like this ticket needs to either decide if it's about improved warnings/notices in admin, or if added notices and possible fix during install is sufficient.

For better warnings/notices just for the case of themes, see #21931.

comment:28 bpetty18 months ago

  • Cc bpetty added

comment:29 miqrogroove18 months ago

They are the same bug, that's why I marked #21931 as a duplicate of this older one. You can't just fix it in the installer and then let the site W.S.O.D. at some other time without informing the admin that the site is broken. I wasn't being selective about which aspects need to be fixed.

comment:30 bpetty18 months ago

The OP describes problems very specific to the installation process here while #21931 is specific to the lack of notifications (yes, about the same problem mostly) in the admin after WordPress has been installed.

They are similar tickets, but really do identify two different issues in two different workflows that will likely require two different patches (or maybe the second patch utilizing code in the first - but still two patches).

I'm not saying one should be fixed without the other, just keeping the tickets separated since they are two different problems.

comment:31 miqrogroove18 months ago

Actually I don't think #21931 is about lack of notifications in the admin. That would be a third bug if you are trying to keep them separate.

comment:32 ircbot4 months ago

This ticket was mentioned in IRC in #wordpress-dev by tierra. View the logs.

comment:33 nacin3 months ago

#26143 was marked as a duplicate.

Note: See TracTickets for help on using tickets.