- Development - lists.drupal.org

[drupal-devel] [bug] Putting a .theme file into themes/ makes all themes not working
by chx 07 Mar '05

07 Mar '05

Issue status update for http://drupal.org/node/16914 Project: Drupal -Version: cvs +Version: cvs Component: theme system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: chx Status: patch Any problems? chx Previous comments: ------------------------------------------------------------------------ February 7, 2005 - 08:13 : chx Try putting a .theme file into themes/ without any subdir. Enable it (maybe just saving theme settings is enough), Nothing will work after this. To restore your site: remove the offending theme, issue an SQL: DELETE FROM {system} WHERE type='theme' and save theme settings again. ------------------------------------------------------------------------ February 7, 2005 - 08:14 : chx effects HEAD, too. ------------------------------------------------------------------------ February 7, 2005 - 13:33 : chx Attachment: http://drupal.org/files/issues/themesfix.patch (542 bytes) Here is a patch which fixes the problem. However, I do not have the feeling that this is "the" solution. Feel free to come up with something better. ------------------------------------------------------------------------ February 7, 2005 - 13:35 : chx Oops forgot to set to CVS, 'cos the fix is against HEAD. Not that 4.5.2 does not need a patch... ------------------------------------------------------------------------ February 7, 2005 - 22:15 : Anonymous While this patch would allow a theme to be placed directly in the "themes" directory, it disables the ability for that theme to use a "style.css" file, which is inconsistent. I would prefer a patch that enforces the criteria that themes MUST be placed in a SUBDIRECTORY of "themes" -- themes placed directly in the "themes" folder should be ignored... maybe we could send the admin a drupal_set_message that would tell them about the problem when visiting admin/themes too. ------------------------------------------------------------------------ February 23, 2005 - 00:07 : chx Attachment: http://drupal.org/files/issues/themefix.patch (693 bytes) Anonymous is right. I have made a better patch. Please commit, this is very important IMHO. ------------------------------------------------------------------------ February 23, 2005 - 00:30 : chx Attachment: http://drupal.org/files/issues/themefix_1.patch (712 bytes) Message was wrong. ------------------------------------------------------------------------ March 2, 2005 - 00:26 : chx May I know why this is not getting commited? This is a very nasty issue, the poor site admin who tries a few themes and misplaces one is totally baffled why his themes do not work after this. ------------------------------------------------------------------------ March 2, 2005 - 09:26 : Dries - That should probably be an error message, rather than a status (success) message. - The coding style needs work. - For consistency the theme name should be wrapped in em's. - I'd prefer not to scan ./themes for .theme files and to ignore any .theme files places directly in ./themes. Won't commit as is. ------------------------------------------------------------------------ March 2, 2005 - 12:37 : chx - That should probably be an error message, rather than a status (success) message. Ah, I was never aware of the second parameter of drupal_set_message! One always learns. - The coding style needs work. Spaces, my doom :( - For consistency the theme name should be wrapped in em's. Understood. - I'd prefer not to scan ./themes for .theme files and to ignore any .theme files places directly in ./themes. Well. Only a system_listing call happens before my patch in system_theme_data. All system_listing does it sets up a maximum of two starting points for a recursive file_scan_directory ($directory and $config/$directory) and file_scan_directory has no options for such an exclusion and it does not seem appropriate to change file_scan_directory only for this. Surely I miss something in here, but I still think that this special thing shall be done in system_theme_data. If this is accepted, the minor (from a coding point of view they are minor) issues will be corrected in no time. ------------------------------------------------------------------------ March 5, 2005 - 20:14 : chx Attachment: http://drupal.org/files/issues/themefix_2.patch (3.2 KB) This is a whole new approach. Some credits: to nsk, because he triggered the bug and bought me Shadow of The Giant for fixing his site, but I took that as a sponsoring to fix this bug. To Steven for the $depth-as-argument and to Dries 'cos of "sensible defaults". We do not throw warning this time 'cos system_theme_data is not even aware of the faulty themes. ------------------------------------------------------------------------ March 5, 2005 - 20:26 : chx Attachment: http://drupal.org/files/issues/themefix_3.patch (3.31 KB) PHPdoc and style fix ------------------------------------------------------------------------ March 5, 2005 - 20:48 : chx Attachment: http://drupal.org/files/issues/themefix_4.patch (3.29 KB) ------------------------------------------------------------------------ March 5, 2005 - 21:13 : chx Attachment: http://drupal.org/files/issues/themefix_5.patch (3.27 KB) ------------------------------------------------------------------------ March 6, 2005 - 11:12 : Dries Can't you perform the $depth <= $max_depth check before you call file_scan_directory()? Looks like you could move the check down to the if-clause surrounding the call to file_scan_directory(): <?php if (is_dir("$dir/$file") && $recurse) { ?> ------------------------------------------------------------------------ March 6, 2005 - 11:31 : chx Attachment: http://drupal.org/files/issues/themefix_6.patch (3.26 KB) Ah, you are proposing that we do not even call file_scan_directory when we are too deep? That's a wise idea, as usual from you ;) However, it can not be in the if-clause surrounding the call to file_scan_directory(), 'cos if you are expecting files, and hit max_depth, you may get some directories too. So it got a separate if.

1 0

[drupal-devel] [bug] Throttle doesn't need hook_perm anymore
by Anonymous 07 Mar '05

07 Mar '05

Issue status update for http://drupal.org/node/18491 Project: Drupal Version: cvs.tar Component: throttle.module Category: bug reports Priority: minor Assigned to: Anonymous Reported by: Anonymous Updated by: Anonymous Status: patch Attachment: http://drupal.org/files/issues/throttle_perm.patch (579 bytes) Since the throttle block has been removed from the throttle module, there is no longer a need for the "access throttle block" permission. Attached patch removes the throttle_perm function. Anonymous

5 6

[drupal-devel] [feature] 2 additional hooks to form_textarea to support module writers who are extending it.
by jbond 07 Mar '05

07 Mar '05

Issue status update for http://drupal.org/node/18417 Project: Drupal Version: 4.5.0 Component: base system Category: feature requests Priority: normal Assigned to: Anonymous Reported by: jbond Updated by: jbond Status: patch Well after much faffing around, I've managed to attach event handlers to the textarea. Hooray! It turns out that IE6 and possibly firefox will only do this in a body onLoad function. And the attachEvent and addEventListener calls take a function pointer not a string so there's no way to pass parameters in. So my handler functions needed to find out what they were being called from. So that gets rid of my need for one of my suggested hooks. The second one is the same as Ber's request for hook_textarea('pre') to be between the title and textarea. I can't see at the moment any way of reversing these in javascript, except possibly getting the textarea, getting it's parent, getting all the child nodes, walking down the list and deleting, appending my toolbar. This again seems an awful lot of work compared with moving the hook. I'll happily submit a patch to do this. but I won't bother if it's just going to be chucked out. I'm tempted to say that the post hook should be between the textarea and the $desc as well but this would break the HTMLarea module. ps. Although I've learnt things in this process, it still seems to me to be a bit perverse to force us to work like this. form_item has an $attributes parameter. So does form_textarea. But there's no way for hook_textarea to get in there and add a $attributes entry. There you go. moan over. jbond Previous comments: ------------------------------------------------------------------------ March 5, 2005 - 09:54 : jbond Attachment: http://drupal.org/files/issues/common.inc_8.patch (1.83 KB) This patch is for common.inc function form_textarea(). It adds two additional hooks for module writers. 1) A new hook "post-title" which works the same as "pre" and "post" but allows html to be inserted between the title of the element and the textarea. This makes it easier to insert a row of buttons (say) that sit just on top of the textarea. 2) A new hook "attributes" which takes an array() from modules that provide it and merges it into the $attributes array for the textarea. Wysywig editors typically need to add javascript handlers for things like onClick and this provides a simple way for the module to add them. ------------------------------------------------------------------------ March 5, 2005 - 10:00 : jbond Attachment: http://drupal.org/files/issues/common.inc_9.patch (1.83 KB) Apologies. the first patch had ms win line endings. This one should be unix line endings. ------------------------------------------------------------------------ March 6, 2005 - 09:17 : gordon -1 I do not think that these 2 additional hooks are needed. This will really add alot of additional overhead as it is going to call every textarea hook 4 times for every textarea on a page. Besides these modifiaction can be handled better and cleaner in the javascript. If you take a look at Xinha/HTMLArea or FckEditor they both manipulate the dom once the page has loaded, and not during. This is a much better and cleaner method as it means that you can attach to any textarea with only a few commands. This is what javascript excels at. So I don't think this is needed. ------------------------------------------------------------------------ March 6, 2005 - 10:03 : Dries I was about to suggest the same as Gordon. IMO, these hooks aren't really necessary. ------------------------------------------------------------------------ March 6, 2005 - 12:04 : Bèr Kessels -1 from me too. More hookss are unnecasary. What *is* neccesary, thoiugh, is to mve the "pre" to inbetween the title and the area! ------------------------------------------------------------------------ March 6, 2005 - 12:23 : jbond I'd agree with you, perhaps, about the attributes tag. I don't think I agree about "post-title". I think it's highly likely that a module would use "pre" or "post- title" but not both. And since these are all in memory hooks I hardly see that calling 4 module hooks instead of 2 is going to make any performance difference at all, at all. Following your logic there's no need for "pre" either, because you can implement that in javascript from "post". Better and cleaner? I find php a hell of a lot easier to code and debug in than Javascript. But then that's just me. ------------------------------------------------------------------------ March 6, 2005 - 12:32 : chx -1 the way JS should be that you insert an onload handler and there is no other trace of JS in your code. use DOM. ------------------------------------------------------------------------ March 6, 2005 - 12:59 : gordon When the textarea hook was first implemented it was only used in the post. But using the pre for functions other then javascript wil allow you to place html above the title, which maybe wanted, depending on the use. Javascript is the primary use, but I think there is other things that use it. As for debugging javascript, this is made extremely easy, actually easier than php by itself with the use of firefox/mozilla and venkman [1] the javascript debugger. [1] http://www.mozilla.org/projects/venkman/ ------------------------------------------------------------------------ March 6, 2005 - 13:31 : Bèr Kessels I meant "Post title" INSTEAD of pre. So: hook_textare('pre'): TITLE ------------------------------------------------------------------------ March 6, 2005 - 13:32 : Bèr Kessels hrm, my cool ascii art example got lost. :( Hoever: i meant that we should change the behavious of form pre, to be rendered after the TITLE. and *not* to introduce a third form hook. ------------------------------------------------------------------------ March 6, 2005 - 16:49 : tangent See my patch in this issue [2] for an example of how to add event handlers. [2] http://drupal.org//node/16204 ------------------------------------------------------------------------ March 6, 2005 - 17:19 : jbond I wonder if there's a compromise in here. The trick is to get hook_textarea('pre') to get new attributes[] back into form_textarea so they can be merged in. - Global $attributes //Yuk! - return array($html, $attributes); //Will break existing uses. Yuk again! Others? Secondly, my efforts to set event handlers in javascript are not working in IE6. I need to end up with onClick = "setCaret(this)" in the textarea definition. ta.addEventListener('select', storeCaret(this), false); doesn't seem to work. neither does ta.onclick = setCaret(this); ------------------------------------------------------------------------ March 7, 2005 - 14:03 : Steven When specifying event handlers, you need to either specify function names, or strings which are pieces of code: ta.onclick = "setCaret(this)"; or ta.onclick = setCaret; But not: ta.onclick = setCaret(this); This last piece of code will in fact execute setCaret(this) and assign the return value to ta.onclick, which is not what you want. JavaScript is really quite logical and a powerful language, especially when it comes to run-time method definitions.

1 0

[drupal-devel] [bug] Line break converter and preformatted text
by mathias 07 Mar '05

07 Mar '05

Issue status update for http://drupal.org/node/12369 Project: Drupal Version: cvs.tar Component: filter.module Category: bug reports Priority: minor Assigned to: Anonymous Reported by: aam Updated by: mathias Status: patch +1 I tested this patch and it indeed prevents content inside <pre> tags from being double-spaced when the line break convertor is enabled. In otherwards, <pre> works as expected now. mathias Previous comments: ------------------------------------------------------------------------ October 31, 2004 - 12:03 : aam The line break converter damages preformatted text in the pre html clause. It converts line breaks into HTML (i.e. br and p tags) and puts therefore additional line breaks in to pre clause of the resulting page. The pre html clause is allowed by default in drupal installation (filtered html), so it does not "work as designed" in this case... ------------------------------------------------------------------------ November 6, 2004 - 11:04 : aam Okay, I understand the new input format and filter system better now, but it is still a nice to have feature. ------------------------------------------------------------------------ March 6, 2005 - 22:29 : mathias It'd be nice to have the line break convertor respect <pre> tags for the 4.6 release. ------------------------------------------------------------------------ March 7, 2005 - 07:38 : Steven Attachment: http://drupal.org/files/issues/pre.patch (4.46 KB) Give the attached patch a whirl.

1 0

[drupal-devel] CivicGroups preview
by Dan Robinson 07 Mar '05

07 Mar '05

Hi Folks, Well we finally have the basics working on the CivicGroups/ForumMail module. ForumMail works with Mailhandler, listhandler and (curently ezmlm) to deliver a more tightly integrated listserve/forum experience. There is a minimalist site up at http://dev.civicactions.net/cg1/. There is some more discussion at the site wrt what the features are and how it is implemented. It would be great if some folks could go over there and kick the tires. Once you get an account you will be automatically subscribed to a mailing list attached to the forum. Currently there is only one forum called "Discussion" visible from the home page. You can control the mail delivery by editing your profile and going to the "my forum mail settings" tab. We are cleaning up some bugs and adjusting the code a little and will shortly release what we've got. Regards, Dan CivicActions, LLC

7 15

[drupal-devel] [feature] 2 additional hooks to form_textarea to support module writers who are extending it.
by Steven 07 Mar '05

07 Mar '05

Issue status update for http://drupal.org/node/18417 Project: Drupal -Version: 4.5.2 +Version: 4.5.0 Component: base system Category: feature requests Priority: normal Assigned to: Anonymous Reported by: jbond Updated by: Steven Status: patch When specifying event handlers, you need to either specify function names, or strings which are pieces of code: ta.onclick = "setCaret(this)"; or ta.onclick = setCaret; But not: ta.onclick = setCaret(this); This last piece of code will in fact execute setCaret(this) and assign the return value to ta.onclick, which is not what you want. JavaScript is really quite logical and a powerful language, especially when it comes to run-time method definitions. Steven Previous comments: ------------------------------------------------------------------------ March 5, 2005 - 10:54 : jbond Attachment: http://drupal.org/files/issues/common.inc_8.patch (1.83 KB) This patch is for common.inc function form_textarea(). It adds two additional hooks for module writers. 1) A new hook "post-title" which works the same as "pre" and "post" but allows html to be inserted between the title of the element and the textarea. This makes it easier to insert a row of buttons (say) that sit just on top of the textarea. 2) A new hook "attributes" which takes an array() from modules that provide it and merges it into the $attributes array for the textarea. Wysywig editors typically need to add javascript handlers for things like onClick and this provides a simple way for the module to add them. ------------------------------------------------------------------------ March 5, 2005 - 11:00 : jbond Attachment: http://drupal.org/files/issues/common.inc_9.patch (1.83 KB) Apologies. the first patch had ms win line endings. This one should be unix line endings. ------------------------------------------------------------------------ March 6, 2005 - 10:17 : gordon -1 I do not think that these 2 additional hooks are needed. This will really add alot of additional overhead as it is going to call every textarea hook 4 times for every textarea on a page. Besides these modifiaction can be handled better and cleaner in the javascript. If you take a look at Xinha/HTMLArea or FckEditor they both manipulate the dom once the page has loaded, and not during. This is a much better and cleaner method as it means that you can attach to any textarea with only a few commands. This is what javascript excels at. So I don't think this is needed. ------------------------------------------------------------------------ March 6, 2005 - 11:03 : Dries I was about to suggest the same as Gordon. IMO, these hooks aren't really necessary. ------------------------------------------------------------------------ March 6, 2005 - 13:04 : Bèr Kessels -1 from me too. More hookss are unnecasary. What *is* neccesary, thoiugh, is to mve the "pre" to inbetween the title and the area! ------------------------------------------------------------------------ March 6, 2005 - 13:23 : jbond I'd agree with you, perhaps, about the attributes tag. I don't think I agree about "post-title". I think it's highly likely that a module would use "pre" or "post- title" but not both. And since these are all in memory hooks I hardly see that calling 4 module hooks instead of 2 is going to make any performance difference at all, at all. Following your logic there's no need for "pre" either, because you can implement that in javascript from "post". Better and cleaner? I find php a hell of a lot easier to code and debug in than Javascript. But then that's just me. ------------------------------------------------------------------------ March 6, 2005 - 13:32 : chx -1 the way JS should be that you insert an onload handler and there is no other trace of JS in your code. use DOM. ------------------------------------------------------------------------ March 6, 2005 - 13:59 : gordon When the textarea hook was first implemented it was only used in the post. But using the pre for functions other then javascript wil allow you to place html above the title, which maybe wanted, depending on the use. Javascript is the primary use, but I think there is other things that use it. As for debugging javascript, this is made extremely easy, actually easier than php by itself with the use of firefox/mozilla and venkman [1] the javascript debugger. [1] http://www.mozilla.org/projects/venkman/ ------------------------------------------------------------------------ March 6, 2005 - 14:31 : Bèr Kessels I meant "Post title" INSTEAD of pre. So: hook_textare('pre'): TITLE ------------------------------------------------------------------------ March 6, 2005 - 14:32 : Bèr Kessels hrm, my cool ascii art example got lost. :( Hoever: i meant that we should change the behavious of form pre, to be rendered after the TITLE. and *not* to introduce a third form hook. ------------------------------------------------------------------------ March 6, 2005 - 17:49 : tangent See my patch in this issue [2] for an example of how to add event handlers. [2] http://drupal.org//node/16204 ------------------------------------------------------------------------ March 6, 2005 - 18:19 : jbond I wonder if there's a compromise in here. The trick is to get hook_textarea('pre') to get new attributes[] back into form_textarea so they can be merged in. - Global $attributes //Yuk! - return array($html, $attributes); //Will break existing uses. Yuk again! Others? Secondly, my efforts to set event handlers in javascript are not working in IE6. I need to end up with onClick = "setCaret(this)" in the textarea definition. ta.addEventListener('select', storeCaret(this), false); doesn't seem to work. neither does ta.onclick = setCaret(this);

1 0

[drupal-devel] [bug] Line break converter and preformatted text
by Steven 07 Mar '05

07 Mar '05

Issue status update for http://drupal.org/node/12369 Project: Drupal Version: cvs.tar Component: filter.module Category: bug reports Priority: minor Assigned to: Anonymous Reported by: aam Updated by: Steven -Status: active +Status: patch Attachment: http://drupal.org/files/issues/pre.patch (4.46 KB) Give the attached patch a whirl. Steven Previous comments: ------------------------------------------------------------------------ October 31, 2004 - 19:03 : aam The line break converter damages preformatted text in the pre html clause. It converts line breaks into HTML (i.e. br and p tags) and puts therefore additional line breaks in to pre clause of the resulting page. The pre html clause is allowed by default in drupal installation (filtered html), so it does not "work as designed" in this case... ------------------------------------------------------------------------ November 6, 2004 - 18:04 : aam Okay, I understand the new input format and filter system better now, but it is still a nice to have feature. ------------------------------------------------------------------------ March 7, 2005 - 05:29 : mathias It'd be nice to have the line break convertor respect <pre> tags for the 4.6 release.

1 0

[drupal-devel] [task] Organize block admin page by region
by drumm 07 Mar '05

07 Mar '05

Issue status update for http://drupal.org/node/18382 Project: Drupal Version: cvs Component: block.module Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: drumm Status: patch Attachment: http://drupal.org/files/issues/block.module_6.diff (3.44 KB) I got tired of hunting through the block admin page for the block I wanted so I went and changed so it is organized by left region, right region, and disabled. Looks a lot like the permissions admin page, especially since it uses the same styles. And there were two copies of this $row = array(...); chunk, which was a bit weird. So that gets removed too. drumm

13 23

[drupal-devel] [bug] Nodeapi settings hook is broken
by chx 07 Mar '05

07 Mar '05

Issue status update for http://drupal.org/node/18216 Project: Drupal Version: cvs Component: node.module Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: chx Status: patch Attachment: http://drupal.org/files/issues/nodeapi_settings.patch (2.49 KB) I have noticed this morning that hook_nodeapi is no longer called by $node as first argument as first setting, but only $type. I documented this in the handbook, 'cos I have thought this is well thought out decision which is executed through the core. But only node.module is updated, comment and upload is broken and every node module out there. I know we do not have backwards compatibility. I think it would be better to introduce a new hook for this. But, as we are in feature freeze, I doubt any of this shall happen. Instead, for now let's change the behaviour back. chx

2 3

[drupal-devel] CivicGroups preview
by Dan Robinson 07 Mar '05

07 Mar '05

Hi Folks, Well we finally have the basics working on the CivicGroups/ForumMail module. ForumMail works with Mailhandler, listhandler and (curently ezmlm) to deliver a more tightly integrated listserve/forum experience. There is a minimalist site up at http://dev.civicactions.net/cg1/. There is some more discussion at the site wrt what the features are and how it is implemented. It would be great if some folks could go over there and kick the tires. Once you get an account you will be automatically subscribed to a mailing list attached to the forum. Currently there is only one forum called "Discussion" visible from the home page. You can control the mail delivery by editing your profile and going to the "my forum mail settings" tab. We are cleaning up some bugs and adjusting the code a little and will shortly release what we've got. Regards, Dan CivicActions, LLC

1 0