Issue status update for http://drupal.org/node/16914 Project: Drupal Version: cvs Component: theme system Category: bug reports Priority: critical Assigned to: chx Reported by: chx Updated by: chx Status: patch 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. 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) { ?>