WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 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 5 years ago.
9647_1.diff (3.6 KB) - added by arena 5 years ago.
9647_2.diff (2.0 KB) - added by arena 5 years ago.
last changes
9647-drop-check.diff (492 bytes) - added by Denis-de-Bernardy 5 years ago.

Download all attachments as: .zip

Change History (17)

arena5 years ago

comment:1 follow-up: arena5 years ago

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

comment:2 in reply to: ↑ 1 arena5 years ago

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

arena5 years ago

comment:3 ryan5 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.

comment:4 follow-up: Denis-de-Bernardy5 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?

comment:5 in reply to: ↑ 4 ; follow-up: arena5 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' );" .

comment:6 in reply to: ↑ 5 Denis-de-Bernardy5 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.)

comment:7 arena5 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 !

arena5 years ago

last changes

comment:8 ryan5 years ago

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

comment:9 ryan5 years ago

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

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

comment:10 Denis-de-Bernardy5 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".

comment:11 ryan5 years ago

We can lose it.

comment:12 Denis-de-Bernardy5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Let's drop the check then.

comment:13 ryan5 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.