Make WordPress Core

Opened 12 years ago

Last modified 9 days ago

#23197 reopened defect (bug)

wp-activate.php, without explanation, does not load site plugins

Reported by: radiok's profile radiok Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.0
Component: Login and Registration Keywords: needs-testing needs-patch
Focuses: multisite Cc:

Description

I am the developer of a Wordpress plugin that modifies the Registration process. I am porting my code to Wordpress Multisite and am running into an odd obstacle. During the signup process, I store meta data in the signups table. I intend to restore that data after the user has activated their account using hooks located in wp-activate.php. I learned the hard way that wp-activate is altering the load sequence with the following flag.

define( 'WP_INSTALLING', true );

The preceding comment says "Define ABSPATH as this file's directory", which to me does not entirely jive up. Regardless, with this flag in place, when Wordpress would normally determine what plugins to load in wp_get_active_and_valid_plugins, instead it returns an empty array. I do not believe this is the desired behavior.

My solution has to been to create a small "must-use" plugin simply for my activation related code. This however, is not a desirable solution since must-use plugins must be manually installed. I cannot determine much of an alternate solution; I cannot modify that flag in a plugin at any level since my plugin would never be loaded for it to get a chance to modify the flag. It is a bit of a chicken and the egg problem.

Attachments (1)

23197.diff (591 bytes) - added by johnbillion 11 years ago.

Download all attachments as: .zip

Change History (25)

#1 @radiok
12 years ago

  • Severity changed from normal to trivial

OK, I figured out how to load my plugin, it must be Network Activated, however, I still think the comment and the code on wp-activate.php aren't completely in sync. A silly, trivial bug, but not without merit, and very little impact to resolve. That is, for someone who truly understands why the WP_INSTALLING flag is being thrown.

#2 @radiok
12 years ago

  • Summary changed from wp-activate does not load plugins to wp-activate, without explanation, does not load site plugins

#3 @johnbillion
12 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Yep, this is frustrating. See #18278 and #17948.

#4 @SergeyBiryukov
12 years ago

The preceding comment says "Define ABSPATH as this file's directory", which to me does not entirely jive up.

Created #23211 to remove the comment.

#5 @johnbillion
11 years ago

  • Resolution duplicate deleted
  • Status changed from closed to reopened

I'm going to reopen this for reconsideration separate from #17948. This is a silly bug for silly people and I think it needs to be fixed regardless of what's happening (or not happening) on #17948.

#6 @johnbillion
11 years ago

  • Keywords needs-patch added

@johnbillion
11 years ago

#7 follow-up: @johnbillion
11 years ago

  • Keywords has-patch dev-feedback added; needs-patch removed
  • Milestone set to Awaiting Review
  • Severity changed from trivial to normal
  • Version set to 3.0

23197.diff removes the definition of WP_INSTALLING from wp-activate.php so plugins are now loaded on this URL.

The addition of the wp_no_robots action on the wp_head hook and the $wp_query->is_404 are both copied from wp-signup.php.

I think this should go in as a stop-gap solution until #17948 is ready for inclusion (which may be a fair way off yet). It's ridiculous that we have this one URL that does not load plugins.

Any opponents?

#8 @SergeyBiryukov
11 years ago

  • Milestone changed from Awaiting Review to 3.7

Moving for review along with #24960.

#9 @johnbillion
11 years ago

  • Keywords dev-feedback removed
  • Milestone changed from 3.7 to 3.8

Punting to 3.8 so we can get it in early in a cycle.

#10 in reply to: ↑ 7 @nacin
11 years ago

Replying to johnbillion:

23197.diff removes the definition of WP_INSTALLING from wp-activate.php so plugins are now loaded on this URL.

This is surely here for other reasons. No idea what, as blaming this will take you to http://mu.trac.wordpress.org/changeset/543. WP_INSTALLING does quite a bit of weird stuff throughout core. If I had to guess, it was/is there for multisite installation purposes. I have no idea if it is still necessary now, but this needs a lot of testing to be sure.

#11 @jeremyfelt
11 years ago

I'm still grokking this and will test some more later, but at first glance it looks like the biggest chance of breakage is around assigning $current_blog and $current_site in ms-settings.php once the WP_INSTALLING definition is removed.

#12 @jeremyfelt
11 years ago

  • Keywords needs-testing added
  • Milestone changed from 3.8 to Future Release

I think we need to push this back to the next cycle. Not worth the chance in beta.

#13 @iseulde
11 years ago

  • Cc jannekevandorpe@… added
  • Summary changed from wp-activate, without explanation, does not load site plugins to wp-activate.php, without explanation, does not load site plugins

#14 @jeremyfelt
11 years ago

  • Component changed from Multisite to Login and Registration
  • Focuses multisite added

#15 @chriscct7
9 years ago

  • Keywords needs-refresh added

#16 @jeremyfelt
9 years ago

Digging a bit more into history on this. https://mu.trac.wordpress.org/ticket/1151 provides a bit more insight, though not much.

Part of me wonders if this is just precautionary, but it's hard to tell. If an activation key is provided, wpmu_create_blog() ends up firing and also ensures that WP_INSTALLING is set to true. That would be after plugins are already loaded if we removed the initial check. I followed the chain and never ran into anything that relied on it.

This still needs some good testing, probably with several popular plugins activated while working the activation process.

#17 @jeremyfelt
9 years ago

  • Keywords needs-patch added; has-patch needs-refresh removed

More info: WP_INSTALLING is set in wp-activate.php so that an activation link can be followed on a new subdomain without that subdomain yet existing in wp_blogs. In ms-settings.php, if WP_INSTALLING is set, then the information for the main site is loaded even though the new site is being activated.

Removing WP_INSTALLING would probably require a slightly refactored activation process.

#18 @jeremyfelt
9 years ago

#35443 was marked as a duplicate.

#19 @afercia
8 years ago

I've just experienced how trying to customise wp-signup.php and wp-activate.php is far from ideal :) I understand removing WP_INSTALLING is not going to happen soon, but maybe worth considering some improvements to make the customisation process less time consuming and more straightforward.

A few things noticed that could be improved:

  • different actions that fire in different places, for example do_action( 'signup_header' ) and do_action( 'activate_header' ) have a similar name but they fire at a different time, very confusing at first; not sure this can be solved
  • wp-activate would need some more hooks for parity with wp-signup, for example it misses a couple hooks around the content which wp-signup does have
  • buttons have a very unique styling, pretty inconsistent with the default WP styles. They don't use the .button CSS class so loading the .buttons.css stylesheet won-t have any effect; the body element also would need a wp-core-ui CSS class
  • I'm not sure to understand why wpmu_signup_stylesheet() and wpmu_activate_stylesheet() output inline styles, maybe it's just for historical reasons; ideally, deprecate them or move the styles to login.css
  • wp-activate fails to output a proper document <title> tag, not sure this can be solved

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


7 years ago

#21 @johnbillion
6 years ago

#44208 was marked as a duplicate.

#22 @herregroen
6 years ago

I'd like to propose a practical fix to this that while not the prettiest should be relatively easy to implement.

I've based this solution on the following:

  • WP_INSTALLING must be set before the WordPress environment is loaded.
  • WP_INSTALLING must be set during a multisite activation that installs a new blog as this puts WordPress into a state where a lot of data and tables do not exist and as such many core functions need to behave differently and plugins should not be activated.
  • Determining if an activation requires installing a new blog can only be determined after the WordPress environment is loaded.
  • Because we need to set WP_INSTALLING before we can determine whether or not it needs to be set it is always set so that installations of a new blog do not lead to massive issues.

I would suggest wp-activate.php be changed to the following flow:

  • WP_INSTALLING is set based on the presence of a query parameter.
  • The WordPress environment is loaded.
  • A new function is called to determine whether the current activation key requires the installation of a new blog.
  • If the result of this new function does not match the presence/absence of the query parameter the user is redirected to the same URL with the query parameter either stripped or added.

In addition to this when generating activation emails this query parameter should be added.

One important thing to note is that in wp-signup.php during the gimmeanotherblog flow a new blog is installed without WP_INSTALLING being true before WordPress is loaded. Meaning that in this instance all plugins will be loaded during the installation of a new blog with the potential risk of those plugins having hooks and filters that may attempt to make database queries at a time not all tables potentially exist.

Either the above flow should also be implemented wp-signup.php to prevent the above from happening or, if it's completely fine that the installation is only toggled later in the request then WP_INSTALLING should just straight up be removed from wp-activate.php with the expectation that plugin authors properly check for wp_installing() when using any hooks and/or filters that may be called during a blog installation.

#23 @zenithcity
4 years ago

There's a ticket I opened concerning this issue on fixing WordPress (but didn't know this one exists).

Here's a comment I pinned on the ticket:

Looking at the activation process in WP, defining the WP_INSTALLING constant is useful, but it's important that other operations are not treated irregularly just to conform to an instance (such as installation) that is currently running.

Beside the main site setup for the first time, site installation thereafter should not be breaking load processes just because we need to bail the installation instance

Last edited 2 years ago by sabernhardt (previous) (diff)

#24 @pacifika
9 days ago

The WP_INSTALLING constant on new user activation (wp-activate.php) breaks the site theme if the site uses MU plugin related functionality in the site header or footer, because MU plugins are not available as a result.

Note: See TracTickets for help on using tickets.