Make WordPress Core

Opened 9 years ago

Last modified 2 years ago

#34669 assigned enhancement

Custom back_link in wp_die and provide a filter

Reported by: eventualo's profile eventualo Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 4.4
Component: General Keywords: has-patch changes-requested reporter-feedback
Focuses: Cc:

Description

At the moment, inside _default_wp_die_handler function, the $args['back_link'] (boolean) set to true allow to show a generic javascript back link: <a href='javascript:history.back()'>.

I know that it's possibile to write own function handler using wp_die_handler filter, but accepting a custom back url, and not only a boolean, could be an easy way without rewrite all the function.

In some cases it could be useful, here is an example.
I wrote a plugin that creates custom roles with custom caps. I use the meta cap filter to let user of custom roles to edit users of other roles, but not admin users.
So when an user of custom roles tries to bulk edit/promote/delete admin and not-admin users together, the user gets a wp_die message like: You can’t edit that user..
When the user goes back with browser, the user will see again the old users page with old (and wrong) info, because the bulk action has edited properly the not-admin users and leave the admins untouch.

I think it could be useful to provide the back url to wp_die message, so you can load the updated version of previous page.

I propose that $args['back_link'] can accept 3 types of values:

  • false: no back link shown (as now);
  • true: show javascript back link shown (as now);
  • {url}: show passed url.

In this way I can use wp_die for custom messages in my plugin pages, like:
wp_die( 'Error message', '', array( 'back_link' => admin_url('whatever.php') ) );

Then, it could be useful to have a filter inside _default_wp_die_handler function to customise error messages of default dashboard pages.
We could use the arguments ($message, $title, $args) to provide context to the filter, e.g.:

apply_filters( 'default_wp_die_handler_back_link', $r['back_link'], $title, $args )
apply_filters( 'default_wp_die_handler_back_link_'.sanitize_key( $title ), $r['back_link'], $args )

Or maybe use wp_get_referer to add more context.

I hope my explanation has been clear.

Attachments (2)

34669.diff (1.0 KB) - added by eventualo 9 years ago.
34669.2.diff (732 bytes) - added by desrosj 2 years ago.

Download all attachments as: .zip

Change History (11)

#1 @eventualo
9 years ago

  • Component changed from Administration to General

I attach a patch with a suggested edit.
I added a 4th accepted value for $args['back_link']:

  • 'referer': that shows the wp_get_referer url, if any, or the default javascript back, if not referer.

So, in my sample scenario as explained in first post, a possible filter to provide a back link to reload users' page in dashboard could be:

<?php
function my_custom_wp_die_handler_back_link ( $back_link, $message, $title, $args, $referer ) {

        if ( strpos( $referer, 'wp-admin/users.php' ) !== false ) {
                $back_link = 'referer';
        }
        return $back_link;
}
add_filter( '_default_wp_die_handler_back_link', 'my_custom_wp_die_handler_back_link', 10, 5 );
Version 0, edited 9 years ago by eventualo (next)

@eventualo
9 years ago

#2 @eventualo
8 years ago

  • Keywords dev-feedback added

This ticket was mentioned in Slack in #core by noisysocks. View the logs.


5 years ago

#4 @isabel_brison
5 years ago

  • Keywords has-patch needs-refresh added
  • Owner set to eventualo
  • Status changed from new to assigned

#5 @desrosj
2 years ago

  • Keywords dev-feedback needs-refresh removed
  • Milestone set to 6.1
  • Owner eventualo deleted

Hi @eventualo,

Apologies that it took so long to receive a proper response on this one!

While I don't necessarily agree with the suggestions for allowed values of $args['back_link'], I think it's reasonable to allow $args to be filtered before calling the desired die handler.

I'm attaching a patch to introduce a filter hook for doing just that.

One note for history.back() is that depending on the browser and how the relevant settings are configured based on user preference, form values may be preserved. If referrer is used, this would not be the case.

@desrosj
2 years ago

This ticket was mentioned in Slack in #core by chaion07. View the logs.


2 years ago

#7 @hellofromTonya
2 years ago

@desrosj while the patch 34669.2.diff makes the args filterable, 'back_link' is used as a flag (i.e. boolean). If I'm understanding @eventualo's request, they are looking for a filter to modify the href attribute in back link:

Current code is this:

if ( isset( $parsed_args['back_link'] ) && $parsed_args['back_link'] ) {
        $back_text = $have_gettext ? __( '&laquo; Back' ) : '&laquo; Back';
        $message  .= "\n<p><a href='javascript:history.back()'>$back_text</a></p>";
}

I agree with you @desrosj about the proposed implementation and history.back().

But I'm questioning the need for an args filter. Why? The reporter noted they are invoking wp_die() in their plugin and passing the args to it, meaning they won't use the new args filter.

What do you think, Jon?

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


2 years ago

#9 @audrasjb
2 years ago

  • Keywords changes-requested reporter-feedback added
  • Milestone changed from 6.1 to Future Release

As per today's bug scrub: A patch was submitted 2 months ago, and Tonya reviewed it 3 days ago. Looks like it will still need some discussion, and probably a new patch.

Moving to Future Release. It can be re-milestoned when we have a consensus on the implementation (and reporter feedback, if possible).

Note: See TracTickets for help on using tickets.