September 2005 - Development - lists.drupal.org

[drupal-devel] [bug] Tablesort and $_REQUEST array handling
by mathias 05 Sep '05

05 Sep '05

Issue status update for http://drupal.org/node/5371 Post a follow up: http://drupal.org/project/comments/add/5371 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: mathias Reported by: mathias Updated by: mathias Status: patch (ready to be committed) The two patches don't conflict as far as I can see. This patch introduces a function to convert arrays (such as the $edit array) to expanded querystrings. A byproduct of this function is you can now have tablesortable and paged resultsets after submitting a POST form. Project module and the ecommerce package are currently using hacks to circumvent this. mathias Previous comments: ------------------------------------------------------------------------ Fri, 23 Jan 2004 16:07:37 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays.patch (652 bytes) Tablesort tries its best to rebuild the URI from which it was called. The problem is when $_REQUEST holds arrays such as $edit. Tablesort will process this as: example.com?edit=Array This can be problematic since many module conditions rely on whether or not the $edit array is set. My patch disallows tablesort to append "array $_REQUEST variables" to it's new query string, since it serves no purpose. ------------------------------------------------------------------------ Sat, 31 Jan 2004 11:00:20 +0000 : Dries I'd like to hear Moshe's take on this as I'm not sure this is the proper fix. I'd think that the tablesort code knows exactly what fields it should add to the URL. ------------------------------------------------------------------------ Sat, 31 Jan 2004 13:24:36 +0000 : moshe weitzman I don't think this is the proper fix either. For example, - browse to the Drupal.org issue search page [1]. - press Search button - On the results page, look at the links in the table header and the pager. The query state gets passed along by tablesort. This patch would break this behavior. Changing Status to 'Active', since this patch shouldn't be applied without modification. [1] http://drupal.org/project/drupal/issues/search ------------------------------------------------------------------------ Sat, 31 Jan 2004 17:01:56 +0000 : mathias I'm pretty sure the project system is hacking the request object just like I am before sending it to tablesort: <?php if ($_SERVER['REQUEST_METHOD'] == 'POST' && is_array($_POST['edit'])) { foreach ($_POST['edit'] as $key => $value) { if (!empty($value) && in_array($key, $fields)) { $query->$key = $value; $_POST[$key] = is_array($value) ? implode(',', $value) : $value; } } unset($_POST['edit'], $_POST['op']); } ?> Basically, we're both moving the edit array into separate name / value pairs and then destroying it before handing it off to tablesort. I don't understand what the benefit is of passing an array into a GET query string since the key/value pairs aren't retained. We could set an option in tablesort to ask it to decompose the array into it's key/value pairs for us and pass them back in the query string. For example an edit array such as: <?php $edit['a'] => 1 $edit['b'] => 2 would have a query string of: &edit_a=1&edit_b=2 instead of the currently meaningless: &edit=Array ?> ------------------------------------------------------------------------ Sat, 31 Jan 2004 17:12:57 +0000 : Goba You mean: <?php $edit['a'] => 1 $edit['b'] => 2 would have a query string of: &edit[a]=1&edit[b]=2 instead of the currently meaningless: &edit=Array ?> This reuses the wonderful array handling provided by PHP (as long as nested arrays are handled properly). ------------------------------------------------------------------------ Sun, 01 Feb 2004 15:02:25 +0000 : Kjartan Indeed project module hacks the request data. I don't really like this as a general fix as it is a hack, but for now it is the best way. I dont think the original patch will break this as it just ignors arrays(). Project module loops through the array and converts them to normal array string elements; $_POST['edit']['status'] = value becomes $_POST['status'] = value, then I think I unset the edit array to clean up the urls. ------------------------------------------------------------------------ Thu, 05 Feb 2004 13:42:16 +0000 : moshe weitzman After feedback from Matias and Kjartan, I now consider this patch to be desireable ------------------------------------------------------------------------ Thu, 12 Aug 2004 15:10:30 +0000 : killes(a)www.drop.org Patch doesn't apply anymore. ------------------------------------------------------------------------ Fri, 08 Oct 2004 04:14:30 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_0.patch (2.55 KB) This patch resolves the hacks project module and the ecommerce package use to rebuild the $_REQUEST arrays when implementing paged resultsets and/or tablesorting. I know some other developers have ran into this bug as well usually after form submissions. Tablesort will now fully handle multi-dimensional $_REQUEST arrays and convert them to a valid querystring and preserve user-submitted data. ------------------------------------------------------------------------ Fri, 08 Oct 2004 04:34:12 +0000 : moshe weitzman very nicely done ... bonus points for converting pager to use the new array2uri() function ... tablesort's poor handling of arrays frustrated me in a recent client project. this will simplify almost all cases where a form post leads to a sortable "results" table, such as in project.module. ------------------------------------------------------------------------ Fri, 15 Oct 2004 14:01:42 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_1.patch (2.99 KB) Moved the new array2uri function to live with its array2* counterparts in common.inc and cleaned up the function documentation a wee bit. ------------------------------------------------------------------------ Tue, 21 Dec 2004 16:43:33 +0000 : moshe weitzman this is a nice code consolidation IMO ------------------------------------------------------------------------ Wed, 22 Dec 2004 10:50:33 +0000 : Dries This patch adds more code than it removes. That aside, it would be nice if the function's PHPdoc comments were extended. Looking at the patch, it is easy to see how/when this function is being used, but reading the PHPdoc comments it is not. I'd add a simple example and some context information. Also, it is unclear what $current_key is used for without inspecting the function's code. For readability, maybe rename $array to $attributes? If that makes sense, maybe rename 'array2uri' to 'attributes2uri'? Which makes me wonder: how does this stack with the existing drupal_attributes() function? Do we really need the recursive array handling? ------------------------------------------------------------------------ Sun, 13 Mar 2005 20:25:06 +0000 : killes(a)www.drop.org Doesn't apply anymore. ------------------------------------------------------------------------ Tue, 12 Jul 2005 22:49:07 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_2.patch (3.19 KB) Updating this patch since I've been bitten by this issue again. The new array2uri() function really shines for tablesorting and/or paging search results after a $_POST request. The only coding change involved for developers is to use $_REQUEST['edit'] and $_REQUEST['op'] instead of $_POST['edit'] and $_POST['op'] within their function callback where this functionality is desired. As an added benefit these pages could also be bookmarked. Dries, I've updated the documentation to hopefully makes things a little easier to understand. I don't think renaming the $array variable to $attributes or calling the function attributes2uri instead of array2uri would make the function and its purpose more transparent. It's not used for HTML attributes like the drupal_attributes function, but rather for form submitted data (e.g., the $edit array) that needs to be tablesorted/paged. Project module and the ecommerce package still currently hack the $_POST vars to circumvent this problem. Not sure if other modules are also doing this. ------------------------------------------------------------------------ Fri, 15 Jul 2005 16:25:46 +0000 : Steven If we're going to have an array2uri() function, we might as well add urlencode() calls in there... this would centralize a lot of existing urlencode() calls, I think. ------------------------------------------------------------------------ Mon, 18 Jul 2005 13:51:56 +0000 : Dries Not going to commit this yet. 1. Let's wait for Mathias' feedback on Steven's comment. 2. I suggest using query or query_string, not querystring. 3. It is not clear why this can't be part of drupal_attributes($array)? If there is an important difference, it would be nice to make that clear in the PHPdoc comments. ------------------------------------------------------------------------ Fri, 22 Jul 2005 03:18:19 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_3.patch (3.23 KB) Agreed with Steven that this is a logical place to urlencode values. The patch has been updated accordingly. There was some IRC discussion as to whether this function should be called drupal_uri, drupal2uri or just plain ol' array2uri. I think since this code doesn't make any Drupal specific function calls, array2uri is the best name. Which brings up a whole new issue for a whole new thread: Perhaps drupal_attributes should be renamed array2attributes? ------------------------------------------------------------------------ Mon, 25 Jul 2005 08:50:00 +0000 : Steven Dries: drupal_attributes is used for outputting attributes of an HTML tag. This patch is about putting data into the query (GET) part of an URL. They are completely different. mathias: couldn't we use this function in drupal_get_destination() as well ? ------------------------------------------------------------------------ Mon, 25 Jul 2005 08:56:05 +0000 : Steven By the way, what exactly is the point of this check: + if (!strstr($query_string, $key_as_uri)) { + $query_string .= '&'. $key_as_uri. '='. urlencode($value); + } It seems to be there to avoid duplicates, but there is no similar check below. If it is required, it needs a comment to explain it. ------------------------------------------------------------------------ Tue, 26 Jul 2005 17:02:50 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_4.patch (3.39 KB) drupal_get_destination() now uses array2uri(). I discovered that the use of strstr(), was to compensate for the function calling itself at times when it didn't need to. This bug is fixed and strstr() is removed. Good catch! ------------------------------------------------------------------------ Tue, 26 Jul 2005 18:57:26 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_5.patch (3.58 KB) Allow a string, NULL or array to be passed into array2uri(), making life easier on developers per Steven's feedback on IRC. ------------------------------------------------------------------------ Thu, 28 Jul 2005 18:15:44 +0000 : moshe weitzman thanks for updating this patch, matt. looks good to me. i did not test it yet though. ------------------------------------------------------------------------ Fri, 29 Jul 2005 18:40:25 +0000 : Steven Err, passing strings to /array/2uri() is pretty non-sensical and not at all what I meant. I'll see if I can make a better patch later :P. ------------------------------------------------------------------------ Sat, 13 Aug 2005 18:37:23 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_6.patch (4 KB) * Further code optimizations. * Made drupal_get_destination() make better use of array2uri(). ------------------------------------------------------------------------ Sat, 13 Aug 2005 18:53:04 +0000 : rbrooks00 +1 for this patch Having the built in ability to sort tabular data is incredibly useful. ------------------------------------------------------------------------ Mon, 05 Sep 2005 18:33:53 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_7.patch (4.02 KB) Syncing with HEAD and setting to commit-worthy as I've been using it in production mode for awhile now. ------------------------------------------------------------------------ Mon, 05 Sep 2005 19:35:27 +0000 : Bèr Kessels just curious: How does this patch break (the -) or interfere with the AJAX table sorting patch?

1 0

[drupal-devel] new CVS accounts
by Dries Buytaert 05 Sep '05

05 Sep '05

Hello world, we approved a bunch of CVS accounts. Please welcome our new contributors, and assist them as necessary. -- Dries Buytaert :: http://www.buytaert.net/

1 0

[drupal-devel] [bug] Tablesort and $_REQUEST array handling
by Bèr Kessels 05 Sep '05

05 Sep '05

Issue status update for http://drupal.org/node/5371 Post a follow up: http://drupal.org/project/comments/add/5371 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: mathias Reported by: mathias Updated by: Bèr Kessels Status: patch (ready to be committed) just curious: How does this patch break (the -) or interfere with the AJAX table sorting patch? Bèr Kessels Previous comments: ------------------------------------------------------------------------ Fri, 23 Jan 2004 16:07:37 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays.patch (652 bytes) Tablesort tries its best to rebuild the URI from which it was called. The problem is when $_REQUEST holds arrays such as $edit. Tablesort will process this as: example.com?edit=Array This can be problematic since many module conditions rely on whether or not the $edit array is set. My patch disallows tablesort to append "array $_REQUEST variables" to it's new query string, since it serves no purpose. ------------------------------------------------------------------------ Sat, 31 Jan 2004 11:00:20 +0000 : Dries I'd like to hear Moshe's take on this as I'm not sure this is the proper fix. I'd think that the tablesort code knows exactly what fields it should add to the URL. ------------------------------------------------------------------------ Sat, 31 Jan 2004 13:24:36 +0000 : moshe weitzman I don't think this is the proper fix either. For example, - browse to the Drupal.org issue search page [1]. - press Search button - On the results page, look at the links in the table header and the pager. The query state gets passed along by tablesort. This patch would break this behavior. Changing Status to 'Active', since this patch shouldn't be applied without modification. [1] http://drupal.org/project/drupal/issues/search ------------------------------------------------------------------------ Sat, 31 Jan 2004 17:01:56 +0000 : mathias I'm pretty sure the project system is hacking the request object just like I am before sending it to tablesort: <?php if ($_SERVER['REQUEST_METHOD'] == 'POST' && is_array($_POST['edit'])) { foreach ($_POST['edit'] as $key => $value) { if (!empty($value) && in_array($key, $fields)) { $query->$key = $value; $_POST[$key] = is_array($value) ? implode(',', $value) : $value; } } unset($_POST['edit'], $_POST['op']); } ?> Basically, we're both moving the edit array into separate name / value pairs and then destroying it before handing it off to tablesort. I don't understand what the benefit is of passing an array into a GET query string since the key/value pairs aren't retained. We could set an option in tablesort to ask it to decompose the array into it's key/value pairs for us and pass them back in the query string. For example an edit array such as: <?php $edit['a'] => 1 $edit['b'] => 2 would have a query string of: &edit_a=1&edit_b=2 instead of the currently meaningless: &edit=Array ?> ------------------------------------------------------------------------ Sat, 31 Jan 2004 17:12:57 +0000 : Goba You mean: <?php $edit['a'] => 1 $edit['b'] => 2 would have a query string of: &edit[a]=1&edit[b]=2 instead of the currently meaningless: &edit=Array ?> This reuses the wonderful array handling provided by PHP (as long as nested arrays are handled properly). ------------------------------------------------------------------------ Sun, 01 Feb 2004 15:02:25 +0000 : Kjartan Indeed project module hacks the request data. I don't really like this as a general fix as it is a hack, but for now it is the best way. I dont think the original patch will break this as it just ignors arrays(). Project module loops through the array and converts them to normal array string elements; $_POST['edit']['status'] = value becomes $_POST['status'] = value, then I think I unset the edit array to clean up the urls. ------------------------------------------------------------------------ Thu, 05 Feb 2004 13:42:16 +0000 : moshe weitzman After feedback from Matias and Kjartan, I now consider this patch to be desireable ------------------------------------------------------------------------ Thu, 12 Aug 2004 15:10:30 +0000 : killes(a)www.drop.org Patch doesn't apply anymore. ------------------------------------------------------------------------ Fri, 08 Oct 2004 04:14:30 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_0.patch (2.55 KB) This patch resolves the hacks project module and the ecommerce package use to rebuild the $_REQUEST arrays when implementing paged resultsets and/or tablesorting. I know some other developers have ran into this bug as well usually after form submissions. Tablesort will now fully handle multi-dimensional $_REQUEST arrays and convert them to a valid querystring and preserve user-submitted data. ------------------------------------------------------------------------ Fri, 08 Oct 2004 04:34:12 +0000 : moshe weitzman very nicely done ... bonus points for converting pager to use the new array2uri() function ... tablesort's poor handling of arrays frustrated me in a recent client project. this will simplify almost all cases where a form post leads to a sortable "results" table, such as in project.module. ------------------------------------------------------------------------ Fri, 15 Oct 2004 14:01:42 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_1.patch (2.99 KB) Moved the new array2uri function to live with its array2* counterparts in common.inc and cleaned up the function documentation a wee bit. ------------------------------------------------------------------------ Tue, 21 Dec 2004 16:43:33 +0000 : moshe weitzman this is a nice code consolidation IMO ------------------------------------------------------------------------ Wed, 22 Dec 2004 10:50:33 +0000 : Dries This patch adds more code than it removes. That aside, it would be nice if the function's PHPdoc comments were extended. Looking at the patch, it is easy to see how/when this function is being used, but reading the PHPdoc comments it is not. I'd add a simple example and some context information. Also, it is unclear what $current_key is used for without inspecting the function's code. For readability, maybe rename $array to $attributes? If that makes sense, maybe rename 'array2uri' to 'attributes2uri'? Which makes me wonder: how does this stack with the existing drupal_attributes() function? Do we really need the recursive array handling? ------------------------------------------------------------------------ Sun, 13 Mar 2005 20:25:06 +0000 : killes(a)www.drop.org Doesn't apply anymore. ------------------------------------------------------------------------ Tue, 12 Jul 2005 22:49:07 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_2.patch (3.19 KB) Updating this patch since I've been bitten by this issue again. The new array2uri() function really shines for tablesorting and/or paging search results after a $_POST request. The only coding change involved for developers is to use $_REQUEST['edit'] and $_REQUEST['op'] instead of $_POST['edit'] and $_POST['op'] within their function callback where this functionality is desired. As an added benefit these pages could also be bookmarked. Dries, I've updated the documentation to hopefully makes things a little easier to understand. I don't think renaming the $array variable to $attributes or calling the function attributes2uri instead of array2uri would make the function and its purpose more transparent. It's not used for HTML attributes like the drupal_attributes function, but rather for form submitted data (e.g., the $edit array) that needs to be tablesorted/paged. Project module and the ecommerce package still currently hack the $_POST vars to circumvent this problem. Not sure if other modules are also doing this. ------------------------------------------------------------------------ Fri, 15 Jul 2005 16:25:46 +0000 : Steven If we're going to have an array2uri() function, we might as well add urlencode() calls in there... this would centralize a lot of existing urlencode() calls, I think. ------------------------------------------------------------------------ Mon, 18 Jul 2005 13:51:56 +0000 : Dries Not going to commit this yet. 1. Let's wait for Mathias' feedback on Steven's comment. 2. I suggest using query or query_string, not querystring. 3. It is not clear why this can't be part of drupal_attributes($array)? If there is an important difference, it would be nice to make that clear in the PHPdoc comments. ------------------------------------------------------------------------ Fri, 22 Jul 2005 03:18:19 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_3.patch (3.23 KB) Agreed with Steven that this is a logical place to urlencode values. The patch has been updated accordingly. There was some IRC discussion as to whether this function should be called drupal_uri, drupal2uri or just plain ol' array2uri. I think since this code doesn't make any Drupal specific function calls, array2uri is the best name. Which brings up a whole new issue for a whole new thread: Perhaps drupal_attributes should be renamed array2attributes? ------------------------------------------------------------------------ Mon, 25 Jul 2005 08:50:00 +0000 : Steven Dries: drupal_attributes is used for outputting attributes of an HTML tag. This patch is about putting data into the query (GET) part of an URL. They are completely different. mathias: couldn't we use this function in drupal_get_destination() as well ? ------------------------------------------------------------------------ Mon, 25 Jul 2005 08:56:05 +0000 : Steven By the way, what exactly is the point of this check: + if (!strstr($query_string, $key_as_uri)) { + $query_string .= '&'. $key_as_uri. '='. urlencode($value); + } It seems to be there to avoid duplicates, but there is no similar check below. If it is required, it needs a comment to explain it. ------------------------------------------------------------------------ Tue, 26 Jul 2005 17:02:50 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_4.patch (3.39 KB) drupal_get_destination() now uses array2uri(). I discovered that the use of strstr(), was to compensate for the function calling itself at times when it didn't need to. This bug is fixed and strstr() is removed. Good catch! ------------------------------------------------------------------------ Tue, 26 Jul 2005 18:57:26 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_5.patch (3.58 KB) Allow a string, NULL or array to be passed into array2uri(), making life easier on developers per Steven's feedback on IRC. ------------------------------------------------------------------------ Thu, 28 Jul 2005 18:15:44 +0000 : moshe weitzman thanks for updating this patch, matt. looks good to me. i did not test it yet though. ------------------------------------------------------------------------ Fri, 29 Jul 2005 18:40:25 +0000 : Steven Err, passing strings to /array/2uri() is pretty non-sensical and not at all what I meant. I'll see if I can make a better patch later :P. ------------------------------------------------------------------------ Sat, 13 Aug 2005 18:37:23 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_6.patch (4 KB) * Further code optimizations. * Made drupal_get_destination() make better use of array2uri(). ------------------------------------------------------------------------ Sat, 13 Aug 2005 18:53:04 +0000 : rbrooks00 +1 for this patch Having the built in ability to sort tabular data is incredibly useful. ------------------------------------------------------------------------ Mon, 05 Sep 2005 18:33:53 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_7.patch (4.02 KB) Syncing with HEAD and setting to commit-worthy as I've been using it in production mode for awhile now.

1 0

[drupal-devel] [bug] Tablesort and $_REQUEST array handling
by mathias 05 Sep '05

05 Sep '05

Issue status update for http://drupal.org/node/5371 Post a follow up: http://drupal.org/project/comments/add/5371 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: mathias Reported by: mathias Updated by: mathias -Status: patch (code needs review) +Status: patch (ready to be committed) Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_7.patch (4.02 KB) Syncing with HEAD and setting to commit-worthy as I've been using it in production mode for awhile now. mathias Previous comments: ------------------------------------------------------------------------ Fri, 23 Jan 2004 16:07:37 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays.patch (652 bytes) Tablesort tries its best to rebuild the URI from which it was called. The problem is when $_REQUEST holds arrays such as $edit. Tablesort will process this as: example.com?edit=Array This can be problematic since many module conditions rely on whether or not the $edit array is set. My patch disallows tablesort to append "array $_REQUEST variables" to it's new query string, since it serves no purpose. ------------------------------------------------------------------------ Sat, 31 Jan 2004 11:00:20 +0000 : Dries I'd like to hear Moshe's take on this as I'm not sure this is the proper fix. I'd think that the tablesort code knows exactly what fields it should add to the URL. ------------------------------------------------------------------------ Sat, 31 Jan 2004 13:24:36 +0000 : moshe weitzman I don't think this is the proper fix either. For example, - browse to the Drupal.org issue search page [1]. - press Search button - On the results page, look at the links in the table header and the pager. The query state gets passed along by tablesort. This patch would break this behavior. Changing Status to 'Active', since this patch shouldn't be applied without modification. [1] http://drupal.org/project/drupal/issues/search ------------------------------------------------------------------------ Sat, 31 Jan 2004 17:01:56 +0000 : mathias I'm pretty sure the project system is hacking the request object just like I am before sending it to tablesort: <?php if ($_SERVER['REQUEST_METHOD'] == 'POST' && is_array($_POST['edit'])) { foreach ($_POST['edit'] as $key => $value) { if (!empty($value) && in_array($key, $fields)) { $query->$key = $value; $_POST[$key] = is_array($value) ? implode(',', $value) : $value; } } unset($_POST['edit'], $_POST['op']); } ?> Basically, we're both moving the edit array into separate name / value pairs and then destroying it before handing it off to tablesort. I don't understand what the benefit is of passing an array into a GET query string since the key/value pairs aren't retained. We could set an option in tablesort to ask it to decompose the array into it's key/value pairs for us and pass them back in the query string. For example an edit array such as: <?php $edit['a'] => 1 $edit['b'] => 2 would have a query string of: &edit_a=1&edit_b=2 instead of the currently meaningless: &edit=Array ?> ------------------------------------------------------------------------ Sat, 31 Jan 2004 17:12:57 +0000 : Goba You mean: <?php $edit['a'] => 1 $edit['b'] => 2 would have a query string of: &edit[a]=1&edit[b]=2 instead of the currently meaningless: &edit=Array ?> This reuses the wonderful array handling provided by PHP (as long as nested arrays are handled properly). ------------------------------------------------------------------------ Sun, 01 Feb 2004 15:02:25 +0000 : Kjartan Indeed project module hacks the request data. I don't really like this as a general fix as it is a hack, but for now it is the best way. I dont think the original patch will break this as it just ignors arrays(). Project module loops through the array and converts them to normal array string elements; $_POST['edit']['status'] = value becomes $_POST['status'] = value, then I think I unset the edit array to clean up the urls. ------------------------------------------------------------------------ Thu, 05 Feb 2004 13:42:16 +0000 : moshe weitzman After feedback from Matias and Kjartan, I now consider this patch to be desireable ------------------------------------------------------------------------ Thu, 12 Aug 2004 15:10:30 +0000 : killes(a)www.drop.org Patch doesn't apply anymore. ------------------------------------------------------------------------ Fri, 08 Oct 2004 04:14:30 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_0.patch (2.55 KB) This patch resolves the hacks project module and the ecommerce package use to rebuild the $_REQUEST arrays when implementing paged resultsets and/or tablesorting. I know some other developers have ran into this bug as well usually after form submissions. Tablesort will now fully handle multi-dimensional $_REQUEST arrays and convert them to a valid querystring and preserve user-submitted data. ------------------------------------------------------------------------ Fri, 08 Oct 2004 04:34:12 +0000 : moshe weitzman very nicely done ... bonus points for converting pager to use the new array2uri() function ... tablesort's poor handling of arrays frustrated me in a recent client project. this will simplify almost all cases where a form post leads to a sortable "results" table, such as in project.module. ------------------------------------------------------------------------ Fri, 15 Oct 2004 14:01:42 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_1.patch (2.99 KB) Moved the new array2uri function to live with its array2* counterparts in common.inc and cleaned up the function documentation a wee bit. ------------------------------------------------------------------------ Tue, 21 Dec 2004 16:43:33 +0000 : moshe weitzman this is a nice code consolidation IMO ------------------------------------------------------------------------ Wed, 22 Dec 2004 10:50:33 +0000 : Dries This patch adds more code than it removes. That aside, it would be nice if the function's PHPdoc comments were extended. Looking at the patch, it is easy to see how/when this function is being used, but reading the PHPdoc comments it is not. I'd add a simple example and some context information. Also, it is unclear what $current_key is used for without inspecting the function's code. For readability, maybe rename $array to $attributes? If that makes sense, maybe rename 'array2uri' to 'attributes2uri'? Which makes me wonder: how does this stack with the existing drupal_attributes() function? Do we really need the recursive array handling? ------------------------------------------------------------------------ Sun, 13 Mar 2005 20:25:06 +0000 : killes(a)www.drop.org Doesn't apply anymore. ------------------------------------------------------------------------ Tue, 12 Jul 2005 22:49:07 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_2.patch (3.19 KB) Updating this patch since I've been bitten by this issue again. The new array2uri() function really shines for tablesorting and/or paging search results after a $_POST request. The only coding change involved for developers is to use $_REQUEST['edit'] and $_REQUEST['op'] instead of $_POST['edit'] and $_POST['op'] within their function callback where this functionality is desired. As an added benefit these pages could also be bookmarked. Dries, I've updated the documentation to hopefully makes things a little easier to understand. I don't think renaming the $array variable to $attributes or calling the function attributes2uri instead of array2uri would make the function and its purpose more transparent. It's not used for HTML attributes like the drupal_attributes function, but rather for form submitted data (e.g., the $edit array) that needs to be tablesorted/paged. Project module and the ecommerce package still currently hack the $_POST vars to circumvent this problem. Not sure if other modules are also doing this. ------------------------------------------------------------------------ Fri, 15 Jul 2005 16:25:46 +0000 : Steven If we're going to have an array2uri() function, we might as well add urlencode() calls in there... this would centralize a lot of existing urlencode() calls, I think. ------------------------------------------------------------------------ Mon, 18 Jul 2005 13:51:56 +0000 : Dries Not going to commit this yet. 1. Let's wait for Mathias' feedback on Steven's comment. 2. I suggest using query or query_string, not querystring. 3. It is not clear why this can't be part of drupal_attributes($array)? If there is an important difference, it would be nice to make that clear in the PHPdoc comments. ------------------------------------------------------------------------ Fri, 22 Jul 2005 03:18:19 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_3.patch (3.23 KB) Agreed with Steven that this is a logical place to urlencode values. The patch has been updated accordingly. There was some IRC discussion as to whether this function should be called drupal_uri, drupal2uri or just plain ol' array2uri. I think since this code doesn't make any Drupal specific function calls, array2uri is the best name. Which brings up a whole new issue for a whole new thread: Perhaps drupal_attributes should be renamed array2attributes? ------------------------------------------------------------------------ Mon, 25 Jul 2005 08:50:00 +0000 : Steven Dries: drupal_attributes is used for outputting attributes of an HTML tag. This patch is about putting data into the query (GET) part of an URL. They are completely different. mathias: couldn't we use this function in drupal_get_destination() as well ? ------------------------------------------------------------------------ Mon, 25 Jul 2005 08:56:05 +0000 : Steven By the way, what exactly is the point of this check: + if (!strstr($query_string, $key_as_uri)) { + $query_string .= '&'. $key_as_uri. '='. urlencode($value); + } It seems to be there to avoid duplicates, but there is no similar check below. If it is required, it needs a comment to explain it. ------------------------------------------------------------------------ Tue, 26 Jul 2005 17:02:50 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_4.patch (3.39 KB) drupal_get_destination() now uses array2uri(). I discovered that the use of strstr(), was to compensate for the function calling itself at times when it didn't need to. This bug is fixed and strstr() is removed. Good catch! ------------------------------------------------------------------------ Tue, 26 Jul 2005 18:57:26 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_5.patch (3.58 KB) Allow a string, NULL or array to be passed into array2uri(), making life easier on developers per Steven's feedback on IRC. ------------------------------------------------------------------------ Thu, 28 Jul 2005 18:15:44 +0000 : moshe weitzman thanks for updating this patch, matt. looks good to me. i did not test it yet though. ------------------------------------------------------------------------ Fri, 29 Jul 2005 18:40:25 +0000 : Steven Err, passing strings to /array/2uri() is pretty non-sensical and not at all what I meant. I'll see if I can make a better patch later :P. ------------------------------------------------------------------------ Sat, 13 Aug 2005 18:37:23 +0000 : mathias Attachment: http://drupal.org/files/issues/tablesort_and_REQUEST_arrays_6.patch (4 KB) * Further code optimizations. * Made drupal_get_destination() make better use of array2uri(). ------------------------------------------------------------------------ Sat, 13 Aug 2005 18:53:04 +0000 : rbrooks00 +1 for this patch Having the built in ability to sort tabular data is incredibly useful.

1 0

[drupal-devel] [feature] Make user count in user block optional
by legoscia 05 Sep '05

05 Sep '05

Issue status update for http://drupal.org/node/29145 Post a follow up: http://drupal.org/project/comments/add/29145 Project: Drupal Version: 4.6.3 Component: user.module Category: feature requests Priority: minor Assigned to: Anonymous Reported by: legoscia Updated by: legoscia Status: patch (code needs review) Attachment: http://drupal.org/files/issues/drupal-user-count.patch (3.4 KB) The normal look of the user block was too big for my site, and the count of online users was not deemed important enough. Thus here is a patch that makes the display of the user count optional, leaving only the list of online users. legoscia

8 15

[drupal-devel] [task] Administrator configurable defaults and overriding variables.
by adrian 05 Sep '05

05 Sep '05

Issue status update for http://drupal.org/node/25745 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: adrian Reported by: adrian Updated by: adrian Status: patch Attachment: http://drupal.org/files/issues/variable_default.patch (0 bytes) When running a mass hosting environment, it is often required to shield users from the ability to break the sites hosted on your system and there are very often also good security reasons to remove the ability to configure certain variables. This patch does two things : 1) Introduces an optional sites/global.php file, that will be loaded after the sites/default/settings.php file and provide a set of global settings for all sites hosted on a server. This means the administrator only needs to configure the variables once. 2) Modifies variable_get and variable_set to check optional 'variable_default($name)' and 'variable_override($name)' functions, that an administrator can define in the above global.php file. There already exists the ability to set variables using $conf[varname] in your settings.php file, but there is no distinction made against variables that have been set this way, and normal variables, apart from the fact that trying to change them in the admin section fails silently. A good example of this would be the need to set up the files dir, and not allow it to be modified, as a user modifying the files dir to another site's files directory could be catastrophic. <?php function variable_override($name) { $conf['file_directory_path'] = conf_init() . '/files'; $conf['file_directory_temp'] = conf_init() . '/files/tmp'; $conf['file_downloads'] = 2; return $conf[$name]; } ?> These values will now be the only accepted and used values to the variables. If the user tries to modify them, they will revert to these values and there will be a message stating that the variable has been locked. Another common requirement is to provide sensible defaults for an install, but not take away the configurability of the option. A common example of this would be the theme setting. <?php function variable_default($name) { $conf['theme_default'] = 'chameleon'; return $conf[$name]; } ?> This will change the default that variable_get falls back on, from the one provided in the parameter list, to the administrator configured default. This is a much cleaner method of setting the defaults, than global search replacing the defaults provided in the code. This patch is one of the changes we have made to the drupal code base for our hosting platform @ bryght. adrian

3 7

[drupal-devel] [bug] Supernumerary messages "The menu item has been removed"
by Jose A Reyero 05 Sep '05

05 Sep '05

Issue status update for http://drupal.org/node/27669 Post a follow up: http://drupal.org/project/comments/add/27669 Project: Drupal Version: cvs Component: menu system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: wiz Updated by: Jose A Reyero Status: patch (code needs review) +1 Patch looks good Jose A Reyero Previous comments: ------------------------------------------------------------------------ Thu, 28 Jul 2005 11:07:22 +0000 : wiz Attachment: http://drupal.org/files/issues/node_delete_menu_msg.patch (583 bytes) When deleting a node, the function modules/menu.module:menu_node_form_delete() removes a custom menu entry for the node if it exists. The problem is that it will always print a message about a removed menu item, even though nothing has been actually deleted. Suggested fix: use db_affected_rows(). ------------------------------------------------------------------------ Sun, 21 Aug 2005 17:27:07 +0000 : killes(a)www.drop.org we use && not "and". ------------------------------------------------------------------------ Wed, 31 Aug 2005 21:27:23 +0000 : wiz Attachment: http://drupal.org/files/issues/node_delete_menu_msg_1.patch (582 bytes) ok I changed and to &&

1 0

[drupal-devel] [bug] Author information block seems to be broken
by robertDouglass 05 Sep '05

05 Sep '05

Issue status update for http://drupal.org/node/27948 Post a follow up: http://drupal.org/project/comments/add/27948 Project: Drupal Version: cvs Component: profile.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: robertDouglass Updated by: robertDouglass Status: patch (ready to be committed) performance aside, we can fix the bug, right? robertDouglass Previous comments: ------------------------------------------------------------------------ Mon, 01 Aug 2005 12:27:39 +0000 : robertDouglass This needs confirmation but I am unable to get fields to be shown in the author information block. The 'Link to full user profile' will work, but when I deselect 'Link to full user profile' from admin/block/configure/profile/0 and select the other fields instead, the block fails to materialize. ------------------------------------------------------------------------ Thu, 11 Aug 2005 23:08:32 +0000 : tostinni Attachment: http://drupal.org/files/issues/profile.module_2.patch (1.09 KB) This little patch does the trick. The bug came from the fact that the query retrieving user profiles, didn't take all public fields. ------------------------------------------------------------------------ Thu, 01 Sep 2005 18:57:21 +0000 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/profile_1.patch (1.12 KB) I've looked at the patch, changes "in" to "IN" and removed trailign DOS newlines, and can confirm that it works. ------------------------------------------------------------------------ Thu, 01 Sep 2005 18:58:15 +0000 : killes(a)www.drop.org changing status ------------------------------------------------------------------------ Fri, 02 Sep 2005 07:21:03 +0000 : Dries Is using IN() equivalent to using OR or is one faster than the other? ------------------------------------------------------------------------ Fri, 02 Sep 2005 12:47:56 +0000 : killes(a)www.drop.org Cvbge and I think that it is equivalent in this case. it is unsure whether it is faster, but IN () is usually regarded as being fast: http://mysql.com/in ------------------------------------------------------------------------ Fri, 02 Sep 2005 13:28:39 +0000 : tostinni I'm not a DB expert, but I got a little experience on Oracle (great you can answer ;) but I think it should happen pretty much the same), so IMVHO, I think that they're doing the same, in fact, looking at Oracle explain plan, it's exactly the same between a OR and a IN clause. They both do the same explain plan using "INLIST ITERATOR" in fact, the cost is the same, but explain plan tells me that it takes slightly less bytes to achieve it... Ok sorry if very off topic and useless, but I thought it worthes to mention that. Back to our query, I think whatever we put OR or IN clause, we shouldn't expect speed problem with this, as it's very little the change. If we do care about it, I think we should add an INDEX on this column. Also, I think IN clauses in this case (using 2 or more coma separated values) is much clearer than writing a lot of OR.

1 0

[drupal-devel] [bug] Empty variable in user module
by robertDouglass 05 Sep '05

05 Sep '05

Issue status update for http://drupal.org/node/30382 Post a follow up: http://drupal.org/project/comments/add/30382 Project: Drupal Version: cvs Component: user.module Category: bug reports Priority: minor Assigned to: Anonymous Reported by: robertDouglass Updated by: robertDouglass Status: patch (ready to be committed) Attachment: http://drupal.org/files/issues/user_pass.txt (696 bytes) $pass is an empty variable (not in scope) and I therefore replaced it with '' robertDouglass

1 0

[drupal-devel] [bug] File.inc parse error
by robertDouglass 05 Sep '05

05 Sep '05

Issue status update for http://drupal.org/node/30390 Post a follow up: http://drupal.org/project/comments/add/30390 Project: Drupal Version: cvs Component: file system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: robertDouglass Updated by: robertDouglass Status: patch (ready to be committed) Attachment: http://drupal.org/files/issues/file_inc_patch.txt (452 bytes) file.inc always registers a PHP parse error in my IDE because of a missing break in a switch statement. Here's a patch where the break is included. robertDouglass

1 0