Remove message parameters from admin URl's in the browser address bar

[mark-k]

Scenario: When a post is being published successfully "message=6" is appended to the URL. If I refresh the page while in that state I will get the "post published" message again, although nothing was done.

There is probably nothing functionally wrong with the way things work now but it can be esthetically more pleasant if that parameter was removed which in turn will eliminate the message problem.

This can be done on browsers that support the history API of HTML5. The following piece of code can be used after displaying the message or maybe it can be generalized to work in the admin footer.

<script type="text/javascript" charset="utf-8">
  url = the canonical URL for the address
  if (typeof history.replaceState === 'function') { // check html5 functionality support
    data = {dummy:true};
    history.replaceState(data,'',url);
  }
</script>

[johnbillion]

This seems like a sane idea. We should introduce a JavaScript version of remove_query_arg() to remove the unwanted query vars from the current URL. ​Here's a starting point for the default list of vars to remove.

Any takers?

[VarunAgw]

I have created a patch for this. I am unsure where to add this. I think maybe wp-admin/js/common.js but I am not sure. I have attached it.

[VarunAgw]

Code required for fix

[nacin]

Hi VarunAgw: common.js seems like a good place, yep!

[ilonaF]

The patch file does not apply anymore since the code around has change significantly since. Needs updating.

[wonderboymusic]

The list of query args could be output into JSON on the page that the script could read. That way: PHP and JS could read the same list of vars and future-proof themselves.

[morganestes]

Refreshed patch for wp-admin/js/common.js

[morganestes]

First run at unit tests for refresh patch.

[morganestes]

I refreshed the common.js.patch to bring it more inline with core standards and write a unit test for it.

This is my first unit test for core, so I'd love some feedback on it. I tested just a single instance of what the URL query stripper would encounter (params in the list removed, not in the list stay).

[wonderboymusic]

@morganestes: thanks! see: https://core.trac.wordpress.org/ticket/23367#comment:7

common.js should be localized using wp_localize_script() to read the query params to remove, instead of listing them in the file. This future-proofs the code when new query params are added.

location doesn't need to be wrapped in jQuery, just read window.location.href

I would use this as an opportunity to introduce some query string JS functions to read all of the params and parse them into a map - something that could possibly be added into wp-util.js. That way you could do:

params = get_query_params(); <----- not a good name
_.each( localizedParams, function (key) {
    delete params[ key ];
} );

Or use some Underscore function to get the difference between the 2 based on keys.

[morganestes]

Replying to wonderboymusic:

@morganestes: thanks! see: https://core.trac.wordpress.org/ticket/23367#comment:7

Sorry, read that last night and immediately forgot it when I started working on the code. I'll work on implementing it.

common.js should be localized using wp_localize_script() to read the query params to remove, instead of listing them in the file. This future-proofs the code when new query params are added.

What's the preferred approach for the JS object: create a new one or attach it to an existing one? If existing, which one is best for general use?

location doesn't need to be wrapped in jQuery, just read window.location.href

Yep, makes sense. I'll make the switch.

I would use this as an opportunity to introduce some query string JS functions to read all of the params and parse them into a map - something that could possibly be added into wp-util.js. That way you could do:

params = get_query_params(); <----- not a good name
_.each( localizedParams, function (key) {
    delete params[ key ];
} );

Or use some Underscore function to get the difference between the 2 based on keys.

I'll see what I can do. I'm imagining this being the utility function that's used to generate the JSON output mentioned above.

[morganestes]

Unified patch of working version.

[morganestes]

Here's the summary of what I came up with:

• Added utility functions to the wp.url namespace object to handle URL string manipulations and placed it in wp-utils.js.
• Localized the script to pass an array of parameters to remove from the query string.
• Created a filter for the array of parameters so they can be modified outside the main code.
• Added a global Object wpUrlParams that contains a map of the original parameters and their values prior to being filtered.

• Note that the actual URL filter relies on document.pushState, which isn't supported in IE 9 or lower without a polyfill. Those browsers will get access to wpUrlParams but not the URL changes, which is current behavior.

[morganestes]

Unified patch, without my test filter example. :/

[morganestes]

Refresh with formatting changes suggested by @drewapicture

[morganestes]

Related: #31233 .

[helen]

I take it from the patch that Underscore.js would now be loaded everywhere in the admin? I also find the name switching in wp_localize_script( 'common', 'removableQueryArgs', $removable_url_params ) to be a little odd, as a nitpick :)

[iseulde]

Maybe this is a stupid question, but why do we do a redirect with query args instead of looking at POST?

[morganestes]

Replying to helen:

I take it from the patch that Underscore.js would now be loaded everywhere in the admin?

I didn't realize it wasn't already. If not, should we be or should I be using jQuery since it already is?

I also find the name switching in wp_localize_script( 'common', 'removableQueryArgs', $removable_url_params ) to be a little odd, as a nitpick :)

Nitpicking is good here. I changed names a few times and forgot to settle on a single one. I'm thinking removableQueryArgs and $removable_query_args, but am open to suggestions if you have any particular leanings.

[morganestes]

Replying to iseulde:

Maybe this is a stupid question, but why do we do a redirect with query args instead of looking at POST?

I'm not sure I follow. We're not doing a redirect, just a location replacement if the browser supports it so we don't get hit with the same message on a refresh.

[iseulde]

I'm not sure I follow. We're not doing a redirect, just a location replacement if the browser supports it so we don't get hit with the same message on a refresh.

Server side. :)

[markjaquith]

Summarizing some feedback I gave @morganestes in Slack:

1. This approach sets a global wp-admin list of URL parameters that can't be used for anything other than transient message passing. If any plugin is using one of these for non-transient information (like selecting a plugin admin menu's sub-tab), a user refresh will result in that query parameter being gone, with unexpected results. It’s not the end of the world, but something to keep in mind.

2. We shouldn't be doing all this work in JavaScript. We have PHP functions for removing query args. We should just make a function that, in the <head> section, outputs two things:

<link id="wp-admin-canonical" rel="canonical" href="<?php echo wp_admin_canonical_url(); ?>" />

and

<script>if ( typeof history.replaceState === 'function' ) { history.replaceState( null, null, document.getElementById('wp-admin-canonical').href ); }</script>

And then wp_admin_canonical_url() would be something like this:

function wp_admin_canonical_url() {
    $parameters_to_remove = array(
    'message',
    'updated',
    'settings-updated',
    'saved',
    'activated',
    'activate',
    'deactivate',
    'locked',
    'skipped',
    'deleted',
    'trashed',
    'untrashed',
    );
    $parameters_to_remove = apply_filters( 'removable_url_params', $parameters_to_remove );
    return remove_query_arg( $parameters_to_remove );
}

Reasons:

1. Less code.
2. Not duplicating something in JS that we already have in PHP.
3. rel=canonical is a standard practice, and I think this is a good use of it.
4. No Underscore.js dependency on every admin screen. Indeed, this has no dependencies.

[morganestes]

Canonical URL filtered with PHP

[morganestes]

New patch with changes suggested by @markjaquith is attached with a couple of things to note:

• Since canonical links must not be relative, force an absolute URL with get_site_url() inside wp_admin_filtered_url(). Otherwise, remove_query_arg() returns a relative URL.
• Use jQuery.isFunction() to check for history.replaceState support as recommended by ​JS coding standards.

[morganestes]

Put it all into a single function since it's limited use.

[morganestes]

@markjaquith: A couple of changes introduced in Patch 5:

• roll the filter and the HTML/JS output into a single function since it's single-use, rather than the filter function standing alone and only being used to populate the canonical link.
• build the current URL manually, then pass that to remove_query_arg() (same process used elsewhere in core), rather than prepending get_site_url() to build the absolute canonical URL.
• ​after some discussion with @DrewAPicture, @azaozz, and @iseulde, switch to if ( window.history.replaceState ) {} for feature detection instead of jQuery.isFunction or doing a typeof check.

[johnbillion]

23367.5.diff​ looks solid to me. I agree that this can go into one function because of its narrow use.

The filter name should be changed to removable_query_args so it's more inline with the add|remove_query_arg() function names.

We can also add update, enabled, and disabled to the list of removable query args (​ref).

[morganestes]

Changes based on johnbillion's recommendations

[ocean90]

Needs a refresh per todays bug scrub.

[morganestes]

Adds conditional check for disabled filter/empty array of removable_query_args.

[morganestes]

@ocean90 I'm going to skip the equality check for current and filtered URLs since there's some issues with $_SERVER['REQUEST_URI'] being filtered elsewhere in core, so equality isn't a guarantee like we want it to be.

I've refreshed the patch with @helen's conditional check for an empty $removable_query_args, so the script tag isn't printed if the filter is effectively disabled.

[ocean90]

In 31736:

Administration: Remove single-use URL parameters and create canonical link based on new URL.

The default removable query args are 'message', 'settings-updated', 'saved', 'update', 'updated','activated', 'activate', 'deactivate', 'locked', 'deleted', 'trashed', 'untrashed', 'enabled', 'disabled', and 'skipped'.

props morganestes.
fixes #23367.

[dd32]

In 31882:

When altering the admin URL to reflect the canonical location, keep the existing hash (if present) in the URL.
Fixes #31758. See #23367

[helen]

In 31973:

Admin notices: Make (most) core notices dismissible.

These no longer return upon refreshing the page when JS is on and working, so users should be able to dismiss them. This is particularly important on the post edit screen when DFW is triggered, but pretty much all notices can be dismissed if needed. A post on Make/Core will follow with information on how this can be leveraged in plugins.

props valendesigns, afercia, paulwilde, adamsilverstein, helen.
fixes #31233. see #23367.

[helen]

Oops, meant to mention in the commit above that I added spammed and unspammed for comments.

[westonruter]

In 40313:

Customize: Prevent links to customize.php from being generated which have query vars from wp_removable_query_args() present.

Props dlh.
See #23367, #32692.
Fixes #31850.

[swissspidy]

In 40331:

Customize: Prevent links to customize.php from being generated which have query vars from wp_removable_query_args() present.

Props dlh.
See #23367, #32692.
Fixes #31850.

Merges [40313] to the 4.7 branch.

[dtbaker]

Just a note: this replaceState call on every page has a nice side effect. It prevents the browser re-submitting POST data on refresh/back button, which is really quite cool (especially dealing with poorly written plugins that do not redirect after submitting a form).

It took me so long to figure out why I could refresh after a POST and the browser would do a GET without prompting to resubmit post data. This only happened with JS enabled.

Details/examples here: ​https://stackoverflow.com/questions/45656405/browser-prevent-post-data-resubmit-on-refresh-using-only-javascript ​https://dtbaker.net/files/prevent-post-resubmit.php

Shweet!