Filter Reset for Modules and Permissions doesn't clear url search query

[yorkshire-pudding]

Description of the bug

This relates to the new functionality on the Permissions page (through #980) and the existing filter on the Modules page. In #5933, the instant reset was improved to clear the input without reloading the page. This works, however, if the search filter was added through a URL query (modules can pass url search to the pages or url query is added by pressing enter on field which some users may do), then this is not cleared when Reset is pressed.

Steps To Reproduce

To reproduce the behavior:

1. Go to admin/config/people/permissions?search=dashboard
2. Click on 'Reset'
3. Reload the page

Actual behavior

Page reloads with the search filter reapplied

Expected behavior

Page reloads with the search filter remaining cleared.

Additional information

Add any other information that could help, such as:

• Backdrop CMS version: 1.24.0
• Web server and its version: n/a
• Operating System and its version: n/a
• Browser(s) and their versions: n/a

---

PR

[yorkshire-pudding]

I've added a PR based on this method:
http://atodorov.org/blog/2013/01/28/remove-query-string-with-javascript-and-html5/

[klonos]

Thanks @yorkshire-pudding ...I initially left a few code suggestions (nit-picking inline comments + extra indentation that needed to be reduced), then another suggestion to reduce code by chaining .split() to the variable used, then finally I have discovered a "native" way to accomplish this, which I have tested and works fine. Would you like to update your PR to use that then?

Side-note: the code in this reset function is identical in both core/modules/system/js/modules.js and core/modules/user/js/user.permissions.js, so I'm wondering if this should be made a generic resetSearchQuery(e) function (name TBD) and moved into a single place where it can be shared/reused (in core/modules/system/js/system.admin.js perhaps?).

[argiepiano]

I'm not sure this is bug - I actually expect a page that contains a url query to behave this way. The field is re-populated with the filter when reloading. In fact, I would find it confusing if I reloaded the page that has a query in the url, for it not to use that query (is that what your PR does, @yorkshire-pudding ?)

[yorkshire-pudding]

@argiepiano - the search box is populated from the url query when the page loads; if you type a search query and press enter (which I can imagine many users doing intuitively - I know I have), then the address bar search query is added. The two are closely linked in every other way but this, and this PR closes the gap.

[yorkshire-pudding]

Side-note: the code in this reset function is identical in both core/modules/system/js/modules.js and core/modules/user/js/user.permissions.js, so I'm wondering if this should be made a generic resetSearchQuery(e) function (name TBD) and moved into a single place where it can be shared/reused (in core/modules/system/js/system.admin.js perhaps?).

I did note that they were essentially the same, but I followed where @quicksketch had originally put the functions rather than rip it up and start again. I think there is an argument for keeping it with the filter functions for coherence but I don't mind.

[klonos]

@yorkshire-pudding fair enough re keeping the code where it currently is - it would have to be a separate issue anyway.

@argiepiano this is a bug, but perhaps the steps to reproduce aren't as clear. If you are in a page that contains the query, then yes, refreshing the page should retain the query, but if you have previously clicked the "reset" button (which clears the search input field), then the expectation is that the search query is also cleared.

[klonos]

...this WFM and the code looks good, but since I was the one that suggested the main code change, I think that someone else should also review/test before marking this RTBC.

[quicksketch]

I'm also not 100% sure about this, but I think the change in behavior is fairly harmless.

Separately, I think there is an issue in the PR. This should use window.history() to remove the query string: https://github.com/backdrop/backdrop/pull/4323/files#r1097649944

[quicksketch]

I missed that the PR was updated. The new approach uses window.history() as I suggested and works great. This looks ready to go!