Add validation for Custom Structure permalinks

[SteveNetRivet]

As a premium theme developer, we see many users confused by the Custom Structure option in the Permalinks settings screen. They often type in static paths without any structure tags - for example, typing in "About" which breaks all internal linking except their page which happens to have this slug.

Please add to this settings box some level of validation so that users cannot save changes if their permalink structure contains no structure tags in the Custom Structure box. Consider including helpful feedback and a link to the Help > Permalink Settings dropdown found in this settings area for further reading.

[swissspidy]

Please add to this settings box some level of validation so that users cannot save changes if their permalink structure contains no structure tags in the Custom Structure box.

Such a validation is pretty easily doable. We could check for existence of the % sign alone or even the available tags themselves (%postname% et al).

At least the documentation could be improved, perhaps warning users that they could easily render their site unusable with a bad setting.

See also: #29872

[johnbillion]

@rockwell15 Thanks for the patch, but enforcing %post_id% or %postname% as the last element in the permalink structure is too restrictive. I've seen sites with funky structures that include things such as a trailing /read element.

I think testing for the existence of a % symbol is enough validation.

[rockwell15]

Updated the error message

[rockwell15]

@johnbillion Ok, thanks. I updated the patch

[swissspidy]

@rockwell15 The latest patch works well. Some notes:

Coding standards in general should be followed, see ​https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/. For example, strict comparison should be used in conditions (e.g. false === strpos... instead of false == strpos...). Always use braces, etc. Also, the error message needs to be translatable.

While testing the patch I realized that sanitize_option() already does the error handling for us. 35936.diff​ is an alternative approach taking advantage of that. Wording of the error message needs to be improved though.

[rockwell15]

@swissspidy Thanks for the tips!

So the only thing left to do for this ticket is improve the wording of the error message?

[swissspidy]

@rockwell15 Yeah, I suggest going forward with 35936.diff​ and fine-tune the error message.

We need something between "No structure tag used! Learn more" and "Make sure to include a structure tag." (from 35936.3.patch​). Not too harsh, not too long, and informative about what went wrong.

[rockwell15]

@swissspidy Hows about "A structure tag is required when using custom permalinks."

Also, what do you think about changing the link to jump to the custom permalinks section so it's easier to follow:
​https://codex.wordpress.org/Using_Permalinks#Choosing_your_permalink_structure

[swissspidy]

Replying to rockwell15:

@swissspidy Hows about "A structure tag is required when using custom permalinks."

Also, what do you think about changing the link to jump to the custom permalinks section so it's easier to follow:
​https://codex.wordpress.org/Using_Permalinks#Choosing_your_permalink_structure

Yeah that could work.

[rockwell15]

Updated the error message & link

[voldemortensen]

Punting from 4.6 unless a patch with unit tests and a review comes soon.

[swissspidy]

35936.2.diff​ makes the permalink structure more strict and adds a test for the sanitization part.

[swissspidy]

Simplify the error string

[ocean90]

35936.3.diff​ looks good so far.
For the placeholder we should add a translator comment /* translators: %s: Codex URL */. And there should be a redirect in case of an error too (PRG pattern). options-permalink.php?error=true should be fine.

[swissspidy]

What seems like an easy task has once again proven to be quite a mess…

35936.4.diff​ adds the translator comment and keeps the redirect in any case. Since sanitize_option() relies on add_settings_error() I had to rewrite the whole message handling to use add_settings_error() / get_settings_errors(). That's why the diff looks so huge, when it basically only brings options-permalink.php in line with the rest of the admin using the settings API.

[ocean90]

35936.4.diff​: The change to src/wp-admin/admin-header.php isn't necessary, $parent_file is options-general.php for all options-* screens.

The block

if ( ! get_settings_errors() ) { 
	$wp_rewrite->set_permalink_structure( $permalink_structure ); 
}

can be removed too. $permalink_structure will be the same as before and WP_Rewrite::set_permalink_structure() does a $permalink_structure != $this->permalink_structure check. get_settings_errors() is also unreliable because it returns errors for all settings.

[swissspidy]

Thanks for your valuable feedback! 35936.5.diff​ is the fifth round of improvement.

Moving to 4.6 for consideration.

[swissspidy]

In 37747:

Permalinks: Validate custom permalink structures.

Custom permalink structures require at least one valid structure tag, e.g. %postname%. If none is included, it would leave users with broken permalinks.
Let's make sure this won't happen by validating the permalink structure.

Adds unit tests.

Props rockwell15 for initial patch.
Fixes #35936.