Make WordPress Core

Opened 12 years ago

Closed 12 years ago

#19248 closed defect (bug) (invalid)

menu_page_url() should return unencoded URL, and echo encoded URL

Reported by: nathanrice's profile nathanrice Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.0
Component: Plugins Keywords: has-patch
Focuses: Cc:

Description

See #18274

menu_page_url() should use esc_url_raw() when returning, and use esc_url when echoing.

Use case:
Let's say you created a custom post type (with its own menu) called restaurant, and you want to put a settings submenu under the top level restaurant menu.

$menu = menu_page_url( 'restaurant-settings', 0 );
// Unencoded http://example.com/wp-admin/edit.php?post_type=restaurant&page=restaurant-settings
// Encoded http://example.com/wp-admin/edit.php?post_type=restaurant&page=restaurant-settings

add_query_arg( 'reset', 'true', $menu );

If $menu is encoded, it can't be passed to add_query_arg(), see #18274.

Attachments (1)

19248.diff (382 bytes) - added by nathanrice 12 years ago.
Do not encode on return

Download all attachments as: .zip

Change History (5)

@nathanrice
12 years ago

Do not encode on return

#1 @nacin
12 years ago

  • Version changed from 3.2 to 3.0

#2 follow-up: @westi
12 years ago

I don't like the idea of changing the way this function works in a backwards incompatible way which is what this effectively does.

Can you not get the url for the settings menu by using the result of add_settings_page(...) as the slug you pass to menu_page_url to solve your usecase ?

#3 in reply to: ↑ 2 ; follow-up: @nathanrice
12 years ago

Replying to westi:

Can you not get the url for the settings menu by using the result of add_settings_page(...) as the slug you pass to menu_page_url to solve your usecase ?

I could, but that doesn't solve the issue. menu_page_url() returns an escaped URL. add_query_arg() barfs when you pass it an escaped URL. So, either we're OK with these two functions NOT being able to work together, or we need to find a way to fix it. As it stands, you cannot take the result of menu_page_url() and pass it to add_query_arg().

#4 in reply to: ↑ 3 @westi
12 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

Replying to nathanrice:

Replying to westi:

Can you not get the url for the settings menu by using the result of add_settings_page(...) as the slug you pass to menu_page_url to solve your usecase ?

I could, but that doesn't solve the issue. menu_page_url() returns an escaped URL. add_query_arg() barfs when you pass it an escaped URL. So, either we're OK with these two functions NOT being able to work together, or we need to find a way to fix it. As it stands, you cannot take the result of menu_page_url() and pass it to add_query_arg().

I'm happy with these two functions not working together.

menu_page_url() was just meant to be a simple helper to stop you having to build urls to registered admin menus manually and it serves this purpose extremely well.

Whether or not returning an escaped url is the right thing it is what we have now, what plugins are using and works for the normal use case of the function which is getting back a url to echo yourself rather than having it printed straight away.

Closing as invalid as I don't see this as a bug.

If we were to change anything it would be simplest to have this function take an array of extra arguments to add to the query string for you - but I don't think that is required.

Note: See TracTickets for help on using tickets.