The cancel links for both the search environment and search page settings forms use referer_uri() to generate the cancel link. This works fine in the happy-path case that the user gets to the form and clicks cancel.

However, in the case where you do a Save and Edit or when you try to save and a validation error occurs, the referrer is now the edit form.

For clarity:

  1. Go to the admin page for Pages/Blocks
  2. Click the edit link for one of the search pages
  3. Click the Save and Edit button
  4. Click cancel
CommentFileSizeAuthor
#9 1931328-9.patch2.21 KBnick_vh
#8 1931328-8.patch2.22 KBnick_vh
#5 1931328-4.patch2.25 KBnick_vh
#3 1931328-3.patch1.98 KBnick_vh
#1 1931328-1.patch1.65 KBnick_vh

Comments

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented
Status: Active » Needs review
StatusFileSize
new 1931328-1.patch1.65 KB

This should fix it. D6 is a little messy though.

kevin.dutra’s picture

kevin.dutra commented
Status: Needs review » Needs work

The logic makes sense, but in practice, I run into an issue with double URL encoding. As an example, after doing a Save and Edit on a search environment, the values end up being:

Referer: http://<domain>/admin/settings/apachesolr/settings/<env_id>/edit?destination=admin%2Fsettings%2Fapachesolr%2Fsettings
Request: http://<domain>/admin/settings/apachesolr/settings/<env_id>/edit%3Fdestination%3Dadmin%252Fsettings%252Fapachesolr%252Fsettings

This appears to be because url() encodes the whole $path argument. (It expects any query params to be passed in via the $options argument to protect the query portion from being double encoded.) Because of the double encoding, the strings don't evaluate to be equal.

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented
Status: Needs work » Needs review
StatusFileSize
new 1931328-3.patch1.98 KB

I see, so what about this? This logic is not perfect and will, if your referer_uri has query params, not function as expected but at least it solves the problem for most of the use cases.

If this passes the tests I'm thinking of committing this directly.

kevin.dutra’s picture

kevin.dutra commented

That definitely works in the case of search pages, but the edit links for search environments are constructed with the destination query param specified specified.

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented
StatusFileSize
new 1931328-4.patch2.25 KB

Seems that drupal does not respect its own API? request_uri with the argument TRUE should omit the query string. See http://api.drupal.org/api/drupal/includes!bootstrap.inc/function/request...

But in the case of the search environments it does keep the query params for some reason. We might need a regex to get rid of that. Attached is a patch that tries that

kevin.dutra’s picture

kevin.dutra commented

Ah, the argument for omitting the query string doesn't pop up until the D8 version of the function.

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented

Oh, must have overlooked that. Removing it from the patch

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented
StatusFileSize
new 1931328-8.patch2.22 KB

Should be good this time

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented
StatusFileSize
new 1931328-9.patch2.21 KB

Some spacing issue

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented
Status: Needs review » Fixed

Tested and committed. Thanks for the help

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.