Turning on permalinks, CPT rewrites are ignored

[andy]

I'm working on rewrite issues in a plugin that registers custom post types. Everything works perfectly if rewrite rules already exist. However, when switching away from the default permalinks (?p=123), no CPT rewrite rules are added. I have to trigger another flush (e.g. by reloading the Permalink Settings page) before the CPT rules appear and my plugin's URLs work.

This bug is due to ​this check. When switching from no permalink_structure to some permalink_structure, the empty permalink_structure option prevents the rewrite rules and permastructs being added, even though they should be allowed in this case. The result is that the rewrite_rules are generated without any CPT rules.

I suppose the option check is purely for performance reasons. The simplest fix would be to omit the option check which would cause register_post_type to take a little bit longer than necessary on sites without a permalink structure. If this is unacceptable, the fix can ignore the option when is_admin() (or some other test).

[andy]

add is_admin() check

[scribu]

Sounds like a good idea to me, but I don't think it warrants being put into the 3.2.1 milestone, since it's been this way since CPT rewrites were first introduced, in 3.0: [12923].

[pavelevap]

Related?

​http://core.trac.wordpress.org/ticket/16736

[andy]

In defense of the 3.2.1 designation, this is a backwards compatible bug fix in the existing API rather than new API functionality.

The damage this bug does is fairly subtle because the correct behavior eventually occurs. The registered rules are obeyed on the second flush after permalink_structure is populated. Flushes are triggered by many common actions: loading the Permalink Settings page, activate a plugin, save a plugin option, do an automatic upgrade, etc.

It is incorrect for the first flush to produce a different set of rules than the second flush. The patch I proposed is an internal change that fixes incorrect behavior.

Replying to pavelevap:

Related?

​http://core.trac.wordpress.org/ticket/16736

Yes, good find.

[nacin]

Patch is quite smart.

Moving back to 3.2.1. Doubt it'll get in, but wouldn't object for 3.2.x if ryan likes it.

[nacin]

Let me muse for a moment.

If the rules are flushed when we're in the admin, then permalink_structure would become populated with basic rules for post types (and, I guess, nothing else).

In a few areas of core, we do check '' == get_option('permalink_structure') to decide whether permalinks are in use, rather than checking $wp_rewrite. I imagine many plugins including some of ours might be the same.

Does this patch risk these rules ending up in the DB even with default permalinks on?

[andy]

Replying to nacin:

If the rules are flushed when we're in the admin, then permalink_structure would become populated with basic rules for post types (and, I guess, nothing else).

It is already true that rules are flushed when you're in (a subset of) the admin because GET /wp-admin/options-permalink.php flushes them. However, this has no effect on, but rather is predicated on, permalink_structure. Flushing populates the array option rewrite_rules if there is a permalink_structure, while the string option permalink_structure is what you save on the options-permalink page.

In a few areas of core, we do check '' == get_option('permalink_structure') to decide whether permalinks are in use, rather than checking $wp_rewrite. I imagine many plugins including some of ours might be the same.

The permalink_structure option check is still valid; $wp_rewrite wouldn't be able to provide a better check in the context of this patch because it doesn't know that the option is going to change in a few milliseconds. The problem arose between the time when you clicked Save and the time when WordPress registered the intention of that click (to enable permalinks). During init, the plugin called a registration function that sees permalinks are disabled and infers that there will be no need to add anything to the $wp_rewrite object properties. That inference is the error behind the error.

Does this patch risk these rules ending up in the DB even with default permalinks on?

If anything, this patch will cause erroneous rules to be removed sooner than they otherwise would.

[nacin]

Looks like I mixed permalink_structure up with rewrite_rules.

Thanks for the further justification. +1 from me.

[azaozz]

This came in a bit too late for inclusion in 3.2.1, moving to 3.2.2

[ryan]

In [18443]:

Fix CPT rewrite generation when turning on permalinks. Props andy. see #18040 for trunk

[nacin]

I imagine Ryan left this open for 3.2.2, but wanted it up soak in trunk first.