April 2005 - Development - lists.drupal.org

[drupal-devel] [feature] Request New Password Security
by Dries 06 Apr '05

06 Apr '05

Issue status update for http://drupal.org/node/18719 Project: Drupal Version: cvs Component: user.module Category: feature requests Priority: critical Assigned to: Anonymous Reported by: neofactor Updated by: Dries Status: patch Good job. Some more remarks: - Don't glue words together. Please use 'login' instead of 'lastlogin'. - Write 'administrator' not 'admin'. - You capitalized some random 'subjects' (eg. those in the form's help text). Dries Previous comments: ------------------------------------------------------------------------ March 11, 2005 - 04:52 : neofactor Problem: Any user can force another user's password to change... simply by selecting "request new password" and putting in their username. The user gets an email with the new.. but this feels like a violation to the user... and a pain. Solution? If someone requests a new password... Don't blindly change it... send an email that says...."Is this a real request authorized by you? Click here to confirm otherwise disregard this message" Please consider this critical for user by-in to Drupal as a secure system. I appreciate your consideration. http://drupal.org/node/18689 ------------------------------------------------------------------------ March 11, 2005 - 06:05 : neofactor I added some code to prevent the admin account from being reset... Please add as a patch. if ($account->uid == 1 { unset($account); form_set_error('name', t('Sorry. The username %name is not allowed to be changed.', array('%name' => '<em>'. $edit['name'] .'</em>'))); } // Just above this code on line 911: if ($account) { $from = variable_get('site_mail', ini_get('sendmail_from')); $pass = user_password(); ------------------------------------------------------------------------ March 11, 2005 - 06:08 : killes(a)www.drop.org Please only set to "patch" if you are actually submitting one. The solution you proposed is rather a workaround and will not be acceptable for core. ------------------------------------------------------------------------ March 11, 2005 - 10:21 : Deno I was in charge of a web site with >10000 paying customers for some time, and the "customers care" was constantly troubled with pasword change requests, until we switched to a system that worked in the following way: 1) "I forgot the password" function accepts either users email, or users login name, generates a random phrase, and adds an entry to "pasword_change_request" table. This entry consists of: - User ID (corresponding to email or login) - random phrase - datestamp User ID is unique key, and the entries in this table older than MAX_TIME are regularly purged by a cron job. 2) System sends an email with a link to "change forgotten password" page with this random phrase as argument to the user. That would be something like: https://drupal.org/user/cfp/d438rwiuodw894732ehdw 3) When user follows the link, "cfp" page checks if the random phrase exists in the table, allows the user to immediately change the password, and sends another email with "your password was changed on RIGHT_NOW ... " note to the user in question. - Password change automatically triggers deleting of the corresponding entry in pasword_change_request table. - Password change automatically logs-in the user as well. This way, users weren't bothered by accidental password changes, and they also understood the system better than the one with automatically generated passwords. For additional security, we also built some brakes that assured that the number of attempts to change the password, such as: - "An email with instructions was sent to you X minutes ago. In case you don't receive it within N hours, please contact the site administrator" - "According to our logs, you have visited the CFP page more than NMAX times within last 24 hours. For security reasons, you will be denied access to this page for the next 24 hours. Please contact the site administrator." hope this helps... ------------------------------------------------------------------------ March 30, 2005 - 22:01 : Jose A Reyero Attachment: http://drupal.org/files/issues/user_one_time_login.patch (6.52 KB) This patch for -cvs- addresses this issue. It generates an *only one use* URL to login into user account, instead of a new password. Then the user clicks on the URL, logs in, and may change his old password if desired. It is a only one use URL, and has some additional security measures, like a configurable time-out -can be confgured in admin/user/settings- and also, if somebody uses it or password is changed in the meanwhile, previously generated login URLs of this type won't work. About how it works, it creates a new url with: - user id, - timestamp - a hash of current timestamp, changed date for account, and the old password hashed With this patch, this wont work for admin accounts, which I think is better, you never know... It has a number of features like: - User can be ignoring these emails if somebody else is requesting access, a new password request wont change current password. However, the user will be warned when seeing the emails - It reveals no sensible information about the user account in the URL itself - the generated hash is not predictable at all if you dont know the old password, as it depends on user's old password - there's a configurable timeout - The URL's are only one use, as they depend on two timestamps, one of which will be changing in case this login is used -or in case user logs in for other means - Any 'new password' request is independent of others, so no one can be interfering with one user asking for his password back. Also doesn't require any new db field nor keeping track of requested passwords. ------------------------------------------------------------------------ March 30, 2005 - 22:41 : chx Very nice patch. Please consider for 4.6. ------------------------------------------------------------------------ March 31, 2005 - 00:16 : drumm Couldn't those nested if statements be made into one with a bit cleaner indentation? It looks like the page which the user arrives at after getting the new login information is their user page, not a page to set the password. The "Time out for password recovery e-mail" configuration is very unfriendly. It should be a drop down select of the most used human readable durations. Or we could just decide on one timeout and leave that hard coded in; who actually needs to use that setting? The url is quite long, what could be done to make it shorter? ------------------------------------------------------------------------ March 31, 2005 - 01:50 : Jose A Reyero /Couldn't those nested if statements be made into one with a bit cleaner indentation?/ Maybe, but I had to split that if conditions in two parts because the $account var didn't get the value before the rest of the conditions -latest two- where evaluated. Someone can help with PHP here? /It looks like the page which the user arrives at after getting the new login information is their user page, not a page to set the password. The "Time out for password recovery e-mail" configuration is very unfriendly. It should be a drop down select of the most used human readable durations. Or we could just decide on one timeout and leave that hard coded in; who actually needs to use that setting?/ Agreed, will be polishing this a little bit, just wating for some more suggestions... For the duration, I'll change to 'hours', ok? Just don't like to restrict options using unneccessary dropdowns, it's only intended for administrators anyway. Maybe I add also a '0' for unlimited. Hardcoding it, would need anyway to be mentioned in the module help, so it's about the same overhead, both for coding and for the admin interface, and harcoding in two places in code is not good anyway. And I like having options, am I the only one? / The url is quite long, what could be done to make it shorter?/ Well, the url has to be long, it has neccessary params and an MD5 hash, which yes, could be cut someway, but that adding inneccessary code and also reducing security -though half an MD5 hash may be secure enough- but I see no point in that. ------------------------------------------------------------------------ March 31, 2005 - 03:18 : FactoryJoe(a)civicspacelabs.org "Or we could just decide on one timeout and leave that hard coded in; who actually needs to use that setting? " +1 for making this *not* configurable! C'mon, Drupal should have standards about security -- and if best practices say that, say, 8 hours, is a good amount of time for that URL to be active, then do that. Cut out the confusion and extra brain juice it takes to decide... "Gee, is 4 hours better than 5?" Additionally, there's a simple way to make this workflow better for the user. If you visit an outdated change password URL, you should be told, "Sorry, but that link you followed to change your password has expired. If you would like to reset your password, please fill out this form: [form] If you did not request to change your password, please notify [administrator's name w/ PM link]". This way, if you missed your change password deadline, you can request your password again right from the timeout page. If you didn't request the change, it will be obvious that someone else was trying to change it, and then you can contact the administrator and find out who it was that was messin' with your account. ------------------------------------------------------------------------ March 31, 2005 - 07:55 : chx Attachment: http://drupal.org/files/issues/user_one_time_login_0.patch (4.72 KB) I cleaned up the patch a bit. I hope code style is OK. So may -- there are no checks against the arguments? There is no need. user_load uses %d for 'uid', so that's dealt with. Comparison by < and > means an implicit cast to integer, so $timestamp is also dealt with automatically. I do not think timeout needs to be configured. Please comment on not letting uid 1 using this one. I am not sure whether this is necessary. ------------------------------------------------------------------------ March 31, 2005 - 08:12 : chx Attachment: http://drupal.org/files/issues/user_one_time_login_1.patch (6.21 KB) Implemented Chris' idea, so if you use a one time URl which is expired, you are sent to user/password with a message to ask for another. I changed a few messages and deleted the "not for admin" feature. Do we really want one gazillion support requests "I locked myself out of my site, user/password tells me not for admin, what now"? ------------------------------------------------------------------------ March 31, 2005 - 09:24 : chx One more point on uid 1. Yes, uid 1 can reset his pwd over the database. But then we would need to lock out administer users privileged users. Then administer nodes, 'cos of XSS. Then administer comments... then administer filters... OK, this is pointless. If someone can eavesdrop on your emails, everything is lost, I think. This is not something Drupal shall address. Maybe we could employ a security question -- secret answer pair. ------------------------------------------------------------------------ March 31, 2005 - 12:52 : Bèr Kessels Eventhough I am a less-settings advocate, I think this must remain a setting :). Why? Sites on Drupal differ a lot, and have a very differnet userbase. I run very low level sites, where it sometimes takes days before someone will read his/her mail. But I also know sites (drupal.org) where days would certainly be too long. Sorry Chris et al, I +1 on configurable duration. (hell, we really need some about:config alike screen in drupal) ------------------------------------------------------------------------ March 31, 2005 - 13:46 : Dries IMO, that setting has nothing to do with a site being "low level". If you request a new password, you probably want to login ASAP. I vote against a configuration option. Either way, this patch needs work: 1. We don't glue words together (eg. $hashedpass, $currenttime, $loginurl). Similarly, I suggest to change 'user/newpassword' to 'user/reset' or 'user/request-password'. 2. The patch exposes technical details to the user; terminology like "one time login URL" is likely to confuse many users. 3. The login URL is not clean URL safe, I think. 4. Please rename the variable $pass1 in user_pass_rehash(). 5. The code comments are often inconsistent. Some end with a punctuation, some don't. Some span two lines, others don't. On a related note, can't we reuse this system for the initial password? That is, instead of mailing a password, we mail an URL and force the user to enter a password. ------------------------------------------------------------------------ March 31, 2005 - 19:20 : Jose A Reyero Attachment: http://drupal.org/files/issues/user_one_time_login_2.patch (12.77 KB) Ok, I've started with chx's reviewed patch, and done most of Dries suggestions Main changes are: ----------------------- - Rewording, variable names, path, etc... as pointed out by Dries. Now it's user/reset. And yes, it was not clean-URL safe; it is now. - Allowed also for admin account, I dont like it though, but seem to be the only one :( - Used also for user registration, but I also kept old style login/password, think it can be some use and anyway, it's up to the site admin to remove it from the welcome e-mail. However it is only for 'no admin approval', see below about this.... - Added back in the config option for time-out, though reworked completely for usability. Default is '1 day' which I think should be reasonable. However, there's no time out for firts time users. About this, read below, please... ----------------------- Using this for first time login has some problems when admin aproval is required, as the hashed pass depends on 'changed' timestamp, and it changes when the admin approves an account. Actually, there's no way to know -or I didn't find it- when it is the first login for an approved account. For no admin approval, I just used 'changed' == 'created' as a criterium. I think we should be separating this changed date and last login date creating a new field in the user table, but that maybe for a future patch. That would also increase security, as the Administrator's info would be more accurate, and 'last login date' could be shown when user logs in, which is also security+ Related with this I've experienced problems, as administrator, when editing user accounts, then last login info for the users gets lost -is reset-... About the config option for time out, though I agree that enforcing security can be good in general, this is one of this cases where more security means less usability. And that decission should be left to Site administrators -each site has its own security requirements-. So I think the target here should be a 'reasonable' default, and then some flexibility for site admins. IMHO this latest patch is quite well balanced. So... everybody happy? ------------------------------------------------------------------------ March 31, 2005 - 21:43 : Dries "So ... everyone happy?" Not quite. The patch is an improvement but it uses tabs, incorrect spacing, incorrect placement of brackets, and does not use theme('placeholder'). If a new database column is required to implement this properly, please add one. This patch won't get committed until it is implemented properly. I still vote to remove the configuration option. We're getting there ... ------------------------------------------------------------------------ April 2, 2005 - 20:45 : factoryjoe I’m going to have to stick by my earlier assertion on this one, and, and, as with most usability issues, none of us are “correct” until we test it 50 times and see whether anyone actually uses the option. Just like with my folksonomy review [1], more options is *not always better* nor does it always help people *get things done*! To further expand this point, Jose says [2]: About the config option for time out, though I agree that enforcing security can be good in general, this is one of this cases where more security means less usability. And that decission should be left to Site administrators -each site has its own security requirements. " But this is a misuse of the term “usability”. What you’re talking about is *functionality*—since usability has more to do with focusing single-mindedly on the task at hand and reducing distraction than it does with providing extraneous options that might be useful for a fraction of all Drupal admins. To speak to Ber’s point [3], I cannot imagine a situation where, if I want to change my password because I’ve forgotten it, that it will take more than a few hours to get the email and fulfill the process. And even still, if it does — for whatever reason — take me a week to get around to resetting my password, then starting the process over again isn’t all that time consuming. Plus, I have the added sense of security that Drupal doesn’t let things like password resetting links stay active forever! So again, I agree that we need a reasonable default—24 hours is very generous for the singular task that we’re trying to serve here, which is, to my knowledge, to make the process for resetting your password easier and more secure. Lastly, about the patch itself: I noticed a couple <em> tags strewn around in lines 38, 43, 54 and 72 (as well as a <strong> on 54). Until we have a style guide for marking up usernames, should we really be using HTML like this? It seems to be that it would be better to use a span with a class of… say… “username” and let the themer determine whether the username should be bold, italicized, red, all caps and so on.*/ [1] http://drupal.org/node/19697#comment-25599 [2] http://drupal.org/node/18719#comment-25480 [3] http://drupal.org/node/18719#comment-25463 ------------------------------------------------------------------------ April 2, 2005 - 22:29 : factoryjoe One other quick question. How does this patch interact with distributed authentication? If I'm logged in with a remote account and ask to reset my password, what happens? ------------------------------------------------------------------------ April 3, 2005 - 11:22 : Dries factoryjoe: you can't reset your password when you are logged in. ------------------------------------------------------------------------ April 3, 2005 - 15:03 : Jose A Reyero About usability I've written this piece [4], so I really prefer to discuss it on the forums... That patch, I'll be cleaning it up a little bit, adding that last login field, and maybe go for that 'reasonable default', which anyway is not worth so much discussion.... [4] http://drupal.org/node/19918 ------------------------------------------------------------------------ April 5, 2005 - 02:20 : Jose A Reyero Attachment: http://drupal.org/files/issues/user_one_time_login_3.patch (20.26 KB) Ok, lets try to make people happy, which will be good for my karma, I hope so ;-) So, changes: - Added 'lastlogin' field to user table. Now there's one timestamp for when an account is edited and another one for login. Of course, I've also taken care of the user's admin page. - Some small detail like help text for e-mail fields and that. - Now it can be used also for accounts with admin approval. - And yes, removed config option, 1 day time out for password recovery, no time out for first login. By the way, I've also patched db files for PostgreSQL, please someone review it, I don't know too much about it. The default e-mails may need better wording. Specially the one for password recovery which just says 'You have requested a new password', but I dont want users get too worried in case someone else has requested that password and the old one that said something like 'This is your new login information..' which may suggest it has been sent by the admin maybe updating accounts..... Anyway, every site admin can configure that email... As an idea for the future or for someone else - I've had enough for now with logging in 100 times to try this-, now that we can know when it is the first login for an user, a nice welcome message or welcome screen would be great :-) Or if someone wants to go for better security *nix style, some message telling the user when last logged in wouldn't be bad... So... ? ------------------------------------------------------------------------ April 5, 2005 - 02:51 : factoryjoe +1 lastlogin time. Also, I just lost my admin password for WP and these are the emails I received at each stage of the process (email titles in bold): *[FactoryCity] Password Reset* "Someone has asked to reset a password for the login this site http://www.factorycity.net/blog [5] Login: admin To reset your password visit the following address, otherwise just ignore this email and nothing will happen. http://www.factorycity.net/blog/wp-login.php?action=resetpass&key=XXXXXf4xx… " *[FactoryCity] Your new password* " Login: admin Password: XxXxXxX http://www.factorycity.net/blog/wp-login.php " *[FactoryCity] Password Lost/Change* "Password Lost and Changed for user: admin " [5] http://www.factorycity.net/blog

1 0

[drupal-devel] [bug] Missing <div> container on HTML output
by Dries 06 Apr '05

06 Apr '05

Issue status update for http://drupal.org/node/20079 Project: Drupal Version: cvs Component: book.module Category: bug reports Priority: minor Assigned to: Anonymous Reported by: budda Updated by: Dries Status: patch Where does Drupal core emit a class="menu" ? Dries Previous comments: ------------------------------------------------------------------------ April 6, 2005 - 19:46 : budda Line 588 should be made to output the tree navigation within a container the same as the core navigation block - to keep consistent. new line 588: return '<div class="menu"><ul>'. $tree .'</ul></div>'; You are still able to format the book tree navigation different to the core navigation block as this tree is a child of css class 'book-block'. ------------------------------------------------------------------------ April 6, 2005 - 19:48 : killes(a)www.drop.org Attachment: http://drupal.org/files/issues/book_28.patch (558 bytes) Here's the patch.

1 0

[drupal-devel] [bug] Cannot delete users
by Dries 06 Apr '05

06 Apr '05

Issue status update for http://drupal.org/node/19750 Project: Drupal Version: cvs Component: user.module Category: bug reports Priority: critical Assigned to: Anonymous Reported by: AtomicMaestro Updated by: Dries Status: patch Mmm, Steven's patch fixes the tabs but still is a bit of a hack. (Also, parts of the strings can't be translated, the use of arg(1) vs $edit['nid'] looks a bit weird.) Can't we fix this properly? Dries Previous comments: ------------------------------------------------------------------------ March 31, 2005 - 11:33 : AtomicMaestro I can't seem to delete users. To reproduce: 1. Create a user. 2. View the user created in 1. 3. Click Delete I would expect to be told I have to block the user first, but instead, I'm sent back to the user report. There's no message, nothing in the log. ------------------------------------------------------------------------ April 1, 2005 - 09:40 : asimmonds Attachment: http://drupal.org/files/issues/fix_delete_user.patch (581 bytes) Patch attached to fix this, the delete code in node.module works the same way ------------------------------------------------------------------------ April 3, 2005 - 16:15 : moshe weitzman when one deletes a user from list of users in admin, he expects to return back to that list after successfully deleting. thats the intent of the $_REQUEST['destination']. I think this patch will defeat that behavior. There certainly is a bug here, but I don't think we have found the right fix yet. I suggest testing a delete from both admin and user/edit pages. make sure you arrive at the expected page after a successful delete. ------------------------------------------------------------------------ April 4, 2005 - 09:19 : asimmonds There are two methods to enter the user edit page as a admin: 1/ View a user, then select the edit tab. 2/ Click 'edit' for the user in the admin/user list. The 2nd method sets destination=admin/user in the URL, when the user edit page is shown. If the Delete button is clicked, it calls drupal_goto("user/$account->uid/delete") (the code's comment mentions this is done to make the tabs disappear). As destination is already set, drupal_goto() redirects to that ie. "admin/user" instead of "user/$account->uid/delete" If after successfully deleting a user, the code calls drupal_goto("admin/user"). I looked at how node.module does the node delete/confirm sequence and it has this unset($_REQUEST['destination']) in there as well. Maybe this delete/confirm sequence pattern needs a re-think for 4.7 ------------------------------------------------------------------------ April 6, 2005 - 06:01 : moshe weitzman Attachment: http://drupal.org/files/issues/user_15.patch (1020 bytes) here is an alternative. this one properly maintains the destination parameter which is useful especially when you are not on the 1st page of your user listing. the redirect insures that you return to the page you were on. another advantage is that i was able to delete a few lines of code which were performing a drupal_goto(). the downside is that the pupose of the drupal_goto() was to get rid of the tabs. I don't mind the tabs on the delete confirm page, so I think my patch is better, and removes a couple lines of code. either patch presented here would work ok. ------------------------------------------------------------------------ April 6, 2005 - 07:13 : moshe weitzman Attachment: http://drupal.org/files/issues/menu_set_location.patch (2.23 KB) here is a different patch suggested by UnConed. Uses menu_set_location() to remove the tabs on the delete confirmation page for both nodes and users.

1 0

[drupal-devel] [task] Document cookie life-time in settings.php
by Uwe Hermann 06 Apr '05

06 Apr '05

Issue status update for http://drupal.org/node/20013 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: Anonymous Reported by: Uwe Hermann Updated by: Uwe Hermann Status: patch Attachment: http://drupal.org/files/issues/settings.php_0.patch (951 bytes) As threads like this [1] show, it would probably be good to document the cookie life-time in settings.php explicitly. Patch attached, might need some re-wording, though. [1] http://drupal.org/node/20002 Uwe Hermann

3 3

[drupal-devel] [bug] Update script should have clearer instructions when admin session is not in database copy
by willmoy 06 Apr '05

06 Apr '05

Issue status update for http://drupal.org/node/19941 Project: Drupal Version: cvs Component: other Category: bug reports Priority: normal Assigned to: willmoy Reported by: willmoy Updated by: willmoy Status: patch Attachment: http://drupal.org/files/issues/update.php_0.patch (1.75 KB) The following is from: http://civicspacelabs.org/home/node/9042 The documentation for error condition when the Admin 1 user was not logged into the database when it was backed up is inadequate. The documentation should read: With an editor find the update.php file on your system. It should be in the main CivicSpace directory that you installed all the files into. Change the line at the top of your update.php page and change what is between the quotes from $access_check = "TRUE"; to $access_check = "FALSE"; As soon as the script is done, you must change the update.php script back to it's original form to $access_check = "TRUE"; We should also explain that next time the user should back up the database with the Admin 1 user logged in to avoid this problem. The final text this patch inserts is: Access denied. You are not authorized to access this page. Please log in as the admin user (the first user you created). If you cannot log in, you will have to edit update.php to bypass this access check. To do this: 1. With a text editor find the update.php file on your system. It should be in the main Drupal directory that you installed all the files into. 2. There is a line near top of your update.php page that says $access_check = "TRUE"; Change it to $access_check = "FALSE"; 3. As soon as the script is done, you must change the update.php script back to it's original form to $access_check = "TRUE"; 4. To avoid having this problem in future, remember to log in to your website as the admin user (the user you first created) before you backup your database at the beginning of the update process. willmoy

3 5

[drupal-devel] [bug] Missing <div> container on HTML output
by killes 06 Apr '05

06 Apr '05

Issue status update for http://drupal.org/node/20079 Project: Drupal Version: cvs Component: book.module Category: bug reports Priority: minor Assigned to: Anonymous Reported by: budda Updated by: killes(a)www.drop.org -Status: active +Status: patch Attachment: http://drupal.org/files/issues/book_28.patch (558 bytes) Here's the patch. killes(a)www.drop.org Previous comments: ------------------------------------------------------------------------ April 6, 2005 - 18:46 : budda Line 588 should be made to output the tree navigation within a container the same as the core navigation block - to keep consistent. new line 588: return '<div class="menu"><ul>'. $tree .'</ul></div>'; You are still able to format the book tree navigation different to the core navigation block as this tree is a child of css class 'book-block'.

1 0

[drupal-devel] Re: Help?
by Sheldon Rampton 06 Apr '05

06 Apr '05

Jim Riggs wrote: >The latest release date is based on when the branch was created, not >when something was last committed. I understand that. That's the problem. I want the current version of the module to be what people get when they click on the link that says "download latest release." Right now they're getting an old version. How do I fix that? --Sheldon

4 3

[drupal-devel] Re: [contributions(DRUPAL-4-5):sheldon]
by Sheldon Rampton 06 Apr '05

06 Apr '05

Goba wrote: >The FAQ and the contributors guide outlines the commands used to >checkout, commit, tag, etc. the files. The above confusions are the very >reason that the php.net projects suggest contributors TO NOT USE visual >CVS tools. They always mess up. All the time. Actually, the Drupal contributors guide advises Macintosh users TO use CVL as their CVS tool: http://drupal.org/node/2972 In fact, that's why I started using the damn thing in the first place. I WAS trying to follow the instructions in the contributor's guide. The contributors guide also advises Windows users to use TortoiseCVS as their front end: http://drupal.org/node/635 Dries wrote: >Does that mean all Drupal 4.5 releases are messed up now? Err. I don't know. When this happened, my CVL console spat out a bunch of messages that looked like errors or refusals. Just now I've tried spot-checking the releases page (drupal.org/project/releases) and all of the projects I've looked at show "latest release" dates from prior to my accident. (For that matter, even my interwiki project still shows a latest release date of December 31, 2004. I've tried several times to move the tags just on the interwiki project and can't get it to take.) It looks to me like either no real harm was done, or else someone caught it quickly and fixed it. However, I think it's clear that I'm far from expert in using CVS, so please don't take my word for anything. Based on my experience, I would recommend revising the contributor's guide so it doesn't advise Macintosh users to use CVL. Since I don't use Windows, I don't know whether continuing to recommend TortoiseCVS is a good idea, but if your goal is to ensure that contributors know what they're doing in CVS, I think it would be better to simply recommend using command-line access via a Unix terminal, and to provide a link to a good tutorial. --Sheldon

7 6

Re: [drupal-devel] Re: [contributions(DRUPAL-4-5):sheldon]
by Jeremy Epstein 06 Apr '05

06 Apr '05

On Apr 5, 2005 1:54 AM, Sheldon Rampton <sheldon(a)prwatch.org> wrote: > > Based on my experience, I would recommend revising the contributor's > guide so it doesn't advise Macintosh users to use CVL. Since I don't > use Windows, I don't know whether continuing to recommend TortoiseCVS > is a good idea, but if your goal is to ensure that contributors know > what they're doing in CVS, I think it would be better to simply > recommend using command-line access via a Unix terminal, and to > provide a link to a good tutorial. > I'm a full-time Windows user (and ashamed of it, as I should be), so I followed the advice in the Drupal handbook, and used TortoiseCVS to access the repository. The handbook has very clear advice on using TortoiseCVS to download and update the repositories to your local PC (this is the simplest thing you can do with CVS). So this is what I've been using TortoiseCVS for over the past month or so: logigng in anonymously, and just grabbing all the files that are available. When I got my CVS contrib account, I tried using TortoiseCVS to make my first commit. I was too scared to do it: the handbook has no advice on how to do this (except using CLI); and there are just so many menu options you can select, and it's quite frightening, knowing that pushing many of them could potentially stuff up other people's files. So my policy now is to use command-line CVS (through Cygwin) for commits, and to use GUI CVS only for refreshing my files (anonymously). I would advise all my fellow newbie-contributors to do the same thing: only make commits using command-line CVS! Especially since the handbook (to date) only has instructions for making commits using the command-line utility. Jaza.

5 5

[drupal-devel] [feature] common.inc Move textarea pre hook between title and textarea
by Dries 06 Apr '05

06 Apr '05

Issue status update for http://drupal.org/node/18828 Project: Drupal Version: cvs Component: base system Category: feature requests Priority: normal Assigned to: Anonymous Reported by: jbond Updated by: Dries Status: patch Committed to HEAD. Thanks. Dries Previous comments: ------------------------------------------------------------------------ March 13, 2005 - 11:52 : jbond Attachment: http://drupal.org/files/issues/common.inc_10.patch (1.8 KB) This patch moves the textarea hook for 'pre' between the title and the textarea. This makes it easier for simply textarea enhancements like quicktags to add a toolbar just above the textarea. It doesn't appear to have any effect on the fckeditor or htmlarea modules. It doesn't introduce any new hooks. ------------------------------------------------------------------------ March 13, 2005 - 19:56 : killes(a)www.drop.org Guess that is a patch. ------------------------------------------------------------------------ March 14, 2005 - 15:38 : Bèr Kessels +1 From me. Any module that adds something, will want to add it after the title. I really cannot think of any case where aditional content should sit above the title, but I can think of loads of cases where content needs to sit between the title and textarea. ------------------------------------------------------------------------ March 15, 2005 - 01:20 : stefan nagtegaal Indeed, nice and simple patch! +1 from me either.. ------------------------------------------------------------------------ March 15, 2005 - 16:13 : jbond http://www.voidstar.com/node.php?id=2263 [1] If this patch gets accepted I'll drop this into contrib. It's similar to quicktags but *I* like it better. ;-) [1] http://www.voidstar.com/node.php?id=2263 ------------------------------------------------------------------------ March 29, 2005 - 12:09 : TDobes +1... I'm pretty sure the current positioning was accidental anyway... it was always meant to go directly above the textarea. ------------------------------------------------------------------------ March 29, 2005 - 16:33 : Bèr Kessels Dries, All people who use this, seem to agree. Any chance this gets in soon? Bèr ------------------------------------------------------------------------ March 29, 2005 - 18:23 : Dries Attachment: http://drupal.org/files/issues/textarea.patch (1.88 KB) Ber: while all people seem to agree, it is unclear how many people tested/reviewed this patch. One ought to be careful. For example, the patch introduces a new variable, $form_element, without properly initializing it ... How about the attached patch? It is slightly cleaner, slightly faster and properly initializes all variables. I haven't tested it with the WYSIWYG editors but maybe someone else (Julian?) can? Will commit after it has been tested. ------------------------------------------------------------------------ April 3, 2005 - 14:38 : Dries Anyone to test this? ------------------------------------------------------------------------ April 6, 2005 - 09:21 : TDobes Attachment: http://drupal.org/files/issues/textarea_updated.patch (1.97 KB) The patch no longer applied, so I updated it. I also tested it with: * Quicktags * TinyMCE * Areatags (mentioned earlier by jbond) It worked with all three... +1 for applying Dries' cleaner patch. jbond: Your module's browser detection code does not work on my server. You seem to expect a $HTTP_USER_AGENT global variable, which does not exist... I think you should instead be using $_SERVER['HTTP_USER_AGENT']. Perhaps your code was created with the assumption that "register_globals" is turned on in php.ini? That setting is off by default, so I wouldn't rely on it.

1 0