Make WordPress Core

Opened 13 years ago

Closed 9 years ago

#20523 closed enhancement (fixed)

Disable autoloader when using class_exists()

Reported by: michaelheuberger's profile michael.heuberger Owned by: wonderboymusic's profile wonderboymusic
Milestone: 4.4 Priority: normal
Severity: normal Version: 3.3.1
Component: Bootstrap/Load Keywords: commit
Focuses: Cc:

Description

A couple of files inside the /wp-includes/pomo directory use class_exists() the wrong way, for example in /wp-includes/pomo/entry.php :

if ( !class_exists( 'Translation_Entry') ):

It should be:

if ( !class_exists( 'Translation_Entry', false) ):

The second parameter 'false' disables autoloading. This is important because I'm integrating some WP functions into another existing website which already comes with an Autoloader for some classes = confusion.

This should be easy to fix. Thanks :)

Attachments (2)

20523-revert.diff (1.7 KB) - added by markjaquith 9 years ago.
20523-revert.2.diff (2.2 KB) - added by markjaquith 9 years ago.

Download all attachments as: .zip

Change History (13)

#1 @kurtpayne
13 years ago

  • Cc kpayne@… added
  • Type changed from defect (bug) to feature request

Can you add your own autoloader to the stack before you add WordPress to your existing codebase?

spl_autoload_register( function( $className ) {
  // If you're the last on the stack, the other autoloader should
  // take care of its own classes, but if it fails to find $className
  // you'll end up here, just don't throw an exception
} );

This might prevent the errors you're seeing.

You could also look at loading only the pieces of WordPress you need with SHORTINIT
http://wordpress.org/support/topic/load-wp-core-in-another-framework-shortinit-wp_query?replies=3

#2 @michael.heuberger
13 years ago

Done, that works. But really, it's not cool and I'll remove that code once this bug has been fixed.

By the way, WordPress should come with an own bootstrapper to load only the pieces we need with SHORTINIT. Do not like the code there in that discussion forum. Shouldn't be like that.

#3 @nacin
11 years ago

  • Component changed from General to Bootstrap/Load

#4 @wonderboymusic
10 years ago

  • Keywords needs-patch good-first-bug added
  • Milestone changed from Awaiting Review to 4.4
  • Type changed from feature request to enhancement

We can probably do some of this

#5 @wonderboymusic
10 years ago

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

In 34348:

Pass false as the 2nd argument to class_exists() to disable autoloading and to not cause problems for those who define __autoload().

Fixes #20523.

#6 @westonruter
10 years ago

@wonderboymusic This change to wp_ajax_add_menu_item broke a plugin. We had defined an autoloader for our plugin namespace and then added a filter like:

add_filter( 'wp_edit_nav_menu_walker', function () {
    return __NAMESPACE__ . '\Walker_Nav_Menu_Custom_Fields';
} );

Since the class_exists() now prevents autoloading, it then caused the class to not get loaded and the add-menu-item Ajax call would fail returning with 0 due to:

if ( ! class_exists( $walker_class_name, false ) )
    wp_die( 0 );

Now, it's easy enough to change our plugin to explicitly require_once the class file in the filter:

add_filter( 'wp_edit_nav_menu_walker', function () {
    require_once( __DIR__ ' . '/class-walker-nav-menu-custom-fields.php' );
    return __NAMESPACE__ . '\Walker_Nav_Menu_Custom_Fields';
} );

But plugins that use autoloaders aren't going to expect to have to do this.

#7 @nacin
9 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Are we concerned that this will break things, per @westonruter's comment?

I don't see how class_exists() is being used in the wrong way here.

#8 @markjaquith
9 years ago

  • Keywords good-first-bug removed

I think we should only be doing this for hardcoded class checks. Not variable ones.

#9 @markjaquith
9 years ago

20523-revert.diff reverts the completely variable instances.

#10 @markjaquith
9 years ago

  • Keywords commit added; needs-patch removed

@nacin reviewed.

#11 @markjaquith
9 years ago

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

In 35749:

Do not pass FALSE as second parameter in variable class_exists() checks

Because these are generally plugin-provided, we want plugins to be
able to use autoloaders.

fixes #20523

Note: See TracTickets for help on using tickets.