- Development - lists.drupal.org

[drupal-devel] [bug] Moving user_module_invoke after retrieving roles from database
by kees 13 Apr '05

13 Apr '05

Issue status update for http://drupal.org/node/20058 Project: Drupal Version: 4.5.2 Component: user.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: kees Updated by: kees Status: patch Hi, To be able to update the $user object from a module, the _user('load') hook must be called AFTER loading the current roles from the database, and not before. It is simply a matter of moving one line a few lines down. Here is the unified patch: --- drupal-4.5.2-hyper/modules/user.module 2005-04-01 10:16:15.000000000 +0200 +++ drupal-4.5.2/modules/user.module 2005-01-05 21:17:33.000000000 +0100 @@ -69,6 +69,7 @@ if (db_num_rows($result)) { $user = db_fetch_object($result); $user = drupal_unpack($user); + user_module_invoke('load', $array, $user); $user->roles = array(); $result = db_query('SELECT r.rid, r.name FROM {role} r INNER JOIN {users_roles} ur ON ur.rid = r.rid WHERE ur.uid = %d', $user->uid); @@ -79,7 +80,6 @@ else { $user = new StdClass(); } - user_module_invoke('load', $array, $user); return $user; } @@ -1191,7 +1191,6 @@ $op = arg(2) ? arg(2) : arg(1); } - switch ($op) { case t('E-mail new password'): case 'password': kees

4 3

[drupal-devel] [bug] node.module is not optional, should not be optional in module list
by moshe weitzman 13 Apr '05

13 Apr '05

Issue status update for http://drupal.org/node/12794 Project: Drupal Version: cvs Component: module system Category: bug reports -Priority: normal +Priority: critical Assigned to: Anonymous Reported by: robertDouglass Updated by: moshe weitzman Status: patch many users getting bitten by this in the forums. upgrading to critical moshe weitzman Previous comments: ------------------------------------------------------------------------ November 10, 2004 - 16:56 : robertDouglass If I disable all modules I get this error when clicking on a subsequent link Fatal error: Call to undefined function: node_list() in \drupal\includes\theme.inc on line 227 ------------------------------------------------------------------------ April 1, 2005 - 02:56 : asimmonds Attachment: http://drupal.org/files/issues/theme_inc_node_list.patch (693 bytes) Still happens if the node.module is disabled, attached patch should fix it.

1 0

[drupal-devel] [bug] Blocks with no title kill Drupal
by Robin Monks 13 Apr '05

13 Apr '05

Issue status update for http://drupal.org/node/19965 Project: Drupal Version: cvs Component: block.module Category: bug reports Priority: normal Assigned to: Robin Monks Reported by: budda Updated by: Robin Monks Status: patch Attachment: http://drupal.org/files/issues/4_0.6 blank title (3.62 KB) That patch had a bug in it, new patch attached. Robin Robin Monks Previous comments: ------------------------------------------------------------------------ April 4, 2005 - 17:03 : budda When setting more than one block's title to '' (blank) Drupal dies, and doesn't catch the SQL query error nicely. The error is as follows: user error: Duplicate entry '' for key 2 query: UPDATE boxes SET title = '', body = ' ...<cut>... in /var/www/html/drupal-cvs/includes/database.mysql.inc on line 66. Using 4.6 build from 4/4/2005 The page ends with: warning: Cannot modify header information - headers already sent by (output started at /var/www/html/drupal-cvs/includes/common.inc:384) in /var/www/html/drupal-cvs/includes/common.inc on line 192. ------------------------------------------------------------------------ April 8, 2005 - 08:08 : robertDouglass There should be a check before the insert is attempted and if the block title already exists the user should be notified with form_error(). ------------------------------------------------------------------------ April 8, 2005 - 09:46 : Poolio this certainly doesn't solve the problem, but I've worked around this issue by using html tags in the title field for blocks that I don't want to show a title. ------------------------------------------------------------------------ April 9, 2005 - 00:31 : Robin Monks Attachment: http://drupal.org/files/issues/block.module.blank.title.patch (955 bytes) A small patch that checks to ensure the title does not already exit in the Blocks list. Also, if the field was left blank, in adds an HTML comment with the current date to the second to ensure that a duplicate title isn't created. This is my first patch, so I probably did something wrong, but I'm willing to fix it too. Robin Monks MozNetwork.org ------------------------------------------------------------------------ April 9, 2005 - 01:22 : Morbus Iff I'm, personally, fine with how you've fixed the blank block title. You've really solved two problems in one: duplicate titles (which is the cause of this Issue), and the desire to have blank (or no) titles (which is the reason for this Issue) for the end user. With that said, I'm not a fan of your error message (but, then again, anal wording is my specialty, so don't think I'm picking on you): I'd rename it to "That block title is already in use; please choose another." The goals here: The user may not remember all the previous titles he's used. Using "unique" starts forcing him to remember. We shouldn't do that. The user already knows he entered it - we don't have to repeat that to him. It's active, not passive (no use of "have"). Use a semi-colon, not a comma. It's much shorter and concise. ------------------------------------------------------------------------ April 9, 2005 - 06:27 : chx Attachment: http://drupal.org/files/issues/box_title.patch (1.49 KB) Why display any message at all? ------------------------------------------------------------------------ April 9, 2005 - 14:11 : Morbus Iff Displaying no message at all would only work if the user wasn't typing in ANY title whatsoever (since then we'd make one for them based on the date). But, if a user names a block "Users" one week, and then creates another block called "Users" three weeks later, than an whoopsidaisy error needs to be shown - we shouldn't modify the text randomly. ------------------------------------------------------------------------ April 9, 2005 - 15:10 : chx Well, let's discuss the main point: why shoiuld be the title unique? ------------------------------------------------------------------------ April 9, 2005 - 15:38 : Morbus Iff Are you asking why Drupal enforces them being unique, or why, visually and to the end-user, they should be unique? ------------------------------------------------------------------------ April 9, 2005 - 15:43 : chx I guess Drupal forces them 'cos for some reasons it is thought that for the users they should be unique. This is not the case, however. Yes, it's confusing if they are on the same page. But I can think on circumstances where they would not be on the same page, so the force from Drupal seems unnecessary. ------------------------------------------------------------------------ April 9, 2005 - 19:00 : robertDouglass agree with chx on the unique point. Like the functionality of adding html comments and timestamp (why timestamp? why not a block id?) if left blank if they must be uniqe. Would be in favor of making the visible title completely optional. Administrators have description to keep them separate on the admin screen. ------------------------------------------------------------------------ April 9, 2005 - 19:48 : Robin Monks Attachment: http://drupal.org/files/issues/drupal_0.patch (3.67 KB) This new patch no longer requires a unique value for title, and also checks to ensure the Block Description is not a duplicate or empty value. Big thanks go out to chx for assistance. Robin ------------------------------------------------------------------------ April 11, 2005 - 14:20 : Anonymous Shouldn't we add a test too to check info unicity ? ------------------------------------------------------------------------ April 11, 2005 - 14:38 : chx <?php if (empty($edit['info']) || db_num_rows(db_query("SELECT info FROM {boxes} WHERE info='%s'", $edit['info']))) { ?> To me, this seems like a check for info unicity... not to mention that the UNIQUE(info) DB constraint is not touched. ------------------------------------------------------------------------ April 11, 2005 - 14:46 : tostinni Ups, sorry for the anonymous and the message, I didn't saw this verification, I was only remembering block.module code. That's all fine for me. ------------------------------------------------------------------------ April 11, 2005 - 16:27 : njivy +1 to Robin Monks' patch. When I install Drupal, the first thing I always do is remove the /unique key/ constraint on block titles. I have not tested the other parts of Monks' patch, though, including the uniqueness constraint on block descriptions. ------------------------------------------------------------------------ April 11, 2005 - 19:04 : drumm Code looks okay. +1 "You must enter unique data into the block description field." doesn't sound like the friendliest of error messages. ------------------------------------------------------------------------ April 11, 2005 - 19:16 : Dries I'm OK with this patch but suggest that we look into improving the error message. Small nit: fix the spacing around '=' in the SQL query that checks for duplicates. ------------------------------------------------------------------------ April 11, 2005 - 23:40 : Robin Monks Attachment: http://drupal.org/files/issues/drupal_1.patch (3.97 KB) Here is a new patch with the changes and with the updates.inc file changed to reflect patchs applied after the original patch was made. Robin ------------------------------------------------------------------------ April 12, 2005 - 18:52 : Dries Committed to HEAD. Thanks. The patch did not apply against DRUPAL-4-6. Possibly needs to get backported. ------------------------------------------------------------------------ April 13, 2005 - 15:36 : Robin Monks Attachment: http://drupal.org/files/issues/4.6 blank title (3.63 KB) Here is the patch backported for 4.6. Robin MozNetwork.org

1 0

[drupal-devel] [bug] Blocks with no title kill Drupal
by Robin Monks 13 Apr '05

13 Apr '05

Issue status update for http://drupal.org/node/19965 Project: Drupal Version: cvs Component: block.module Category: bug reports Priority: normal Assigned to: Robin Monks Reported by: budda Updated by: Robin Monks -Status: fixed +Status: patch Attachment: http://drupal.org/files/issues/4.6 blank title (3.63 KB) Here is the patch backported for 4.6. Robin MozNetwork.org Robin Monks Previous comments: ------------------------------------------------------------------------ April 4, 2005 - 17:03 : budda When setting more than one block's title to '' (blank) Drupal dies, and doesn't catch the SQL query error nicely. The error is as follows: user error: Duplicate entry '' for key 2 query: UPDATE boxes SET title = '', body = ' ...<cut>... in /var/www/html/drupal-cvs/includes/database.mysql.inc on line 66. Using 4.6 build from 4/4/2005 The page ends with: warning: Cannot modify header information - headers already sent by (output started at /var/www/html/drupal-cvs/includes/common.inc:384) in /var/www/html/drupal-cvs/includes/common.inc on line 192. ------------------------------------------------------------------------ April 8, 2005 - 08:08 : robertDouglass There should be a check before the insert is attempted and if the block title already exists the user should be notified with form_error(). ------------------------------------------------------------------------ April 8, 2005 - 09:46 : Poolio this certainly doesn't solve the problem, but I've worked around this issue by using html tags in the title field for blocks that I don't want to show a title. ------------------------------------------------------------------------ April 9, 2005 - 00:31 : Robin Monks Attachment: http://drupal.org/files/issues/block.module.blank.title.patch (955 bytes) A small patch that checks to ensure the title does not already exit in the Blocks list. Also, if the field was left blank, in adds an HTML comment with the current date to the second to ensure that a duplicate title isn't created. This is my first patch, so I probably did something wrong, but I'm willing to fix it too. Robin Monks MozNetwork.org ------------------------------------------------------------------------ April 9, 2005 - 01:22 : Morbus Iff I'm, personally, fine with how you've fixed the blank block title. You've really solved two problems in one: duplicate titles (which is the cause of this Issue), and the desire to have blank (or no) titles (which is the reason for this Issue) for the end user. With that said, I'm not a fan of your error message (but, then again, anal wording is my specialty, so don't think I'm picking on you): I'd rename it to "That block title is already in use; please choose another." The goals here: The user may not remember all the previous titles he's used. Using "unique" starts forcing him to remember. We shouldn't do that. The user already knows he entered it - we don't have to repeat that to him. It's active, not passive (no use of "have"). Use a semi-colon, not a comma. It's much shorter and concise. ------------------------------------------------------------------------ April 9, 2005 - 06:27 : chx Attachment: http://drupal.org/files/issues/box_title.patch (1.49 KB) Why display any message at all? ------------------------------------------------------------------------ April 9, 2005 - 14:11 : Morbus Iff Displaying no message at all would only work if the user wasn't typing in ANY title whatsoever (since then we'd make one for them based on the date). But, if a user names a block "Users" one week, and then creates another block called "Users" three weeks later, than an whoopsidaisy error needs to be shown - we shouldn't modify the text randomly. ------------------------------------------------------------------------ April 9, 2005 - 15:10 : chx Well, let's discuss the main point: why shoiuld be the title unique? ------------------------------------------------------------------------ April 9, 2005 - 15:38 : Morbus Iff Are you asking why Drupal enforces them being unique, or why, visually and to the end-user, they should be unique? ------------------------------------------------------------------------ April 9, 2005 - 15:43 : chx I guess Drupal forces them 'cos for some reasons it is thought that for the users they should be unique. This is not the case, however. Yes, it's confusing if they are on the same page. But I can think on circumstances where they would not be on the same page, so the force from Drupal seems unnecessary. ------------------------------------------------------------------------ April 9, 2005 - 19:00 : robertDouglass agree with chx on the unique point. Like the functionality of adding html comments and timestamp (why timestamp? why not a block id?) if left blank if they must be uniqe. Would be in favor of making the visible title completely optional. Administrators have description to keep them separate on the admin screen. ------------------------------------------------------------------------ April 9, 2005 - 19:48 : Robin Monks Attachment: http://drupal.org/files/issues/drupal_0.patch (3.67 KB) This new patch no longer requires a unique value for title, and also checks to ensure the Block Description is not a duplicate or empty value. Big thanks go out to chx for assistance. Robin ------------------------------------------------------------------------ April 11, 2005 - 14:20 : Anonymous Shouldn't we add a test too to check info unicity ? ------------------------------------------------------------------------ April 11, 2005 - 14:38 : chx <?php if (empty($edit['info']) || db_num_rows(db_query("SELECT info FROM {boxes} WHERE info='%s'", $edit['info']))) { ?> To me, this seems like a check for info unicity... not to mention that the UNIQUE(info) DB constraint is not touched. ------------------------------------------------------------------------ April 11, 2005 - 14:46 : tostinni Ups, sorry for the anonymous and the message, I didn't saw this verification, I was only remembering block.module code. That's all fine for me. ------------------------------------------------------------------------ April 11, 2005 - 16:27 : njivy +1 to Robin Monks' patch. When I install Drupal, the first thing I always do is remove the /unique key/ constraint on block titles. I have not tested the other parts of Monks' patch, though, including the uniqueness constraint on block descriptions. ------------------------------------------------------------------------ April 11, 2005 - 19:04 : drumm Code looks okay. +1 "You must enter unique data into the block description field." doesn't sound like the friendliest of error messages. ------------------------------------------------------------------------ April 11, 2005 - 19:16 : Dries I'm OK with this patch but suggest that we look into improving the error message. Small nit: fix the spacing around '=' in the SQL query that checks for duplicates. ------------------------------------------------------------------------ April 11, 2005 - 23:40 : Robin Monks Attachment: http://drupal.org/files/issues/drupal_1.patch (3.97 KB) Here is a new patch with the changes and with the updates.inc file changed to reflect patchs applied after the original patch was made. Robin ------------------------------------------------------------------------ April 12, 2005 - 18:52 : Dries Committed to HEAD. Thanks. The patch did not apply against DRUPAL-4-6. Possibly needs to get backported.

1 0

[drupal-devel] [bug] Trimmed post element not updated when node is edited via blogapi
by Anonymous 13 Apr '05

13 Apr '05

Issue status update for http://drupal.org/node/15071 Project: Drupal -Version: cvs +Version: 4.5.2 Component: blogapi.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: teradome Updated by: Anonymous -Status: active +Status: patch Hi, this is my solution (ugly hack?) for blogapi.module and function blogapi_edit_post(). Just insert this: unset($node->teaser); // update teaser for blog (see node_validate below) on line 184 or somewhere before line: $node = node_validate($node); Problem definition: A $node->teaser is not empty after call node_load() function and blogs does not support teasers, so when node_validate($node) function is called a teaser is not recreated. máca Anonymous Previous comments: ------------------------------------------------------------------------ January 3, 2005 - 19:57 : teradome Editing an existing post via blogapi (in my case, set to MovableType) does not update the node's "trimmed post" string--it remains the same as when last committted from the Drupal installation itself.

1 0

[drupal-devel] Node access module
by Karoly Negyesi 13 Apr '05

13 Apr '05

Hi! Although there are two contrib modules (node_privacy_by_role and nodeperm_role) which leverages the node access system, I have written a third one which is in my (chx) sandbox as nodeaccess.module. I do not intend it to be a contrib module -- I'd like to see it in 4.7 core. Features are: 1) You may give grants per role for each node type 2) Every node may have a grant tab where ordinary users may give additional grants for other users. Admins may edit grants per roles. Whether this grant tabs appears can be controlled again per node type. 3) Users may save these additional grants as default for their content. I think the UI is straightforward. TODO a) provide a mechanism to delete the default in 3) b) provide hooks for group modules so that users may operate on those, too So, this module is much more community oriented than the previous attempts while the admin remains in the driver's seat. Installation now requires a manual emptying of the node access table. Will improve this one, too. Regards Karoly Negyesi

7 8

[drupal-devel] [feature] More changes to forum help text
by nysus 13 Apr '05

13 Apr '05

Issue status update for http://drupal.org/node/19434 Project: Drupal Version: cvs Component: forum.module Category: feature requests Priority: normal Assigned to: nysus Reported by: nysus Updated by: nysus Status: patch Attachment: http://drupal.org/files/issues/forum_help.patch (5.98 KB) Here's some more changes to help text in the forum.module by order of significance: "Parent" selection help text: Added help description for this form item found on "admin/forum/add/container" and "admin/forum/add/forum". The help description is different depending on whether the type of component being added is a forum or a container. This change required the modifcation to the argument list passed to an internal function to forum.module. Removed duplicate help language created by the change above. Cut the fat from the help blurb on the "create content" page. "Weight" selection help text on "admin/forum/add/container" and "admin/forum/add/forum": Improved wording and made it more detailed. Help language for "name" and "description" fields on "admin/forum/add/container" and "admin/forum/add/forum" pages made more consistent and concise. Changed instances of "topic discussions" to "discussion topics" nysus

5 9

[drupal-devel] CVS question: Multiple branch tags on a module
by Syscrusher 13 Apr '05

13 Apr '05

Hi, all This time I'm asking before I screw something up. Dries has kindly wiped out the mangled mess I made in CVS, and I have successfully re-added my files and tagged them with DRUPAL-4-6 and DRUPAL-4-5 branches (the one code version works with both Drupal cores, because my code is fairly loosely-coupled). Now, when I modify a file and do a CVS commit of the new version, my understanding of the documentation on drupal.org is that I: 1. From a checked out local copy of HEAD, do a "cvs update -dP" to make sure I have the latest version of everything relevant. 2. Do whatever changes are needed to the file(s) of my module. 3. Do a "cvs commit", which will update HEAD only and not 4.6 or 4.5 branches. 4. Do a "cvs tag -b DRUPAL-4-6 {filenames}" and repeat for DRUPAL-4-5. Have I missed anything? Did I misunderstand any of this? Now that I've got things correct, I don't want to mess it up again. :-) Scott -- ------------------------------------------------------------------------------- Scott Courtney Drupal user name: "syscrusher" http://drupal.org/user/9184 scott(a)4th.com Drupal projects: http://drupal.org/project/user/9184 Sandbox: http://cvs.drupal.org/viewcvs/drupal/contributions/sandbox/syscrusher

3 5

[drupal-devel] Re: CVS question: Multiple branch tags on a module
by Sheldon Rampton 13 Apr '05

13 Apr '05

Jim Riggs wrote: >Now you can get more sophisticated with merging, but it's often not >worth the headache. Make changes in the various places independently >(copy and paste are your friends ;-) and then commit on each branch. >Don't try to tag again, though. It's already a done deal. Thanks for providing that explanation. Separately from Sycrusher, I've been trying to figure out how to do more or less the same thing with my interwiki module. Your instructions helped me figure out how to update the 4.5 branch of the module. However, the 4.5 branch still doesn't appear as the "latest release" when I go to the project page: http://drupal.org/project/interwiki If people click on the "download latest release," they still get an old version of the module rather than the current version. What can I do to fix this? --Sheldon Rampton

2 1

[drupal-devel] [bug] form_checkboxes saves "no selection" as string
by jhriggs 13 Apr '05

13 Apr '05

Issue status update for http://drupal.org/node/18663 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: critical Assigned to: chx Reported by: killes(a)www.drop.org Updated by: jhriggs Status: patch -1 on this patch. I still don't understand the problem. What do you mean by "form_checkboxes expected to be an array"? Is there code in core that assumes it is an array but it is not checked with is_array()? If so, let's add the missing checks. The same goes for contrib. This seems like swatting a fly with a sledge hammer to me. If we do go ahead and make this change, however, then we will need to change form_select() also (when multiple = TRUE). Radios and single select should be consistent, as should checkboxes and multiple select. jhriggs Previous comments: ------------------------------------------------------------------------ March 9, 2005 - 19:51 : killes(a)www.drop.org If you select nothing from a set of checkboxes an empty string is save to the variables table. This is a problem if the calling code assumes, it would get an array. We should make it save an empty array. The problem is the form_hidden field that we add. even if we replace form_hidden($name, 0) by form_hidden($name, array()) it isn't going to work, because the empty array is eaten by htmlspecialchars. redLED found this bug and he does not like it: 02:53 < redLED> i just like things saying 'CRITICAL' in red 02:53 < redLED> it gives this warm impending doom feeling 02:53 < redLED> and is generally associated with nuclear reactors in people's subconsciousness Suggestions welcome. ------------------------------------------------------------------------ March 12, 2005 - 12:47 : Steven There is no way to fix it in form_checkboxes() itself afaik, because even without htmlspecialchars, it would still come out as "Array". You can only create arrays indirectly through form submissions, by naming your values "foo[]". But you can not create empty arrays this way. So it is really up to the module which uses form_checkboxes() to handle this. A simple is_array() should do the trick. ------------------------------------------------------------------------ March 28, 2005 - 10:27 : chx Attachment: http://drupal.org/files/issues/checkboxes.patch (1.71 KB) system_settings_save can not really cope with this situation. This makes a problem with empty node options. And the fact that we have a check everywhere we use form_checkboxes is error prone, too. Here is a patch. If this accepted I'll comb the core for dead code. For eg. the first three lines of taxonomy_save_vocabulary and so. ------------------------------------------------------------------------ March 28, 2005 - 10:32 : chx Attachment: http://drupal.org/files/issues/checkboxes_0.patch (1.71 KB) Oh NO! One strayed space! I fixed it before steven beheads me.... ------------------------------------------------------------------------ March 28, 2005 - 11:36 : chx Attachment: http://drupal.org/files/issues/checkboxes_1.patch (1.64 KB) Among other changes, I have moved the hidden field form_checkboxes[] out of $edit. I have included HTML code, yes, but this is a very special thing, so it does not merit changing form_input just 'cos of this. And you won't need to theme a hidden field... ------------------------------------------------------------------------ April 8, 2005 - 01:19 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_2.patch (1.9 KB) killes did not like the previous version. ------------------------------------------------------------------------ April 11, 2005 - 17:15 : chx Dries? ------------------------------------------------------------------------ April 12, 2005 - 13:15 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_3.patch (1.64 KB) ------------------------------------------------------------------------ April 12, 2005 - 13:39 : jhriggs I have to agree with Steven that it is up to the calling code...and I hardly think this is critical. Code that is expecting an array from form_checkboxes() should ensure that it is dealing with an array (i.e. is_array()). As for dealing with variables set through things like system_settings_save(), the same thing applies. The code that is retrieving the variable should ensure that it is working with an array. With regards to your patch, what's the difference if the calling code has to remember to check via is_array() or remember to call fix_checkboxes()? This method is no less error prone as it's just as easy to forget this call as it is to check for an array. That is unless you are suggesting that fix_checkboxes() gets called for every page load which seems rather crufty. I wrote the original patch to add form_checkboxes() -- which didn't exist -- and if I remember, this was the desired behavior if nothing was selected. Isn't this the same behavior one would see using a multiple form_select() if the user chooses nothing? They should behave the same. A single select widget is the same as a radio group, and a multiple select widget is the same as a group of checkboxes. They should behave the same; the select types just take up less realestate. ------------------------------------------------------------------------ April 12, 2005 - 15:23 : chx Attachment: http://drupal.org/files/issues/form_checkboxes_4.patch (2.06 KB) Previous patch had index.php missing. jhriggs, the current problem is that a form_checkboxes expected to be an array. Currently, empty form_checkboxes is a string. That's bad.

1 0