Opened 16 years ago
Last modified 4 years ago
#9296 reopened defect (bug)
Settings API & Permalink Settings Page Bug
Reported by: | jfarthing84 | Owned by: | jfarthing84 |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | major | Version: | 2.7.1 |
Component: | Permalinks | Keywords: | needs-patch settings-api |
Focuses: | administration | Cc: |
Description
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.
Attachments (7)
Change History (66)
#3
@
16 years ago
- Keywords has-patch added; register_setting settings api permalink settings removed
- Resolution set to fixed
- Status changed from assigned to closed
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.
#5
@
15 years ago
- Cc dimadin added
- Milestone changed from 2.7.2 to 2.8.4
- Priority changed from normal to high
- Resolution fixed deleted
- Status changed from closed to reopened
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.
#6
@
15 years ago
- Component changed from General to Administration
- Keywords needs-patch added; has-patch removed
- Milestone changed from 2.8.4 to 2.9
- Priority changed from high to normal
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)
#8
@
15 years ago
- Cc steph@… added
- Version changed from 2.7.1 to 2.9.1
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."
#9
@
15 years ago
- Milestone changed from Future Release to 3.0
- Version changed from 2.9.1 to 2.7.1
reset version
#11
@
15 years ago
- Keywords early added
- Milestone changed from 3.0 to 3.1
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.
#12
@
14 years ago
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.
#13
@
14 years ago
- Keywords 3.2-early added
- Milestone changed from Awaiting Triage to Future Release
#15
@
14 years ago
- Cc jjj@… added
Just bumped my head into this for a while trying to merge the bbPress plugin slugs into options-permalink.php. (cc'ed)
#17
@
14 years ago
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.
#18
@
14 years ago
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.
#21
in reply to:
↑ 20
@
13 years ago
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.
#22
@
13 years ago
Heh. Possibly this should be merged in with "make all the settings page actually use the settings API" sort of thing?
#23
@
13 years ago
I imagine this will get picked up as part of the settings screen iterations of 3.3, which is probably part of that, yeh.
#24
@
13 years ago
- Keywords 3.2-early removed
- Milestone changed from Future Release to 3.3
Related: #18285
#25
@
13 years ago
- Milestone changed from 3.3 to Future Release
Punting until we finally update the settings API.
#29
@
12 years ago
- Keywords has-patch added; early needs-patch needs-refresh removed
- Milestone changed from Future Release to 3.5
#30
@
12 years ago
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?
#32
@
12 years ago
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.
#36
@
12 years ago
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?
Update: actually ignore this, 9263.diff doesn't work. Working on it :)
#37
@
12 years ago
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.
#38
@
12 years ago
- Keywords needs-testing added
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!
#45
@
12 years ago
- Keywords needs-refresh added
@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?
#54
@
9 years ago
- Keywords needs-patch added; has-patch needs-testing 3.6-early settings-api needs-refresh removed
@chriscct7: Any interest in taking a stab at this as part of the other settings changes?
#56
@
6 years ago
Still broken: proper settings API setup won't save the user settings.
Must hook into:
add_action('load-options-permalink.php', 'my_load_options_permalink_callback'));
('admin_post'
wont work)
and process and save into database $_POST data manually.
#57
@
5 years ago
I can't believe this is still not fixed after a decade!
+1 @DrLightman for the workaround.
#58
@
5 years ago
Related: #38734
WordPress has had a Settings API for eight years, but the core settings screens in WordPress don't use it.
See also https://core.trac.wordpress.org/query?status=!closed&keywords=~settings-api
Contains options.php, options-head.php & options-permalink.php