September 2005 - Development - lists.drupal.org

[drupal-devel] Drupal Developer Releases
by Harry Slaughter 11 Sep '05

11 Sep '05

I'd like to create a discussion about creating developer releases of Drupal. Sepeck suggested I mail the list with a pointer to my related post: http://drupal.org/node/30903 Thanks Harry (hlslaughter) -- Harry Slaughter <-> harry(a)slaughters.com Home: 619-303-5952 Cell: 619-249-8780

2 1

[drupal-devel] [bug] theme_image() doesn't actually output width and height of image
by m3avrck 11 Sep '05

11 Sep '05

Issue status update for http://drupal.org/node/30935 Post a follow up: http://drupal.org/project/comments/add/30935 Project: Drupal Version: cvs Component: theme system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: m3avrck Updated by: m3avrck Status: patch (ready to be committed) Attachment: http://drupal.org/files/issues/drupal_13.patch (2.75 KB) Fixed a spacing issue. m3avrck Previous comments: ------------------------------------------------------------------------ Sun, 11 Sep 2005 16:22:28 +0000 : m3avrck Function theme_image() doesn't actually return a width and height for an image like it claims to do. ------------------------------------------------------------------------ Sun, 11 Sep 2005 19:30:35 +0000 : m3avrck Attachment: http://drupal.org/files/issues/drupal_11.patch (2.74 KB) Ok patched attached, which fixes this issue. Also, included a patch for system.module which sets the screen shots to 'TRUE' so image dimensions will also be outputted there as well (which they should be!). ------------------------------------------------------------------------ Sun, 11 Sep 2005 19:33:41 +0000 : m3avrck Attachment: http://drupal.org/files/issues/drupal_12.patch (2.75 KB) Fixed a tab issue.

1 0

[drupal-devel] [bug] theme_image() doesn't actually output width and height of image
by m3avrck 11 Sep '05

11 Sep '05

Issue status update for http://drupal.org/node/30935 Post a follow up: http://drupal.org/project/comments/add/30935 Project: Drupal Version: cvs Component: theme system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: m3avrck Updated by: m3avrck -Status: patch (code needs review) +Status: patch (ready to be committed) Attachment: http://drupal.org/files/issues/drupal_12.patch (2.75 KB) Fixed a tab issue. m3avrck Previous comments: ------------------------------------------------------------------------ Sun, 11 Sep 2005 16:22:28 +0000 : m3avrck Function theme_image() doesn't actually return a width and height for an image like it claims to do. ------------------------------------------------------------------------ Sun, 11 Sep 2005 19:30:35 +0000 : m3avrck Attachment: http://drupal.org/files/issues/drupal_11.patch (2.74 KB) Ok patched attached, which fixes this issue. Also, included a patch for system.module which sets the screen shots to 'TRUE' so image dimensions will also be outputted there as well (which they should be!).

1 0

[drupal-devel] [bug] theme_image() doesn't actually output width and height of image
by m3avrck 11 Sep '05

11 Sep '05

Issue status update for http://drupal.org/node/30935 Post a follow up: http://drupal.org/project/comments/add/30935 Project: Drupal Version: cvs Component: theme system Category: bug reports Priority: normal Assigned to: Anonymous Reported by: m3avrck Updated by: m3avrck -Status: active +Status: patch (code needs review) Attachment: http://drupal.org/files/issues/drupal_11.patch (2.74 KB) Ok patched attached, which fixes this issue. Also, included a patch for system.module which sets the screen shots to 'TRUE' so image dimensions will also be outputted there as well (which they should be!). m3avrck Previous comments: ------------------------------------------------------------------------ Sun, 11 Sep 2005 16:22:28 +0000 : m3avrck Function theme_image() doesn't actually return a width and height for an image like it claims to do.

1 0

[drupal-devel] [bug] Fix for PHP5 notices: undefined vars, etc...
by m3avrck 11 Sep '05

11 Sep '05

Issue status update for http://drupal.org/node/30930 Post a follow up: http://drupal.org/project/comments/add/30930 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: m3avrck Reported by: m3avrck Updated by: m3avrck -Status: active +Status: patch (code needs review) Attachment: http://drupal.org/files/issues/drupal_10.patch (20.18 KB) Ok guys here is Phase1 of the PHP5 megapatch. This fixes roughly 60-70% of all the warnings/notices/etc present with PHP5 error_reportin(E_ALL) in place. Please look over comment and let's get this into HEAD ASAP! I *do not* want to reroll this patch if it gets out of date :) I've also tried not to patch certain core modules waiting on bigger patches (e.g., filter module). So please, comment now and let's get this in right now :) I'll continue to provide additionally phases of this patch till we can get all issues cleared up. Thanks! m3avrck Previous comments: ------------------------------------------------------------------------ Sun, 11 Sep 2005 15:11:48 +0000 : m3avrck Ok guys I'm currently working on a patch that erradicates all of the PHP5 notices and warnings from the main Drupal core. Expect to see a patch soon, the PHP5 mega patch, that is ;) ------------------------------------------------------------------------ Sun, 11 Sep 2005 15:13:45 +0000 : m3avrck This patch incorporates this patch: http://drupal.org/node/28948 And addresses the issues brought up with this patch: http://drupal.org/node/30784 A 4.6.3 version can be found here: http://drupal.org/node/30800 ------------------------------------------------------------------------ Sun, 11 Sep 2005 15:21:13 +0000 : m3avrck This patch also fixes this bug (as discovered through an undefined variable): http://drupal.org/node/30807 " m3avrck hey anyone familiar with node_access_view_all_nodes() ... i think i found a bug as i'm going through cleaning up undefined variables drumm m3avrck: I think jonbob, but he is still gone m3avrck yeah definetly a bug m3avrck yeah drumm just created this issue: http://drupal.org/node/30807 drumm I'm guessing $node->nid can simply be taken out. I'm not sure a %d or %s will ever make it's way into $sql chx drumm: re. that node access problem and node->nid , it's a minor bug -- the query has no arguments thus needs no arguments. chx it's not drumm it's m3avrck chx m3avrck: so, roll a patch which removes that $node->nid m3avrck ok thanks chx chx I think the argument was removed during the fix of the so-called DrupalCon bug :) which moshe pointed out to me and JonBob and me have fixed on site. Real life debugging with those guys were real fun! chx I am not totally sure though -- but it was related to this function for sure " ------------------------------------------------------------------------ Sun, 11 Sep 2005 16:13:59 +0000 : m3avrck This patch also fixes a somewhat incorrect use of PHP getimagesize() : http://us2.php.net/manual/en/function.getimagesize.php ... none of the $attr from list() = getimagesize() are ever used (couldn't find a example when these ever would be used), hence they are always undefined. Additionally, we don't even make use of the $width and $height vars that we define already, this is fixed in this patch as well. ------------------------------------------------------------------------ Sun, 11 Sep 2005 16:23:17 +0000 : m3avrck Looks like that getimagesize() is outside the scope of this patch now, created a seperate issue with patch (coming soon!): http://drupal.org/node/30935

1 0

[drupal-devel] [bug] comment preview "Required" is easily bypassed
by Abalieno 11 Sep '05

11 Sep '05

Issue status update for http://drupal.org/node/28420 Post a follow up: http://drupal.org/project/comments/add/28420 Project: Drupal Version: cvs Component: comment.module Category: bug reports Priority: normal Assigned to: Jeremy(a)kerneltrap.org Reported by: Jeremy(a)kerneltrap.org Updated by: Abalieno Status: patch (code needs review) It's is for cvs but I'm trying to manually apply it to 4.6.3. Will comment later to tell how it went. Abalieno Previous comments: ------------------------------------------------------------------------ Mon, 08 Aug 2005 01:55:34 +0000 : Jeremy(a)kerneltrap.org Setting "Preview comment" to "Required" does not strictly require that the comment be previewed first. This is being abused by spammers to quickly and efficiently post spam comments. I discovered this after I added a new feature to my new spam module [1] to auto-blacklist spammer IP addresses, allowing me to block comment spammers when they preview a comment and thus preventing them from ever inserting their spam into my database. I configured my comment module to "require" comment previews, and yet found that the comments were slipping past my filter. I finally realized what the spammer is doing is setting $_POST['op'] to 'Post comment', effectively bypassing the preview phase. I'm currently looking for a clean solution to this. At the moment the only idea I have is to generate a token at the preview phase, and validate the token at the post phase. Unfortunately the token would have to be stored in the databse between the preview and the post, which adds overhead. Alternatively, I've considered using a time-based hash which would constantly update depending on the time of day. This could easily be validated without storing anything in the database. If too long has gone between the preview and the post, an additional preview step would be required... The down side here is that the time-based hash would be publically available, and thus the spammer could easily duplicate it in their script. A private key could solve for that, but increases the complexity as it adds a configuration step. I have the feeling I'm missing a simpler, cleaner solution. Suggestions? [1] http://kerneltrap.org/jeremy/drupal/spam/ ------------------------------------------------------------------------ Mon, 08 Aug 2005 02:26:21 +0000 : moshe weitzman even if you get this fixed, won't these bots just add a preview step? this 'preview required' feature is designed to maintain high quality submissions by forcing users to proof read. it isn;t designed for security. i think you want to hook into comment_validate(). just add a hook here - there is already a hook_comment() waiting for you to add an operation. ------------------------------------------------------------------------ Mon, 08 Aug 2005 08:49:30 +0000 : Eaton I posted a patch a few days ago (http://drupal.org/node/28255) that adds validation and form construction hooks for comments. It's similar to the one that the captcha module uses, though it adds comment form_pre and form_post hooks instead of a single comment form hook. ------------------------------------------------------------------------ Mon, 08 Aug 2005 13:30:34 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_11.patch (2.5 KB) > even if you get this fixed, won't these bots just add a preview step? Eventually, yes, but it drastically changes their ability to fling spam at a site. As is, they simply have a script that shoots data out at high speed without having to wait for messages to return from the server. It is the server that is doing all the work, thus making it simple for a spammer to DoS a site. If "preview required" really meant "preview required", they would be forced to first automate clicking "preview", and then wait for a response before clicking "submit". This requires more resources on their side, and allows us to add delays after clicking "preview" (if we detect that they are a spammer) further using their resources. > this 'preview required' feature is designed to maintain high quality > submissions by forcing users to proof read. it isn;t designed for security. Regardless of the intention, I was misled to believe that configuring my site to require previews would require that all comments were first previewed. As a site administrator, I would prefer to know that "required" really means "required". > i think you want to hook into comment_validate(). just add a hook here - > there is already a hook_comment() waiting for you to add an operation. Yes and no. Ultimately yes that will work and will allow my spam module to prevent the spam from ever being posted. But it still leaves the greatest burden on the web server, instead of on the spammer. The spammer can still use a very simple script that only pushes data, and thus can generate spam at an unbelievable rapid rate. Here is an example patch to enforce "preview required". It's one idea, I'm sure there are better ones. ------------------------------------------------------------------------ Mon, 08 Aug 2005 14:01:38 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_12.patch (1.4 KB) Here's a second version of the patch that doesn't require any manual configuration. ------------------------------------------------------------------------ Mon, 08 Aug 2005 16:27:27 +0000 : Jose A Reyero I like this idea, and the patch looks good Still, I think it misses something, like some timestamp related hash, because once you get the hash code you can post multiple comments with that. Another problem I can think of is, what happens when a cron run happens between the preview and the post?? I'm afraid comments would get lost For this second problem, I think a key generated only once after module activation could do. About the first one....mmm... I'll sit down for a while and think..... ------------------------------------------------------------------------ Tue, 09 Aug 2005 12:27:20 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_13.patch (1.33 KB) > I think it misses something, like some timestamp related hash, because > once you get the hash code you can post multiple comments with that. Using a timestamp will mean that the comment form "expires". That is, if you wait too long to preview your comment, it will generate an error when you try to post. Yes, technically a spammer could post one real comment, then based on what was in the session from that they could post the same identical comment over and over, so long as it was attached to the same node. But this is not what they do, they try and spread their spam throughout your webpage. Furthermore, the spam module is perfectly capable of detecting and preventing this. > Another problem I can think of is, what happens when a cron run happens > between the preview and the post?? I'm afraid comments would get lost The key is only generated once, that's what the first test is about. In any case, in the unlikely event that the key were to change between preview and post they would simply have to post a second time. My earlier patch wasn't quite right, I was testing the token in the wrong place. This patch fixes that. BTW: This is beneficial for maintaining high quality submissions too, as prior to this change someone could: 1) enter a comment 2) press preview 3) completely change their comment (introducing a mistake) 4) press submit and the comment (mistake and all) would go into the database unpreviewed After this change: 1) enter a comment 2) press preview 3) completely change their comment (introducing a mistake) 4) press submit and they get an error because they didn't preview their changes - forcing them to preview once after any change ------------------------------------------------------------------------ Wed, 10 Aug 2005 03:50:33 +0000 : Jeremy(a)kerneltrap.org FWIW: I've been getting slammed by spam attacks this whole week. Installing this patch has made a huge difference. Well over 100 spam attempts per minute (sometimes two and three times that) and I hardly notice the spammer, whereas before it was choking my database. (Granted, the spammer has not yet upgraded his script to first preview, then submit. But even if he did it wouldn't help him as testing has verified that the new spam module would prevent the comments from ever getting to the database.) Additionally, user and anonymous (nonspam) comments continue to show up at a normal rate. ------------------------------------------------------------------------ Tue, 16 Aug 2005 14:08:04 +0000 : Jeremy(a)kerneltrap.org I would love to see _any_ discussion on this. Drupal is currently too easy to spam, with little effort on the spammer's side, and lots of resources wasted on the Drupal side. A patch like this will greatly increase the spammer's burden, and make it possible to effectively block even the most aggressive spammer attacks. ------------------------------------------------------------------------ Wed, 17 Aug 2005 16:24:04 +0000 : Jose A Reyero Well, this patch is definitely better than what we have, and would save some spam for sure. But maybe keeping track, at the session level, of generated hashes for a user, and then removing them when the comment is sent, could do the work. This way we can forget about previewing comments or not, and also the "permission" to post the comment would expire when the session expires. Any randomly generated value could do for this, no need for complex hashes, but having nid and pid in the hash would add some extra security. ------------------------------------------------------------------------ Wed, 17 Aug 2005 19:58:02 +0000 : breyten Jeremy, a big +1 on the idea, but why not generate the private key when it is actually needed (Ie, when displaying the comment form), instead of wasting a _cron() hook on it? ------------------------------------------------------------------------ Thu, 18 Aug 2005 03:20:02 +0000 : Jeremy(a)kerneltrap.org > Well, this patch is definitely better than what we have, and would save some spam for sure. It is continuing to work very well on my site, which seems to be under nearly perpetual spam attacks from multiple sources. > But maybe keeping track, at the session level, of generated hashes for a user, and then > removing them when the comment is sent, could do the work. The catch is: the key has to be something unique to the server, not guessable or learnable from the outside Simply storing the hash data in the session alone is not enough, as then the spammer could create any random data and store it in the session. That said, the hash could be generated off something other than the text of the comment as it is now, so that a preview is not required. I'll look at doing something like that and submit another patch. > This way we can forget about previewing comments or not, and also the "permission" to > post the comment would expire when the session expires. Any randomly generated value > could do for this, no need for complex hashes, but having nid and pid in the hash would > add some extra security. nid and pid alone are worthless, as they are easy to learn. The pid can always be 0 (spam is rarely attached to a pre-existing comment). The nid is obtained in the path of where the spam is being posted. The solution is a "private-key", which is what my patch adds. Then sure, hash the private key plus the nid and the pid, and you've got enough protection to prevent most spammers. To make it even more secure, automatic rey-keying could be easily accomplished. ------------------------------------------------------------------------ Thu, 18 Aug 2005 04:09:10 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_15.patch (1.18 KB) The attached patch: 1) gets rid of the _cron() hook 2) no longer requires that comments be previewed Prior to this patch, comment spammers were able to send data to a Drupal server acting as though they'd filled out a comment form and pressed submit. As they didn't actually use the form, they could submit spam comments at an obscene rate. With this patch, comment spammers will have to actually load the form, enter text, and press submit. Yes, that can still be automated, but it takes much more work and slows them down, as they have to wait for the entry form to load each time. Unfortunately a spammer could manually submit one comment, then re-use that same session info over and over to attach repeated spam comments to the same node. Such an attempt would be detected and blocked by the spam module if enabled, but again such a session re-use attack could be done without loading the form each time. Fortunately there is much less gain for a spammer to submit 100 spam comments on the same page, versus submitting 100 spam comments each on a different page as they do now. Ideas to improve upon this concept include: - re-key every day or week, changing the private key regularly to be sure it couldn't ever be permanently cracked - add a key table, and generate a unique key for every comment form. essentially, upon comment form creation generate a random key which is stored both in a database table and in the session. when a comment is submited, look for the key from the session in the database table, if a match is found delete it from the database table and post the comment. this would prevent session re-use, but adds overhead. i don't know if it's worth it, perhaps as an external module if the hooks were available. ------------------------------------------------------------------------ Fri, 19 Aug 2005 18:55:11 +0000 : drumm <?php form_set_error('error', t('Validation error, please be sure cookies are enabled on your browser.')); ?> form_set_error [2]()'s fist argument should be the name of a form field, not 'error.' Using (..., 'error') would be better in this case. And the actual message needs work. Since this is a hidden field I don't think it has anything to do with cookies. [2] http://drupaldocs.org/api/head/function/form_set_error ------------------------------------------------------------------------ Fri, 19 Aug 2005 18:56:41 +0000 : drumm The unclosed link in my last update was supposed to say drupal_set_message(..., 'error') ------------------------------------------------------------------------ Sat, 20 Aug 2005 16:00:15 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_16.patch (1.11 KB) drupal_set_message(..., 'error') isn't sufficient to prevent the comment from being posted. I have instead updated the patch to set the error on the hidden 'token' form field. I have updated the message to read: "Unable to validate your comment, please try again. If this error persists, please contact the site administrator." If you don't like the error message, better suggestions are welcome. ------------------------------------------------------------------------ Fri, 09 Sep 2005 03:16:06 +0000 : Jeremy(a)kerneltrap.org Any feedback on this patch? I have been running it on my website for a couple of weeks, and it has completely stopped the most persistent auto-spam scripts that had been posting poker type comments constantly to my site. ------------------------------------------------------------------------ Sat, 10 Sep 2005 18:12:15 +0000 : Zed Pobre This patch is against HEAD? It doesn't want to apply to my 2.6.3 comment.module.

1 0

[drupal-devel] [bug] Deleting Users & validate role name
by kubaZygmunt 11 Sep '05

11 Sep '05

Issue status update for http://drupal.org/node/27951 Post a follow up: http://drupal.org/project/comments/add/27951 Project: Drupal Version: cvs Component: user.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: kubaZygmunt Updated by: kubaZygmunt Status: patch (code needs review) Attachment: http://drupal.org/files/issues/user.module.patch.txt (914 bytes) I've found two bugs in user.module 1. Deleting button in admin section doesn't work ( deleting works only by url user/$id/delete ), I've upadated condition 2. If you change role name to 0 then system deletes role. (I've added isset($edit['name') ) kubaZygmunt

2 2

[drupal-devel] [feature] Enable override and extension of core module functions
by nedjo 11 Sep '05

11 Sep '05

Issue status update for http://drupal.org/node/29428 Post a follow up: http://drupal.org/project/comments/add/29428 Project: Drupal Version: cvs Component: module system Category: feature requests Priority: normal Assigned to: Anonymous Reported by: nedjo Updated by: nedjo Status: patch (code needs review) Attachment: http://drupal.org/files/issues/module-extension-and-override.patch (1.38 KB) A frequently requested functionality is the ability to override or extend core module functions. This small patch is an initial take on how to enable function extension and overriding. It takes the existing module_invoke() function and adds tests for override and extension functions, in the forms modulename_originalfunction_override() and modulename_originalfunction_extension(). So, for example, an override function for the core taxonomy_node_form() function as defined by a module named testmodule would be called testmodule_taxonomy_node_form_override(), and would be run instead of taxonomy_node_form(). And testmodule_taxonomy_node_form_extension() would be run every time taxonomy_node_form() was called, hence "extending" it. Of course, this approach would only be useful if we converted most - or all! - of our current module function calls to use module_invoke()... nedjo

8 15

[drupal-devel] [feature] Enhance hook_settings - add an optional 'advanced' tab
by Bèr Kessels 11 Sep '05

11 Sep '05

Issue status update for http://drupal.org/node/30515 Post a follow up: http://drupal.org/project/comments/add/30515 Project: Drupal Version: cvs Component: system.module Category: feature requests Priority: normal Assigned to: moshe weitzman Reported by: moshe weitzman Updated by: Bèr Kessels Status: patch (code needs review) -1 for the concept. "Advanced" settings are bad UI. It is not up to us to decide what is "advanced" and what not. You also simpley split them up on an imaginary and non-existing border. Tehre is no difference between advanced and simple, so making usch a border is bad. On top of that, "advaced" allows lazy UI design. "I would like to have a better look over how to render these options, and I still need to write better descriptions" vs "hmm, dunno what to do with them yet, lets stick em in advanced". That off course is over siplified, but I hope it exlpains the problem. Please, please, do not introduce "advanced" tabs or areas. No modern environments (desktops, apps etc) have this, for the reason that "advanced" is a non-existing group. (well except FF, but it is a much debated issue, and is considered one of teh top UI mistakes in FF). If this does not convince you people, I will try to find some good online resources, for II only know this from what I read in usability books. Also, when discussing usability issues please just post a screenshot with the patch, or even jsut a mockup before a patch. Usability is *not* about code, thus a patch is not a good method of communication. A mockup also saves a lot of time when developing issues. And last, this, in fact is a duplicate of: http://drupal.org/node/30843 one of these two should be marked as dup. Bèr Kessels Previous comments: ------------------------------------------------------------------------ Tue, 06 Sep 2005 16:05:53 +0000 : moshe weitzman Attachment: http://drupal.org/files/issues/hook_settings.patch (25.19 KB) This patch opens the door for advanced preferences in Drupal. If you have sufficient permissions, you will see an *advanced* tab on settings pages when a module chooses to put stuff there. The permission is global across all Drupal - 'access advanced functionality'. This patch enhances hook_settings so that it accepts a $type parameter. By default, $type='simple' and everything works as today. If modules return HTML when $type=advanced, a new tab appears on the settings page. To see this in action, visit admin/settings/aggregator applying the patch and after giving yourself the permission above. The patch appears long only because of whitespace changes. It is a minor change to all hook_settings() implementations. This 'advanced tab' paradigm can be used outside hook_settings as well. Just protect your tab/feature with the permission above. Contrib modules will continue to work without patching but will show an duplicate tab called 'advanced'. To fix, just add an if($type == 'simple') block in hook_settings(). I will add upgrading instructions to drupal.org and update hook_settings() docs if this is committed. I feel that this is a good balance between those who repeatedly yell for 'fewer settings' and those who want their site to work a specific way but are uneager to hack the source code. This 'advanced settings' paradigm has worked well for Mozilla project (about:config) and many others. If these advanced features clutter and bother you, don't enable this permission or don't click on the advanced tab. There is no way that Drupal becomes a cluttering steaming pile of configuration options - our culture precludes this. Lighten up, and give the people what they want. We can't expect everyone to hack the source to get commonly requested behavior. I also simplified upload config by renaming upload_admin() to upload_settings() and removing an unneeded menu entry. ------------------------------------------------------------------------ Sun, 11 Sep 2005 05:28:51 +0000 : moshe weitzman i'd like to hear some comments from CVS review team on this. If not desired, alternatives are appreciated. ------------------------------------------------------------------------ Sun, 11 Sep 2005 06:10:03 +0000 : Crell This was discussed recently on the -devel mailing list, and for reasons discussed there I have to -1 this in concept. An "Advanced" tab means the UI designer failed to properly design the UI to discourage the user from doing stupid things. That's not something we should encourage. If there are too many configuration options in some module for them to be just shown linearly (I don't actually know many modules with that problem), that's what <fieldset> is for. If some module has enough options that even <fieldset> isn't enough, then rather than hard-coding an "advanced" tab we should allow arbitrary module-developer-defined tabs that all get the extra magic of the settings page. The $delta methodology of hook_block() would be a good model to follow here. If you just want to split some options off as "Danger, Will Robinson!", frankly I can't think of any good examples. Perhaps you could offer some? ------------------------------------------------------------------------ Sun, 11 Sep 2005 14:16:23 +0000 : Jose A Reyero + 1 for the concept I think we are always trying to find what is an imposible balance between ease of use (less settings) and options/flexibility (more settings), so having simple and advanced settings would be a good thing. However, about the implementation, I wouldn't like having an 'advanced' tab, so from an advanced administrator's point of view you would have to guess which settings are in 'simple' and which ones are 'advanced' to find them. I'd rather have a single page having all settings -that would be more if you have that advanced permission. So you can just return all simple settings as deffault, and then a few more if the parameter is 'advanced', and then we'd need to rewrite that settings hook only when additional advanced settings are provided.

1 0

[drupal-devel] [feature] New themable function theme_search_form to unify search form and put logic at the correct level
by robertDouglass 11 Sep '05

11 Sep '05

Issue status update for http://drupal.org/node/27843 Post a follow up: http://drupal.org/project/comments/add/27843 Project: Drupal Version: cvs Component: search.module Category: feature requests Priority: normal Assigned to: robertDouglass Reported by: robertDouglass Updated by: robertDouglass Status: patch Attachment: http://drupal.org/files/issues/make_search_themable.txt (3.99 KB) The search form should be themable, but it shouldn't be left to the theme('page') function to do this, as is the current implementation. This patch adds a theme_search_form function to theme.inc (not search module because that would cause a dependency) and removes the then unneeded variables from the phptemplate.engine. Pushbutton and Bluemarine are updated to take advantage of the new function. In the current implementation, several variables are created needlessly in phptemplate.engine (like the text for the button) and then pushed to the template. This seems messy and this patch fixes that. Remaining questions: should the function accept any parameters? I think not, but other opinions are welcome. robertDouglass

3 2