- Development - lists.drupal.org

[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: cvs Component: theme system Category: bug reports Priority: normal Assigned to: Robin Monks Reported by: stefan nagtegaal Updated by: Robin Monks Status: patch Attachment: http://drupal.org/files/issues/tablesort_header_1.patch (1.61 KB) Thox suggested using indicator instead of header. 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'. ------------------------------------------------------------------------ June 20, 2005 - 12: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 ------------------------------------------------------------------------ June 20, 2005 - 12:47 : Bèr Kessels 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. ------------------------------------------------------------------------ June 20, 2005 - 13:32 : Robin Monks Attachment: http://drupal.org/files/issues/tablesort_header_0.patch (1.6 KB) Here is a new patch using if instead of the switch. 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: cvs Component: theme system Category: bug reports Priority: normal Assigned to: Robin Monks Reported by: stefan nagtegaal Updated by: Robin Monks Status: patch Attachment: http://drupal.org/files/issues/tablesort_header_0.patch (1.6 KB) Here is a new patch using if instead of the switch. 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'. ------------------------------------------------------------------------ June 20, 2005 - 12: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 ------------------------------------------------------------------------ June 20, 2005 - 12:47 : Bèr Kessels 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.

1 0

[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