Here is a patch I've been wanting to finish for a while. This patch assures that you end up on the proper page after you edit/delete a node, comment, user, or url alias. This is true no matter if you go through the usual interface or the admin interface. Further, if click the 'edit' link from 3rd page of a custom sorted view (e.g. admin/comment&from=100&sort=asc&order=Author) you still are returned to the right page.

The technique used here is generally available for module developers. I've minimally enhanced drupal_goto() so that it will redirect to the url specified in a 'destination' querystring parameter if such parameter exists. If it does not exist, we redirect just as today. No changes are required to existing drupal_goto() calls. A new helper function, drupal_get_destination() was added; it helps contruct the 'destination' string which is appended to add/edit links.

The only downside I can see to this patch is that a few URLs are less pretty than before. These urls are only shown to admins. This could only be avoided by having each admin page implement its own way of passing a destination, or stashing the destination in the $_SESSION. We recently tried storing referer in $_SESSION, and it was eventually removed because of poor coordination when a user has multiple browser windows open.

In addition to the above,
- I cleaned up some 'destination' handling in user login code
- I assured that after adding a new taxo term, we arrive back on the 'Add' page. That restores prior behavior

CommentFileSizeAuthor
#7 drdest_0.patch2.03 KBmoshe weitzman
drdest.patch11.05 KBmoshe weitzman
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Stefan Nagtegaal’s picture

This is another great improvement when we look at usability! Moshe, you did a terrific job on this..
After this patch is applied every submitted page drupal_goto()'s the page you expect it to go..

This is really one of the best patches i'd seen and tested lately, so ++ for this patch in HEAD..

Dries’s picture

The code looks good, the functionality is handy but I'd like to hear other people's thoughts on this.

chx’s picture

great one. +1

Bèr Kessels’s picture

It never really bothered /me/ that I was redirected to odd places, since I have a drupal-sitemap printed in my head ;). However, asking some clients, learned me that this patch would be greatly appreciated.

+1 from me.

One question though (not criticism!) why did you choose to do the testing inside druopal_goto as

if ($destination = $_REQUEST['destination'] ? $_REQUEST['destination'] : $_REQUEST['edit']['destination']) {

Seems odd to me to have a one-line-if inside another if.

Dries’s picture

Berkes: that line is a odd, indeed.

Actually, I'm not convinced that embedding this logic in drupal_goto() is appropriate. Personally, I'd rather have us write:

  drupal_goto($_REQUEST['destiation']);

I'd like to believe it is more transparant.

Dries’s picture

Committed to HEAD. Thanks.

moshe weitzman’s picture

FileSize
2.03 KB

Here is a patch for node.module since thta hunk failed last time. Note that I've removed the 'delete' link since it doesn't work and is mostly obsolete with the 'mass delete' feature.

I've also added smart destination andling to the 'url alias delete' feature.

Dries’s picture

Committed to HEAD.

Anonymous’s picture

You removed the "delete" link, but did not change the colspan for the "operations" header. We should probably remove the colspan attribute entirely.

Dries’s picture

Fixed in HEAD. Thanks.

asimmonds’s picture

With the operations column colspan=2 removed, it still leaves this at line 942


if (!$rows) {
$rows[] = array(array('data' => t('No posts available.'), 'colspan' => '7'));
}

which should really be colspan=6 as we have one less column now.

Dries’s picture

Fixed in HEAD.

Anonymous’s picture