Settings API & Permalink Settings Page Bug

[jfarthing84]

Although there is a hook in the options-permalink.php to insert custom settings, it does not actually save any custom setting which is added to that page. Instead of posting to options.php like all the other options pages, it posts to itself and only handles the form data which is built into the wordpress core. It should be implemented on that page to also store custom settings that may be hooked onto that page.

Contains options.php, options-head.php & options-permalink.php

In order to make the options-permalink.php page capable of saving custom options hooked in using the add_settings_field() and register_setting() functions, I modified three files. The plan was to make options-permalink.php submit to post.php, like all of the other options pages, instead of submitting to itself and handling it's own post.

I had to modify options-permalink.php by changing the form HTML to post to options.php and also added the settings_fields('permalink') in place of the wp_nonce_field('update_permalink'). I changed the line for the update message handling from $_POSTsubmit? to $_GETupdated? since we will no longer be posting to this page, but the updated value will be passed back upon success. Finally, I removed the whole form processing code to insert into options.php.

With options.php, I added the form processing code from options-permalink.php right before the custom options get saved. I used an if clause to obviously only save those options when options-permalink.php is submitted.

I then found with the modifications made to those two files that upon successfully updating the permalinks, I would receive two different success messages. One was coming from options-permalink.php's own success messages, plus one from options-head.php. So, I modified options-head.php to not display any messages if it is on the options-permalink.php page and allow options-permalink.php to use it's own notifications.

In the end, all is working and you can now save custom settings from the permalinks settings page.

Since this is still not fixed, people ​could spend a lot of time trying to figure out where they made mistake in adding field on permalinks page, unaware that there is a bug that prevent them from it.

This is possibly something which affects other pages.. there may already be an API for this, i'm not sure..

milestone: current trunk for new defects, point release back-ports at commiters discretion, future release for all other defects(long standing)

Definitely affecting other pages. I just tried to add some settings to options-privacy.php and failed. On submitting changes, I get an error on options.php: "Error! Options page not found."

reset version

This patch has the right idea in mind.

Does someone feel like updating the patch?

I'm moving this to 3.1 for now due to no patch.

I think I'd rather drop the requirement to post to options.php. Move the validation and update logic into a function that can be called from options-permalink.php, maybe. Make the options pages use the registration API so we don't have to maintain a big array of all default options.

Just bumped my head into this for a while trying to merge the bbPress plugin slugs into options-permalink.php. (cc'ed)

Updated patch for 3.1.

Looking to fast-track this in to 3.2 early for sure. The bbPress plugin and future versions of BuddyPress should rely on hooking into this permalinks screen rather than recreating their own.

Looking over my patch, I'm wondering if messages should be handled via add_settings_error() in options.php as well? The permalink messages depend on some variables, so if those messages are moved, those variables will either have to be moved or copied (since they are used elsewhere in options-permalink.php). Ryan said he would like to see the messages moved as well, but just to a different function, not options.php.

Thoughts? I'd be willing to take this on to the best of my abilities, just want some direction on what might be the best route.

updating keywords

Bump. Can we fix this please? Getting emails about it now.

Replying to Otto42:

Bump. Can we fix this please? Getting emails about it now.

If that's all it takes, allow me to gun it to 88, go back to 2009, and send you a few.

Heh. Possibly this should be merged in with "make all the settings page actually use the settings API" sort of thing?

I imagine this will get picked up as part of the settings screen iterations of 3.3, which is probably part of that, yeh.

Related: #18285

Punting until we finally update the settings API.

What can I do to make this happen in 3.4?

Related: #21488

Latest patch from @andrewryno is stale now too, but there seems to be more interest in #18285 and #21488, which both affect anything written to fix this (and should fix this ticket in the process).

Refreshed

Related #21824, #18285

Until we revamp the whole settings handling code, I guess it's better to have consistency, as an intermediate step.

Could we move that whole block inside if ( 'permalink' == $option_page ) { to a function, though? There should be a hook for it, right?

Okay, I'm convinced. Seems like this could help #14345 and others.

Let's see if we can make this work more like everything else, even if just a little.

• 9296.cat-tag-base.diff​ shows how category_base and tag_base can get pulled into a sanitize_option() callback. Doing this for permalink_structure is a bit more tricky.

• But if we are able to handle permalink_structure through sanitize_option(), we could issue our own add_settings_error() call to handle our custom 'Permalink structure updated.' message.

• A lot of variables used in the options.php branch aren't set.

In general, I'd like it if options.php can stay as generalized as possible. Let's see what we can do to make this work.

Refreshed

In 9296.3.diff​ tested and refreshed. Seems to work well for me and for anything registered to permalink with register_setting. We can also move settings errors/updates out from options-permalink.php and into options.php to keep things closer to each other, and to avoid the 'permalink' != $option_page hack around add_settings_error in options.php. What do you think?

Nitpicking: not sure if it actually matters in terms of validation in HTML5, but I guess in 9296.3.diff​, method="post" in line 135 of options-permalink.php should stay in lower case. See [14626] for a related change.

Thanks @SergeyBiryukov!

In 9296.4.diff​ fixed the updating of the permalink structure in .3 and .2. Moved settings errors to options.php with add_settings_error, though I now feel bad about code duplication in options.php and options-permalink.php, however, hopefully it can all be easily cleaned up and migrated when #18285 is in place.

I also tested against #14345 and could not reproduce the issue with the new patch, though I'd love someone to test it out, especially on IIS.

Thanks!

@nacin / @kovshenin / @johnnamesjacoby / @jfarthing84

Would you be interested in refreshing this code once 3.5.1 is out in the next couple of weeks to get this into 3.6?

In 24060:

Redirect after save on options-permalink.php to ensure permalinks are fully flushed. fixes #9296. see #14345 for more.

I switched the two ticket references in [24060] — fixes #14345, see #9296 for more.