Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: moshe weitzman Reported by: moshe weitzman Updated by: Dries Status: patch The code looks good, the functionality is handy but I'd like to hear other people's thoughts on this. Dries Previous comments: ------------------------------------------------------------------------ January 26, 2005 - 21:18 : moshe weitzman Attachment: http://drupal.org/files/issues/drdest.patch (11.05 KB) 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 ------------------------------------------------------------------------ January 26, 2005 - 23:44 : stefan nagtegaal 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.. -- View: http://drupal.org/node/16246 Edit: http://drupal.org/project/comments/add/16246

Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: moshe weitzman Reported by: moshe weitzman Updated by: Bèr Kessels Status: patch 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 Seems odd to me to have a one-line-if inside another if. Bèr Kessels Previous comments: ------------------------------------------------------------------------ January 26, 2005 - 20:18 : moshe weitzman Attachment: http://drupal.org/files/issues/drdest.patch (11.05 KB) 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 ------------------------------------------------------------------------ January 26, 2005 - 22:44 : stefan nagtegaal 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.. ------------------------------------------------------------------------ January 27, 2005 - 07:12 : Dries The code looks good, the functionality is handy but I'd like to hear other people's thoughts on this. ------------------------------------------------------------------------ January 27, 2005 - 08:19 : chx great one. +1 -- View: http://drupal.org/node/16246 Edit: http://drupal.org/project/comments/add/16246

Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: moshe weitzman Reported by: moshe weitzman Updated by: moshe weitzman -Status: fixed +Status: patch Attachment: http://drupal.org/files/issues/drdest_0.patch (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. moshe weitzman Previous comments: ------------------------------------------------------------------------ January 26, 2005 - 14:18 : moshe weitzman Attachment: http://drupal.org/files/issues/drdest.patch (11.05 KB) 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 ------------------------------------------------------------------------ January 26, 2005 - 16:44 : stefan nagtegaal 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.. ------------------------------------------------------------------------ January 27, 2005 - 01:12 : Dries The code looks good, the functionality is handy but I'd like to hear other people's thoughts on this. ------------------------------------------------------------------------ January 27, 2005 - 02:19 : chx great one. +1 ------------------------------------------------------------------------ January 27, 2005 - 09:05 : Bèr Kessels 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 <?php if ($destination = $_REQUEST['destination'] ? $_REQUEST['destination'] : $_REQUEST['edit']['destination']) { ?> Seems odd to me to have a one-line-if inside another if. ------------------------------------------------------------------------ January 27, 2005 - 09:18 : Dries 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: <?php drupal_goto($_REQUEST['destiation']); ?> I'd like to believe it is more transparant. ------------------------------------------------------------------------ February 1, 2005 - 14:46 : Dries Committed to HEAD. Thanks. -- View: http://drupal.org/node/16246 Edit: http://drupal.org/project/comments/add/16246

Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: moshe weitzman Reported by: moshe weitzman Updated by: asimmonds -Status: fixed +Status: patch 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. asimmonds Previous comments: ------------------------------------------------------------------------ January 27, 2005 - 08:18 : moshe weitzman Attachment: http://drupal.org/files/issues/drdest.patch (11.05 KB) 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 ------------------------------------------------------------------------ January 27, 2005 - 10:44 : stefan nagtegaal 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.. ------------------------------------------------------------------------ January 27, 2005 - 19:12 : Dries The code looks good, the functionality is handy but I'd like to hear other people's thoughts on this. ------------------------------------------------------------------------ January 27, 2005 - 20:19 : chx great one. +1 ------------------------------------------------------------------------ January 28, 2005 - 03:05 : Bèr Kessels 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 <?php if ($destination = $_REQUEST['destination'] ? $_REQUEST['destination'] : $_REQUEST['edit']['destination']) { ?> Seems odd to me to have a one-line-if inside another if. ------------------------------------------------------------------------ January 28, 2005 - 03:18 : Dries 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: <?php drupal_goto($_REQUEST['destiation']); ?> I'd like to believe it is more transparant. ------------------------------------------------------------------------ February 2, 2005 - 08:46 : Dries Committed to HEAD. Thanks. ------------------------------------------------------------------------ February 10, 2005 - 07:11 : moshe weitzman Attachment: http://drupal.org/files/issues/drdest_0.patch (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. ------------------------------------------------------------------------ February 11, 2005 - 08:30 : Dries Committed to HEAD. ------------------------------------------------------------------------ February 11, 2005 - 17:08 : Anonymous You removed the "delete" link, but did not change the colspan for the "operations" header. We should probably remove the colspan attribute entirely. ------------------------------------------------------------------------ February 12, 2005 - 08:10 : Dries Fixed in HEAD. Thanks. -- View: http://drupal.org/node/16246 Edit: http://drupal.org/project/comments/add/16246