Cancel links do not always take you where you expect [#1931328]

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:

Go to the admin page for Pages/Blocks
Click the edit link for one of the search pages
Click the Save and Edit button
Click cancel

Comment	File	Size	Author
#9	1931328-9.patch	2.21 KB	Nick_vh
#9
#8	1931328-8.patch	2.22 KB	Nick_vh
#8
#5	1931328-4.patch	2.25 KB	Nick_vh
#5
#3	1931328-3.patch	1.98 KB	Nick_vh
#3
#1	1931328-1.patch	1.65 KB	Nick_vh
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Nick_vh

he/him

Ghent

CreditAttribution: Nick_vh commented 22 May 2013 at 23:51

Status:

Active

» Needs review

File	Size
1931328-1.patch	1.65 KB

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

Comment #2

kevin.dutra CreditAttribution: kevin.dutra commented 23 May 2013 at 15:06

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.

Comment #3

Nick_vh

he/him

Ghent

CreditAttribution: Nick_vh commented 23 May 2013 at 18:50

Status:

Needs work

» Needs review

File	Size
1931328-3.patch	1.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.

Comment #4

kevin.dutra CreditAttribution: kevin.dutra commented 23 May 2013 at 19:12

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.

Comment #5

Nick_vh

he/him

Ghent

CreditAttribution: Nick_vh commented 23 May 2013 at 19:36

File	Size
1931328-4.patch	2.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