June 2005 - Development - lists.drupal.org

[drupal-devel] [bug] PROFILE_HIDDEN updates
by pyromanfo 20 Jun '05

20 Jun '05

Issue status update for http://drupal.org/node/21219 Project: Drupal Version: cvs Component: profile.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: pyromanfo Updated by: pyromanfo Status: patch Attachment: http://drupal.org/files/issues/profile_hidden_update.patch (3.28 KB) Here are some updates to the PROFILE_HIDDEN functionality. Basically now if you have 'administer users' access, you can edit these fields and they show up for you in the user's profile when editing. If you don't then they don't show up for editing at all. They still should show up when simply viewing though. pyromanfo

9 12

[drupal-devel] [bug] Theming tablesort icons is inconsistent
by Bèr Kessels 20 Jun '05

20 Jun '05

Issue status update for http://drupal.org/node/11927 Project: Drupal Version: cvs Component: theme system Category: bug reports Priority: normal Assigned to: Robin Monks Reported by: stefan nagtegaal Updated by: Bèr Kessels Status: patch I am not sure about teh switch; AFAIK there will be only asc and desc. So an if would be enough. Saves some lines of code, like "break"s; etc. Bèr Kessels Previous comments: ------------------------------------------------------------------------ October 22, 2004 - 17:56 : stefan nagtegaal Attachment: http://drupal.org/files/issues/tablesort-sorting-theme-improvement.patch (1.25 KB) Attached is a patch for tablesort.inc much cleaner, friendlier and more consistent way to theme the tablesort icons. Marking as a bug becuase it is inconsistent with the rest of drupal.. Please review and apply! ------------------------------------------------------------------------ October 22, 2004 - 17:57 : stefan nagtegaal Setting status to 'Patch' instead of 'Fixed' so it shows up in the patch queue ------------------------------------------------------------------------ October 23, 2004 - 17:27 : Dries Please move the proposed theme functions to includes/theme.inc (so Doxygen includes them in the list of theme_ functions). ------------------------------------------------------------------------ October 23, 2004 - 21:39 : Steven We should avoid entities like   in the code. This can be achieved through CSS instead in the theme. ------------------------------------------------------------------------ October 23, 2004 - 21:41 : Steven (ack submitted too early) Also, I wonder about the granularity of these theme functions. I don't see the point in putting ascending/descending in a separate function: they are different styles for the same element (the tablesort indicator). ------------------------------------------------------------------------ October 24, 2004 - 01:08 : Dries Stefan's patch actually removes the entities, not (or is my jet lag worse than it appears)? ------------------------------------------------------------------------ October 24, 2004 - 11:56 : stefan nagtegaal Attachment: http://drupal.org/files/issues/tablesort-sorting-theme-improvement_0.patch (1.73 KB) Updated patch which removes the and moves the theme_tablesort_asc() and theme_tablesort_desc() to theme.inc. This is much better imo than the current approach.. ------------------------------------------------------------------------ October 24, 2004 - 16:10 : Steven Yeah the entity was actually a +1... I should stop doing so much things at the same time. My other concern is still valid though: theme functions should theme page /elements/, not versions of that element. Otherwise we would have theme_node(), theme_node_sticky(), theme_node_teaser(), ... ------------------------------------------------------------------------ November 15, 2004 - 13:25 : Dries I agree with Steven. We should have a function called theme_tablesort_header($style) where $style can be either 'ascending' or 'descending'. ------------------------------------------------------------------------ June 20, 2005 - 13:43 : Robin Monks Attachment: http://drupal.org/files/issues/tablesort_header.patch (1.49 KB) Here is a new patch with theme_tablesort_header($style) that takes either asc or desc as arguments. Tested to work on Drupal CVS HEAD. Robin

1 0

[drupal-devel] [bug] Theming tablesort icons is inconsistent
by Robin Monks 20 Jun '05

20 Jun '05

Issue status update for http://drupal.org/node/11927 Project: Drupal -Version: 4.5.0 +Version: cvs Component: theme system Category: bug reports Priority: normal -Assigned to: stefan nagtegaal +Assigned to: Robin Monks Reported by: stefan nagtegaal Updated by: Robin Monks -Status: active +Status: patch Attachment: http://drupal.org/files/issues/tablesort_header.patch (1.49 KB) Here is a new patch with theme_tablesort_header($style) that takes either asc or desc as arguments. Tested to work on Drupal CVS HEAD. Robin Robin Monks Previous comments: ------------------------------------------------------------------------ October 22, 2004 - 16:56 : stefan nagtegaal Attachment: http://drupal.org/files/issues/tablesort-sorting-theme-improvement.patch (1.25 KB) Attached is a patch for tablesort.inc much cleaner, friendlier and more consistent way to theme the tablesort icons. Marking as a bug becuase it is inconsistent with the rest of drupal.. Please review and apply! ------------------------------------------------------------------------ October 22, 2004 - 16:57 : stefan nagtegaal Setting status to 'Patch' instead of 'Fixed' so it shows up in the patch queue ------------------------------------------------------------------------ October 23, 2004 - 16:27 : Dries Please move the proposed theme functions to includes/theme.inc (so Doxygen includes them in the list of theme_ functions). ------------------------------------------------------------------------ October 23, 2004 - 20:39 : Steven We should avoid entities like   in the code. This can be achieved through CSS instead in the theme. ------------------------------------------------------------------------ October 23, 2004 - 20:41 : Steven (ack submitted too early) Also, I wonder about the granularity of these theme functions. I don't see the point in putting ascending/descending in a separate function: they are different styles for the same element (the tablesort indicator). ------------------------------------------------------------------------ October 24, 2004 - 00:08 : Dries Stefan's patch actually removes the entities, not (or is my jet lag worse than it appears)? ------------------------------------------------------------------------ October 24, 2004 - 10:56 : stefan nagtegaal Attachment: http://drupal.org/files/issues/tablesort-sorting-theme-improvement_0.patch (1.73 KB) Updated patch which removes the and moves the theme_tablesort_asc() and theme_tablesort_desc() to theme.inc. This is much better imo than the current approach.. ------------------------------------------------------------------------ October 24, 2004 - 15:10 : Steven Yeah the entity was actually a +1... I should stop doing so much things at the same time. My other concern is still valid though: theme functions should theme page /elements/, not versions of that element. Otherwise we would have theme_node(), theme_node_sticky(), theme_node_teaser(), ... ------------------------------------------------------------------------ November 15, 2004 - 12:25 : Dries I agree with Steven. We should have a function called theme_tablesort_header($style) where $style can be either 'ascending' or 'descending'.

1 0

[drupal-devel] [bug] Improved internal/external referrer differentiation
by Robin Monks 20 Jun '05

20 Jun '05

Issue status update for http://drupal.org/node/9508 Project: Drupal Version: cvs Component: statistics.module Category: bug reports Priority: normal Assigned to: Robin Monks Reported by: njivy Updated by: Robin Monks Status: patch Attachment: http://drupal.org/files/issues/parse_url.diff (1.7 KB) Here is a patch using parse_url. Robin Robin Monks Previous comments: ------------------------------------------------------------------------ July 25, 2004 - 17:32 : njivy Attachment: http://drupal.org/files/issues/statistics.http_host.patch (2.34 KB) Some PHP setups return $_SERVER['HTTP_HOST'] with a port designation -- e.g. http://domain.tld:80. With this patch, statistics.module removes the port designation before distinguishing between internal and external referrers. Without this patch, all referrers will be labeled as external in PHP setups where the port designation is included in HTTP_HOST. ------------------------------------------------------------------------ July 25, 2004 - 18:43 : Anonymous I have provided a patch before which also enabled stats module to treat more domain names as local ones, since a site might be accessible via multiple domain names (like www.drupal.org and drupal.org) That was not accepted. It is not just port numbers that are problems. ------------------------------------------------------------------------ October 10, 2004 - 18:24 : killes(a)www.drop.org Doesn't apply anymore- ------------------------------------------------------------------------ June 13, 2005 - 23:22 : Robin Monks Attachment: http://drupal.org/files/issues/incorrect.external.fix.patch (1.73 KB) Updated patch to work with latest HEAD. Tested to work with CVS HEAD. I'll take over any further problems that this patch may incur. Robin ------------------------------------------------------------------------ June 19, 2005 - 20:28 : Dries Not sure but it might be shorter/better to use PHP's parse_url()?

1 0

[drupal-devel] [bug] Javascript caching of autocomplete results broken
by Thox 20 Jun '05

20 Jun '05

Issue status update for http://drupal.org/node/25354 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: Thox Reported by: Thox Updated by: Thox Status: patch Attachment: http://drupal.org/files/issues/autocomplete.js_1.patch (792 bytes) Closer inspection of the autocomplete code showed that javascript caching isn't working. This patch fixes it. Improvements: faster response and less server traffic for matches previously found. Thox

1 0

[drupal-devel] [bug] user_access returns a string not a boolean as specified
by deekayen 20 Jun '05

20 Jun '05

Issue status update for http://drupal.org/node/21566 Project: Drupal Version: 4.6.0 Component: user system Category: bug reports Priority: minor Assigned to: deekayen Reported by: gjf Updated by: deekayen Status: patch Actually, there's a bug in the benchmark code that causes $result to not populate. Switch the following: for($j=0; $j>10; $j+=1) { to for($j=0; $j<10; $j+=1) { Then the string search is more like 8.1727991104126 seconds verses like 0.46953892707825 for the array search, an even bigger difference. deekayen Previous comments: ------------------------------------------------------------------------ April 29, 2005 - 00:53 : gjf user_access() function returns a string not a boolean. This caused memory consumption problems for us due to a bug elsewhere in our code. i.e. return strstr($perm[$account->uid], "$string, "); should probably be if ( strstr($perm[$account->uid], "$string, ")) { return true; } else { return false; } ------------------------------------------------------------------------ June 20, 2005 - 02:35 : deekayen Attachment: http://drupal.org/files/issues/user_access.returnbooleans.diff (1.28 KB) Attached patch has a spelling correction and makes user_access() return only booleans. Diff'ed against HEAD. ------------------------------------------------------------------------ June 20, 2005 - 02:55 : deekayen Attachment: http://drupal.org/files/issues/user_access.returnbooleans_0.diff (1.59 KB) It bothered me to have a string search instead of an array search for permissions. This patch does what the previous patch does, but it uses in_array() on the static $perm instead of strstr() on a string. ------------------------------------------------------------------------ June 20, 2005 - 03:02 : killes(a)www.drop.org Can you please provide a benchmark test of the strstr approach vs the in_array one? I am concerned that it might be slower. user_access gets called quite often. ------------------------------------------------------------------------ June 20, 2005 - 03:47 : deekayen That's a fair request. The output of the following code on my WinXP, PHP5.1-dev, 2.133 Ghz Athlon is: 0.29749894142151 0.29712684249878 1.0804200172424 0.56814384460449 0.10924220085144 0.12113380432129 0.13284420967102 <?php function utime($a = '') { static $utime; $time = explode(' ', microtime()); $usec = (double)$time[0]; $sec = (double)$time[1]; switch($a) { case 'bm': $to_return = $usec + $sec - $utime; break; default: $utime = $usec + $sec; return $utime; break; } $utime = $usec + $sec; return $to_return; } $account->uid = 1; utime(); for($i=0; $i<500000; $i+=1) { if ($account->uid == 1) { // blah } } echo utime('bm') .' '; for($i=0; $i<500000; $i+=1) { if ((int)$account->uid === 1) { // blah } } echo utime('bm') .' '; for($i=0; $i<50; $i+=1) { $result[$i] = ''; for($j=0; $j>10; $j+=1) { $result[$i] .= chr(rand(65-126)); } } $perm[$account->uid] = ''; utime(); for($i=0; $i<2000; $i+=1) { foreach($result as $row) { $perm[$account->uid] .= "$row, "; } isset($perm[$account->uid]) && strstr($perm[$account->uid], "{$result['45']}, "); } echo utime('bm') .' '; $perm[$account->uid] = array(); utime(); for($i=0; $i<2000; $i+=1) { foreach($result as $row) { $perm[$account->uid][] = $row; } isset($perm[$account->uid]) && in_array($result['45'], $perm[$account->uid]); } echo utime('bm') .' '; for($i=0; $i<500000; $i+=1) { // blah } echo utime('bm') .' '; for($i=0; $i<500000; $i++) { // blah } echo utime('bm') .' '; for($i=0; $i<500000; $i = $i+1) { // blah } echo utime('bm') .' '; ?>

1 0

[drupal-devel] [bug] user_access returns a string not a boolean as specified
by deekayen 20 Jun '05

20 Jun '05

Issue status update for http://drupal.org/node/21566 Project: Drupal Version: 4.6.0 Component: user system Category: bug reports Priority: minor -Assigned to: Anonymous +Assigned to: deekayen Reported by: gjf Updated by: deekayen -Status: active +Status: patch That's a fair request. The output of the following code on my WinXP, PHP5.1-dev, 2.133 Ghz Athlon is: 0.29749894142151 0.29712684249878 1.0804200172424 0.56814384460449 0.10924220085144 0.12113380432129 0.13284420967102 <?php function utime($a = '') { static $utime; $time = explode(' ', microtime()); $usec = (double)$time[0]; $sec = (double)$time[1]; switch($a) { case 'bm': $to_return = $usec + $sec - $utime; break; default: $utime = $usec + $sec; return $utime; break; } $utime = $usec + $sec; return $to_return; } $account->uid = 1; utime(); for($i=0; $i<500000; $i+=1) { if ($account->uid == 1) { // blah } } echo utime('bm') .' '; for($i=0; $i<500000; $i+=1) { if ((int)$account->uid === 1) { // blah } } echo utime('bm') .' '; for($i=0; $i<50; $i+=1) { $result[$i] = ''; for($j=0; $j>10; $j+=1) { $result[$i] .= chr(rand(65-126)); } } $perm[$account->uid] = ''; utime(); for($i=0; $i<2000; $i+=1) { foreach($result as $row) { $perm[$account->uid] .= "$row, "; } isset($perm[$account->uid]) && strstr($perm[$account->uid], "{$result['45']}, "); } echo utime('bm') .' '; $perm[$account->uid] = array(); utime(); for($i=0; $i<2000; $i+=1) { foreach($result as $row) { $perm[$account->uid][] = $row; } isset($perm[$account->uid]) && in_array($result['45'], $perm[$account->uid]); } echo utime('bm') .' '; for($i=0; $i<500000; $i+=1) { // blah } echo utime('bm') .' '; for($i=0; $i<500000; $i++) { // blah } echo utime('bm') .' '; for($i=0; $i<500000; $i = $i+1) { // blah } echo utime('bm') .' '; ?> deekayen Previous comments: ------------------------------------------------------------------------ April 29, 2005 - 00:53 : gjf user_access() function returns a string not a boolean. This caused memory consumption problems for us due to a bug elsewhere in our code. i.e. return strstr($perm[$account->uid], "$string, "); should probably be if ( strstr($perm[$account->uid], "$string, ")) { return true; } else { return false; } ------------------------------------------------------------------------ June 20, 2005 - 02:35 : deekayen Attachment: http://drupal.org/files/issues/user_access.returnbooleans.diff (1.28 KB) Attached patch has a spelling correction and makes user_access() return only booleans. Diff'ed against HEAD. ------------------------------------------------------------------------ June 20, 2005 - 02:55 : deekayen Attachment: http://drupal.org/files/issues/user_access.returnbooleans_0.diff (1.59 KB) It bothered me to have a string search instead of an array search for permissions. This patch does what the previous patch does, but it uses in_array() on the static $perm instead of strstr() on a string. ------------------------------------------------------------------------ June 20, 2005 - 03:02 : killes(a)www.drop.org Can you please provide a benchmark test of the strstr approach vs the in_array one? I am concerned that it might be slower. user_access gets called quite often.

1 0

[drupal-devel] question re maps in drupal (fwd)
by Anselm Hook 20 Jun '05

20 Jun '05

---------- Forwarded message ---------- Date: Fri, 17 Jun 2005 13:57:41 -0700 (MST) From: Anselm Hook <anselm(a)hook.org> To: civicspace-dev(a)civicspacelabs.org Subject: question re maps in drupal I have some questions about approach... Here is some work we are doing on supporting maps in drupal / civicspace: http://ghost.thingster.org/cs//?q=node/24 It is based on this piece of work that we published a couple of days ago: http://maps.civicactions.net The approach I am taking is to minimize the weight of the mapping layer inside of drupal itself and to rely entirely on a rich client-side engine written in javascript. The engine talks to any conventional WMS server and is optimized for tile based access for performance. This relies heavily on the idea of drupal publishing RSS feeds where nodes are decorated with location information in the form of geo:long and geo:lat. The location module is also something I am relying on to do this. Not everything in drupal is published as XML however. You could not right now easily build a Javascript version of a drupal site for example ( I think ). Is there an AJAX plan in the works and any time frame for that? In particular I'd like to build a small 'filter users by location' XML exporter - but there are other things beyond that one example. Should I hack this myself, wait, do something comprehensive etc? Basically there is a tipping point between 'do this one little hack' and 'build out an AJAX API'... I'm not on top of what the drupal / civicspace plan is however and any input is welcome. - a

5 7

[drupal-devel] [bug] Wrong Login Error
by deekayen 20 Jun '05

20 Jun '05

Issue status update for http://drupal.org/node/3962 Project: Drupal Version: cvs Component: user system Category: bug reports Priority: minor Assigned to: deekayen Reported by: dziemecki Updated by: deekayen Status: patch Attachment: http://drupal.org/files/issues/blockedusermessage_0.diff (3.14 KB) This patch uses "deactivated" instead of "blocked", but is otherwise not different from the previous patch. deekayen Previous comments: ------------------------------------------------------------------------ November 5, 2003 - 21:01 : dziemecki When a new user logs on after they're ID and password were sent to them, but before their account was "unblocked", they get the follwoing error: "Sorry. Unrecognized username or password. Have you forgotten your password?" That may or may not count as a bug, but it not entirely accurate, and might be a little confusing. ------------------------------------------------------------------------ June 14, 2005 - 03:19 : deekayen I don't mind assigning this one to myself, but I'm not familiar enough with the function of the access table to complete the patch. Should blocked users be entered into the access table? Right now, user.module has a drupal_deny check with the correct error message, but it doesn't get displayed because the only place a person gets blocked is in the users table. To be more normalized, I'd just leave the block in the users table, but does someone intend to move the user block to the access table? ------------------------------------------------------------------------ June 15, 2005 - 21:37 : deekayen Attachment: http://drupal.org/files/issues/blockedloginmessage.patch (1.26 KB) Right now when users are blocked, they're not added to the access table, so this patch just checks the users table for blocked logins. The check in user.module's user_login() should work now and give the correct error message that was there all along. ------------------------------------------------------------------------ June 15, 2005 - 21:45 : deekayen Attachment: http://drupal.org/files/issues/removeactivereset.patch (882 bytes) This patch goes with the previous one. Without it, when you edit a blocked user to make them active again, they remain blocked and the edit screen shows an error message about the user being blocked. I don't really know if that's the intended behavior to have blocks be permanent, but permanent blocks don't seem right to me. ------------------------------------------------------------------------ June 16, 2005 - 09:04 : moshe weitzman I'm not so sure about this one. We should continue to distinguish between a 'user block' and a 'host ban'. ------------------------------------------------------------------------ June 16, 2005 - 09:12 : killes(a)www.drop.org I agree with Moshe. I think we should test whether a user hasn't logged in before and then give a different message then when he got blocked later. ------------------------------------------------------------------------ June 17, 2005 - 00:37 : deekayen Attachment: http://drupal.org/files/issues/blockeduser.diff (1.29 KB) Just your comments made me see where I went wrong with the previous patches. I'd never used the access control rules before, so I thought blocked user == access controlled username. As far as I could tell, the only place to where a blocked username check was needed is at login. This patch adds a user_blocked() to just user.module and has a separate blocked error message from the access check. ------------------------------------------------------------------------ June 17, 2005 - 00:46 : deekayen Attachment: http://drupal.org/files/issues/blockeduser_0.diff (1.29 KB) I'm not used to the Drupal bracket formatting, yet. This is a spacing revision. ------------------------------------------------------------------------ June 19, 2005 - 02:28 : Dries I think this patch makes sense, however the difference between 'the name X has been denied access' and 'the name X has been blocked' is too subtle for the average Drupal user/administrator to grasp. I'd like to have us think about rewording the error messages so they are somewhat more descriptive to administrators without exposing internal/technical detail to the user. How about writing 'the name X is a reserved user name' for the former, and 'the user with name X is no longer allowed access' for the latter? (One could argue we need the drupal_deny() check at login time.) ------------------------------------------------------------------------ June 19, 2005 - 14:48 : deekayen Attachment: http://drupal.org/files/issues/blockedusermessage.diff (1.5 KB) What about "The username %s has been blocked." and "The name %s is a reserved username."? I think it's important that the user administration "blocked" status matches the error message. Otherwise perhaps the user administration "blocked" status should be changed to "Deactivated" or "Denied" with "The username %s has been deactivated." as the error message. The attached patch is revised to have the first two messages.

1 0

[drupal-devel] [bug] Wrong Login Error
by deekayen 19 Jun '05

19 Jun '05

Issue status update for http://drupal.org/node/3962 Project: Drupal Version: cvs Component: user system Category: bug reports Priority: minor Assigned to: deekayen Reported by: dziemecki Updated by: deekayen Status: patch Attachment: http://drupal.org/files/issues/blockedusermessage.diff (1.5 KB) What about "The username %s has been blocked." and "The name %s is a reserved username."? I think it's important that the user administration "blocked" status matches the error message. Otherwise perhaps the user administration "blocked" status should be changed to "Deactivated" or "Denied" with "The username %s has been deactivated." as the error message. The attached patch is revised to have the first two messages. deekayen Previous comments: ------------------------------------------------------------------------ November 5, 2003 - 21:01 : dziemecki When a new user logs on after they're ID and password were sent to them, but before their account was "unblocked", they get the follwoing error: "Sorry. Unrecognized username or password. Have you forgotten your password?" That may or may not count as a bug, but it not entirely accurate, and might be a little confusing. ------------------------------------------------------------------------ June 14, 2005 - 03:19 : deekayen I don't mind assigning this one to myself, but I'm not familiar enough with the function of the access table to complete the patch. Should blocked users be entered into the access table? Right now, user.module has a drupal_deny check with the correct error message, but it doesn't get displayed because the only place a person gets blocked is in the users table. To be more normalized, I'd just leave the block in the users table, but does someone intend to move the user block to the access table? ------------------------------------------------------------------------ June 15, 2005 - 21:37 : deekayen Attachment: http://drupal.org/files/issues/blockedloginmessage.patch (1.26 KB) Right now when users are blocked, they're not added to the access table, so this patch just checks the users table for blocked logins. The check in user.module's user_login() should work now and give the correct error message that was there all along. ------------------------------------------------------------------------ June 15, 2005 - 21:45 : deekayen Attachment: http://drupal.org/files/issues/removeactivereset.patch (882 bytes) This patch goes with the previous one. Without it, when you edit a blocked user to make them active again, they remain blocked and the edit screen shows an error message about the user being blocked. I don't really know if that's the intended behavior to have blocks be permanent, but permanent blocks don't seem right to me. ------------------------------------------------------------------------ June 16, 2005 - 09:04 : moshe weitzman I'm not so sure about this one. We should continue to distinguish between a 'user block' and a 'host ban'. ------------------------------------------------------------------------ June 16, 2005 - 09:12 : killes(a)www.drop.org I agree with Moshe. I think we should test whether a user hasn't logged in before and then give a different message then when he got blocked later. ------------------------------------------------------------------------ June 17, 2005 - 00:37 : deekayen Attachment: http://drupal.org/files/issues/blockeduser.diff (1.29 KB) Just your comments made me see where I went wrong with the previous patches. I'd never used the access control rules before, so I thought blocked user == access controlled username. As far as I could tell, the only place to where a blocked username check was needed is at login. This patch adds a user_blocked() to just user.module and has a separate blocked error message from the access check. ------------------------------------------------------------------------ June 17, 2005 - 00:46 : deekayen Attachment: http://drupal.org/files/issues/blockeduser_0.diff (1.29 KB) I'm not used to the Drupal bracket formatting, yet. This is a spacing revision. ------------------------------------------------------------------------ June 19, 2005 - 02:28 : Dries I think this patch makes sense, however the difference between 'the name X has been denied access' and 'the name X has been blocked' is too subtle for the average Drupal user/administrator to grasp. I'd like to have us think about rewording the error messages so they are somewhat more descriptive to administrators without exposing internal/technical detail to the user. How about writing 'the name X is a reserved user name' for the former, and 'the user with name X is no longer allowed access' for the latter? (One could argue we need the drupal_deny() check at login time.)

1 0