WordPress.org

Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#9647 closed defect (bug) (fixed)

initial taxonomies

Reported by: arena Owned by: ryan
Milestone: 2.8 Priority: high
Severity: normal Version: 2.8
Component: Taxonomy Keywords: has-patch needs-testing
Focuses: Cc:

Description

In 2.7 initial taxonomies where created in wp-includes/taxonomy.php

In 2.8 initial taxonomies are created on event 'init' which appears after 'plugins_loaded' event

will propose a patch for this !

Attachments (4)

9647.diff (2.1 KB) - added by arena 9 years ago.
9647_1.diff (3.6 KB) - added by arena 9 years ago.
9647_2.diff (2.0 KB) - added by arena 9 years ago.
last changes
9647-drop-check.diff (492 bytes) - added by Denis-de-Bernardy 9 years ago.

Download all attachments as: .zip

Change History (17)

@arena
9 years ago

#1 follow-up: @arena
9 years ago

  • Keywords has-patch needs-testing added
  • Version set to 2.8

#2 in reply to: ↑ 1 @arena
9 years ago

the 2.8 changes was introduced by #9399 for a translation issue ...

@arena
9 years ago

#3 @ryan
9 years ago

Passing variable to __() will pollute the translation catalog.

Plugins should never do anything before init. Since we accidentally allowed registering taxonomies before init, we can try having registration calls made before init delay registration until init fires, but this can still result in plugin added taxonomies not having properly translated strings.

#4 follow-up: @Denis-de-Bernardy
9 years ago

@Ryan: couldn't the load_default_textdomain() call be moved further up? Is there any specific reasons for placing it so low in wp-settings.php?

#5 in reply to: ↑ 4 ; follow-up: @arena
9 years ago

@Ryan, @Denis-de-Bernardy : agree with all your comments,

  • about register_taxonomy() : + should initialize $wp_taxonomies if not set

if (!is_array($wp_taxonomies)) $wp_taxonomies = array();

+ maybe adding following test

if (isset($wp_taxonomies[$taxonomy])) return;

  • about #9399 patch, + create_initial_taxonomies() should use register_taxonomy(). + add_action( 'init', 'create_initial_taxonomies' ); should have the highest priority to be executed before any other add_action( 'init' ...
  • about wp-settings.php + load_default_textdomain() should be moved further up in wp-settings.php (apparently only needs constant WP_LANG_DIR). + maybe create_initial_taxonomies() code should be in wp-settings.php and we would get rid of the "add_action( 'init', 'create_initial_taxonomies' );" .

I am still a bit concerned about existing plugins that are doing things on wordpress "plugins_loaded" event. They could even define some new taxonomies that will be erased by the #9399 patch's "add_action( 'init', 'create_initial_taxonomies' );" .

#6 in reply to: ↑ 5 @Denis-de-Bernardy
9 years ago

Replying to arena:

I am still a bit concerned about existing plugins that are doing things on wordpress "plugins_loaded" event. They could even define some new taxonomies that will be erased by the #9399 patch's "add_action( 'init', 'create_initial_taxonomies' );" .

I use that hook in a Fixes plugin, that moves plugin hooks around depending on which plugins are loaded. Else there really isn't much use for it. (And even less use for the mu_plugins_loaded hook.)

#7 @arena
9 years ago

Here attached is a patch 9647_2.diff replacing the previous ones.

  • register_taxonomy() : as following tests added

if (!is_array($wp_taxonomies)) $wp_taxonomies = array();

if (isset($wp_taxonomies[$taxonomy])) return;

  • create_initial_taxonomies() using register_taxonomy()and add_action( 'init', 'create_initial_taxonomies', 0); with the highest priority.

I think load_default_textdomain() to be moved further up should be delayed to a next release, new ticket !

@arena
9 years ago

last changes

#8 @ryan
9 years ago

Looks good. We should still encourage waiting for init to register, but this will preserve old behavior.

#9 @ryan
9 years ago

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

(In [11122]) Allow taxonomy registration before init. Props arena. fixes #9647

#10 @Denis-de-Bernardy
9 years ago

Is this check truly needed?

if (isset($wp_taxonomies[$taxonomy])) 
    return;

I'm itching to open a ticket along the lines of "register_taxonomy() should allow to override a default taxonomy".

#11 @ryan
9 years ago

We can lose it.

#12 @Denis-de-Bernardy
9 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Let's drop the check then.

#13 @ryan
9 years ago

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

(In [11129]) Allow plugins to override default taxonomies. Props Denis-de-Bernardy. fixes #9647

Note: See TracTickets for help on using tickets.