August 2005 - Development - lists.drupal.org

[drupal-devel] [feature] new permission: create new books
by lgarfiel 11 Aug '05

11 Aug '05

Issue status update for http://drupal.org/node/8253 Post a follow up: http://drupal.org/project/comments/add/8253 Project: Drupal Version: cvs Component: book.module Category: feature requests Priority: normal Assigned to: moshe weitzman Reported by: moshe weitzman Updated by: lgarfiel Status: patch (code needs work) To be honest, I don't like the whole "administer XYZ" permission format in the first place. What the heck does "administer" mean, anyway? Create? Edit Own? Edit Others? Delete Own? Delete Others? That's five different possible operations. Which of them does "Administer" cover? I honestly have no idea. :-) Finer-grain permissions for those five operations would from a UI perspective be much much better, simply because they are easier to understand. (That goes for books and for everything else.) lgarfiel Previous comments: ------------------------------------------------------------------------ Wed, 02 Jun 2004 23:39:34 +0000 : moshe weitzman Attachment: http://drupal.org/files/issues/drbook.patch (1.39 KB) in order to create a new book, the user currently must have 'administer nodes' permission. this is a bit too broad. this patch introduces a 'create books' permission and improves the help text a bit. ------------------------------------------------------------------------ Tue, 10 Aug 2004 01:54:25 +0000 : moshe weitzman this patch is old now, with new menu system. functionality still needed. ------------------------------------------------------------------------ Fri, 01 Oct 2004 15:10:15 +0000 : moshe weitzman Updated patch. It is far less risky to grant 'create new book' as opposed to 'administer nodes'. ------------------------------------------------------------------------ Sun, 03 Oct 2004 16:11:29 +0000 : nedjo +1, this makes sense as a distinct permission. Should the permission test however be if (user_access('create books') || user_access('administer nodes')) {, instead of if (user_access('administer nodes')) {, on the assumption that 'administer nodes' includes creating books? ------------------------------------------------------------------------ Sun, 13 Mar 2005 21:04:11 +0000 : killes(a)www.drop.org Makes sense to me, but the patch is outdated. ------------------------------------------------------------------------ Wed, 10 Aug 2005 23:46:53 +0000 : killes(a)www.drop.org maybe somebody wants to look at it again. ------------------------------------------------------------------------ Thu, 11 Aug 2005 00:01:07 +0000 : merlinofchaos Should this permission be 'create new books' or simply 'administer books' or should they be separated out? I ask because if someone's going to work on this, it seems like a good idea to simply back away from administer node privileges entirely. The question that remains, to me, is whether or not there is any real gain by having create new books as its own permission. ------------------------------------------------------------------------ Thu, 11 Aug 2005 00:15:41 +0000 : puregin The entire issue of permissions for book module needs to be re-examined. See also http://drupal.org/node/21559. In the meantime, I believe that 'administer books' and 'create new books' should be kept distinct. Administer books, for example, implies permissions to delete other user's book pages. I can think of many situations where one might want a user to be able to create new books, but not mess up other peoples' books. Djun

1 0

[drupal-devel] [feature] new permission: create new books
by puregin 11 Aug '05

11 Aug '05

Issue status update for http://drupal.org/node/8253 Post a follow up: http://drupal.org/project/comments/add/8253 Project: Drupal Version: cvs Component: book.module Category: feature requests Priority: normal Assigned to: moshe weitzman Reported by: moshe weitzman Updated by: puregin Status: patch (code needs work) The entire issue of permissions for book module needs to be re-examined. See also http://drupal.org/node/21559. In the meantime, I believe that 'administer books' and 'create new books' should be kept distinct. Administer books, for example, implies permissions to delete other user's book pages. I can think of many situations where one might want a user to be able to create new books, but not mess up other peoples' books. Djun puregin Previous comments: ------------------------------------------------------------------------ Wed, 02 Jun 2004 23:39:34 +0000 : moshe weitzman Attachment: http://drupal.org/files/issues/drbook.patch (1.39 KB) in order to create a new book, the user currently must have 'administer nodes' permission. this is a bit too broad. this patch introduces a 'create books' permission and improves the help text a bit. ------------------------------------------------------------------------ Tue, 10 Aug 2004 01:54:25 +0000 : moshe weitzman this patch is old now, with new menu system. functionality still needed. ------------------------------------------------------------------------ Fri, 01 Oct 2004 15:10:15 +0000 : moshe weitzman Updated patch. It is far less risky to grant 'create new book' as opposed to 'administer nodes'. ------------------------------------------------------------------------ Sun, 03 Oct 2004 16:11:29 +0000 : nedjo +1, this makes sense as a distinct permission. Should the permission test however be if (user_access('create books') || user_access('administer nodes')) {, instead of if (user_access('administer nodes')) {, on the assumption that 'administer nodes' includes creating books? ------------------------------------------------------------------------ Sun, 13 Mar 2005 21:04:11 +0000 : killes(a)www.drop.org Makes sense to me, but the patch is outdated. ------------------------------------------------------------------------ Wed, 10 Aug 2005 23:46:53 +0000 : killes(a)www.drop.org maybe somebody wants to look at it again. ------------------------------------------------------------------------ Thu, 11 Aug 2005 00:01:07 +0000 : merlinofchaos Should this permission be 'create new books' or simply 'administer books' or should they be separated out? I ask because if someone's going to work on this, it seems like a good idea to simply back away from administer node privileges entirely. The question that remains, to me, is whether or not there is any real gain by having create new books as its own permission.

1 0

[drupal-devel] problem with IE and form_group_collapse()
by Gordon Heydon 11 Aug '05

11 Aug '05

Hi, I have having a problem with tables in a collapsible group when using IE. if you use the following code when you try to expand the group under IE the the table does not appear. I do think that this is some that I have done because on the upload module the upload group works fine once it has files attached it works fine, but for some reason I cannot work out why it does not work. Can someone please shed some light. Here is some example code that I just put into a node with the PHP filter and it replicates the problem. <?php $header = array('col 1', 'col 2'); $rows[] = array('col 1', 'col 2'); $tbl = theme('table', $header, $rows); echo form_group_collapsible('test form', $tbl, TRUE); ?> Thanks in advance. Gordon.

1 0

[drupal-devel] [feature] new permission: create new books
by merlinofchaos 11 Aug '05

11 Aug '05

Issue status update for http://drupal.org/node/8253 Post a follow up: http://drupal.org/project/comments/add/8253 Project: Drupal Version: cvs Component: book.module Category: feature requests Priority: normal Assigned to: moshe weitzman Reported by: moshe weitzman Updated by: merlinofchaos Status: patch (code needs work) Should this permission be 'create new books' or simply 'administer books' or should they be separated out? I ask because if someone's going to work on this, it seems like a good idea to simply back away from administer node privileges entirely. The question that remains, to me, is whether or not there is any real gain by having create new books as its own permission. merlinofchaos Previous comments: ------------------------------------------------------------------------ Wed, 02 Jun 2004 23:39:34 +0000 : moshe weitzman Attachment: http://drupal.org/files/issues/drbook.patch (1.39 KB) in order to create a new book, the user currently must have 'administer nodes' permission. this is a bit too broad. this patch introduces a 'create books' permission and improves the help text a bit. ------------------------------------------------------------------------ Tue, 10 Aug 2004 01:54:25 +0000 : moshe weitzman this patch is old now, with new menu system. functionality still needed. ------------------------------------------------------------------------ Fri, 01 Oct 2004 15:10:15 +0000 : moshe weitzman Updated patch. It is far less risky to grant 'create new book' as opposed to 'administer nodes'. ------------------------------------------------------------------------ Sun, 03 Oct 2004 16:11:29 +0000 : nedjo +1, this makes sense as a distinct permission. Should the permission test however be if (user_access('create books') || user_access('administer nodes')) {, instead of if (user_access('administer nodes')) {, on the assumption that 'administer nodes' includes creating books? ------------------------------------------------------------------------ Sun, 13 Mar 2005 21:04:11 +0000 : killes(a)www.drop.org Makes sense to me, but the patch is outdated. ------------------------------------------------------------------------ Wed, 10 Aug 2005 23:46:53 +0000 : killes(a)www.drop.org maybe somebody wants to look at it again.

1 0

[drupal-devel] [feature] new permission: create new books
by killes 11 Aug '05

11 Aug '05

Issue status update for http://drupal.org/node/8253 Post a follow up: http://drupal.org/project/comments/add/8253 Project: Drupal Version: cvs Component: book.module Category: feature requests Priority: normal Assigned to: moshe weitzman Reported by: moshe weitzman Updated by: killes(a)www.drop.org -Status: active +Status: patch (code needs work) maybe somebody wants to look at it again. killes(a)www.drop.org Previous comments: ------------------------------------------------------------------------ Wed, 02 Jun 2004 23:39:34 +0000 : moshe weitzman Attachment: http://drupal.org/files/issues/drbook.patch (1.39 KB) in order to create a new book, the user currently must have 'administer nodes' permission. this is a bit too broad. this patch introduces a 'create books' permission and improves the help text a bit. ------------------------------------------------------------------------ Tue, 10 Aug 2004 01:54:25 +0000 : moshe weitzman this patch is old now, with new menu system. functionality still needed. ------------------------------------------------------------------------ Fri, 01 Oct 2004 15:10:15 +0000 : moshe weitzman Updated patch. It is far less risky to grant 'create new book' as opposed to 'administer nodes'. ------------------------------------------------------------------------ Sun, 03 Oct 2004 16:11:29 +0000 : nedjo +1, this makes sense as a distinct permission. Should the permission test however be if (user_access('create books') || user_access('administer nodes')) {, instead of if (user_access('administer nodes')) {, on the assumption that 'administer nodes' includes creating books? ------------------------------------------------------------------------ Sun, 13 Mar 2005 21:04:11 +0000 : killes(a)www.drop.org Makes sense to me, but the patch is outdated.

1 0

[drupal-devel] [task] Remove theme_pager_detail()
by drumm 11 Aug '05

11 Aug '05

Issue status update for http://drupal.org/node/27980 Post a follow up: http://drupal.org/project/comments/add/27980 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: drumm Status: patch (ready to be committed) This themeable function is not used in core. It is used once in contrib. The author of that module is okay with removing it. drumm

5 8

[drupal-devel] [bug] Menu system requires a local task to have more than 1 child in order to create secondary local tasks
by andremolnar 10 Aug '05

10 Aug '05

Project: Drupal Version: cvs Component: menu system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: andremolnar Updated by: andremolnar Status: patch Attachment: http://drupal.org/files/issues/menu_inc_1a.patch (826 bytes) This is a minor bug. The menu system requires a local task to have more than 1 child in order to create secondary local tasks. In other words if a local task only had 1 child, menu system would not create a secondary local task menu item. This patch fixes that with the addition of 1 character in the code. andre andremolnar -- View: http://drupal.org/node/17022 Edit: http://drupal.org/project/comments/add/17022

4 5

[drupal-devel] Memory profiling for modules
by Mike Gifford 10 Aug '05

10 Aug '05

Hi folks, I was installing Civicspace a little while ago and was surprised that the installer recommended a non-standard memory_limit (8Mb is likely what most hosts provide and expect it is what most distros send php out with). I decided to set it up with 8M locally anyways, and most stuff seems to run except for a list of admin modules: ?q=en/admin/modules Neil Drumm informed me that the modules page "needs to get the metadata for all the modules which is stored in each module's php code. So that page loads /every/ module. That takes a lot of memory." Might be good to build some type of pagination in here for users who don't have a lot of flexibility with their web hosts, root access or confidence to muck about in the php.ini file. Anyways, I created this small patch to the drupal CVS which would allow developers to pretty quickly see what modules are sucking up the most memory: It produces results like this (although this is based on CivicSpace 0.8.1.2 code): Memory Usage before modules are loaded: 1,148 Kb Memory Usage (1) block: 1,267 Kb -- Change 119 Kb + Memory Usage (2) blog: 1,327 Kb -- Change 60 Kb Memory Usage (3) book: 1,499 Kb -- Change 173 Kb + Memory Usage (4) comment: 1,903 Kb -- Change 404 Kb ++++ Memory Usage (5) contact_manager: 2,255 Kb -- Change 352 Kb +++ Memory Usage (6) contact_manager_forms: 2,319 Kb -- Change 64 Kb Memory Usage (7) drupal: 2,371 Kb -- Change 52 Kb Memory Usage (8) filter: 2,592 Kb -- Change 222 Kb ++ Memory Usage (9) forms: 2,659 Kb -- Change 66 Kb Memory Usage (10) forum: 2,873 Kb -- Change 214 Kb ++ Memory Usage (11) help: 2,889 Kb -- Change 16 Kb Memory Usage (12) massmailer: 3,211 Kb -- Change 322 Kb +++ Memory Usage (13) menu: 3,325 Kb -- Change 114 Kb + Memory Usage (14) node: 3,719 Kb -- Change 394 Kb +++ Memory Usage (15) node_import: 3,799 Kb -- Change 80 Kb Memory Usage (16) notify: 3,816 Kb -- Change 17 Kb Memory Usage (17) page: 3,829 Kb -- Change 12 Kb Memory Usage (18) path: 3,885 Kb -- Change 56 Kb Memory Usage (19) phplist: 3,996 Kb -- Change 111 Kb + Memory Usage (20) search: 4,132 Kb -- Change 137 Kb + Memory Usage (21) statistics: 4,241 Kb -- Change 109 Kb + Memory Usage (22) story: 4,253 Kb -- Change 11 Kb Memory Usage (23) system: 4,445 Kb -- Change 192 Kb + Memory Usage (24) taxonomy: 4,683 Kb -- Change 238 Kb ++ Memory Usage (25) textile: 5,308 Kb -- Change 625 Kb ++++++ Memory Usage (26) urlfilter: 5,316 Kb -- Change 9 Kb Memory Usage (27) user: 5,802 Kb -- Change 486 Kb ++++ Memory Usage (28) watchdog: 5,840 Kb -- Change 38 Kb Memory Usage after all modules are loaded: 5,840 Kb I think that this could be useful for other folks who are looking at general profiling issues too. You need to enable the code by turning $profileMemory = true; There's likely a more graceful way to do this, but I'm still rather new to Drupal. This code also only works if you have memory_get_usage in your php install. Mike -- Mike Gifford, OpenConcept Consulting Free Software for Social Change -> http://www.openconcept.ca Latest launch Canadian Labour Congress / Congrès du travail du Canada http://canadianlabour.ca / http://congresdutravail.ca

5 4

[drupal-devel] [task] db_num_rows(db_query()) inefficiency
by Thomas Ilsche 10 Aug '05

10 Aug '05

Issue status update for http://drupal.org/node/28631 Post a follow up: http://drupal.org/project/comments/add/28631 Project: Drupal Version: 4.6.0 Component: search.module Category: tasks Priority: minor Assigned to: Anonymous Reported by: Thomas Ilsche Updated by: Thomas Ilsche Status: patch (code needs review) Attachment: http://drupal.org/files/issues/search.module.patch.txt (848 bytes) search.module uses $count = db_num_rows(db_query("SELECT DISTINCT i.sid, i.type FROM {search_index} i $join WHERE $conditions", $arguments)); I think using COUNT(i.sid) should provide an increase in effiecency. Waiting for comments. BTW there are 5 more occurances of "db_num_rows(db_query(" is there any reason for that? Thomas Ilsche

2 1

[drupal-devel] [bug] Problem when no node options are selected
by killes 10 Aug '05

10 Aug '05

Issue status update for http://drupal.org/node/19041 Post a follow up: http://drupal.org/project/comments/add/19041 Project: Drupal Version: cvs Component: node.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: wiz Updated by: killes(a)www.drop.org -Status: active +Status: patch (code needs work) changing status. the patch applies and probably also fixes the problem, but does nto follow Drupal's code conventions. killes(a)www.drop.org Previous comments: ------------------------------------------------------------------------ Thu, 17 Mar 2005 08:13:19 +0000 : wiz Attachment: http://drupal.org/files/issues/node-options-1.patch (1.21 KB) When you deselect all node options (published, in moderation queue etc.), an error occurs when you next edit this node. Reason: includes/common.inc:form_checkboxes() adds a hidden field with the value "0" that is overwritten when at least one checkbox is checked. Usually the value of the $edit variable is an array, in this case it is "0". A possible fix is to convert "0" to array() at the appropriate code points. ------------------------------------------------------------------------ Thu, 09 Jun 2005 21:58:08 +0000 : nysus I can confirm this bug. ------------------------------------------------------------------------ Thu, 09 Jun 2005 22:02:09 +0000 : nysus Patch seems to have fixed the bug. But I just did one quick test.

1 0