bug in template engine admin

Not really a bug, but files copied from OSX to Linux machines seem to all have files that start with "._" in front of them.

I've been installing Drupal 4.5 from my local OS X to a Linux hosted server all week, without running into this problem.

Which ftp application are you using? Could it be an application setting that's responsible?

I did not manage to replicate the "null" character problem, but many errors appeared once I created an empty ._xtemplate.xhtml file in a any theme's folder. This is because xtemplate_templates uses file_scan_directory($directory, 'xtemplate.xtmpl$'); and the one with ._ comes in first.

Why is this function matching the end of the file name only? Are there legitimate cases where this is needed? I do not think Drupal should be worrying about user's software inserting random files into web-accessible folders, but even some linux editors create helper files with the same name as the one being edited, plus a prefix (well, there is usually a suffix as well, but anyway...) Why not match the entire filename? Or at least when there is more than one, take the closer/exact match?

I am attaching a very small patch which would prevent files such as ._xtemplate.xhtml to be used instead xtemplate.xhtml in the same folder. That is NOT the real solution for this issue, but I would really like to hear from those who know more.

If you apply this patch after you already HAD ._xtemplate.xhtml file in a theme's folder, you will still not be able to use that theme. This patch will only prevent similar problems from appearing in the future - e.g. if ._xtemplate.xhtml suddenly appears in another theme's folder. Now it will be ignored from the start, and it will never get into the system table.

However, if you have such weird files in your theme folders before applying this patch, they will be saved into the system table, and it seems that you can only remove them manually from the database. They do not get removed by Drupal even if you disable the theme itself. I did not try temporarily removing the theme's folder all together. Would that refresh the system table?

This is the point where somebody who actually knows how all this works steps in and sheds some light on the subject :-)

That patch seems to overcomplicate the RE. The change in this patch is simpler and cleaner.

Original regexp was matching any file name that ends with "xtemplate.xhtml". I tried to keep it same, but to skip any file that starts with a dot (as those are usually hidden temporary files etc.) Your patch does an exact match. Does this mean that we always want to do exact matches here? If so, how about matches for "styles.css" elsewhere in the templating code? Shouldn't those also be made exact? Right now they do same thing as the unpatched xtemplate match. This means that ._styles.css or .#styles.css etc. would also get picked up.

Also, I would still like to know why the templates in the system table do not get completely updated when template configuration is saved? E.g. before this patch I have 5 templates due to a rogue ._xtemplate.xhtml file. Now (after the patch) when I enter theme configuration only the real templates are recognized, so it should be back to 4. However, if I submit the theme configuration the old rogue entry stays in the database. I suppose that code that updates this does not expect more than one template per theme, so it just updates one of them (the correct one) and does not touch the rogue one anymore. I will try to make another patch if I find where this stuff happens...

This patch resolves the problem that may occur on systems that do not have the previous patch installed (posted by me in this same thread and improved by jhriggs.) It adds a delete query to system_obtain_theme_info - to remove any 'theme' entries from system table that are not being updated in the current call to this function. I grepped, and it seems that the only place which calls this function is in the same file, and it always calls it with all the available themes.

Notes:

1. There are other ways to do the same thing, and perhaps Drupal already has a different recommended way of doing a query like this (it uses NOT IN, and a join on array of strings.)

2. Perhaps there are non-core modules that also call this function with a more limited set of themes. This patch would break things in that case, so perhaps I can add an optional argument that would only be set in this particular call I know about, so "bogus" theme entries are removed only in this case. Other calls to the same function would not trigger the new delete query.

svemir: Would you mind investigating to see if a similar situation occurs with modules were inserted into the system table then deleted? I noticed this while working on the theme system, but left it untouched because I wasn't sure if it was intentional for some reason or not. (this behavior existed in 4.4 as well)

+1 for jhriggs' patch. A similar fix has already made it into CVS HEAD with the multi-site patch.

Yes, the same thing happens with modules in both regards, but it is much less likely to cause problems there. The first issue (matching anything that ends with .module grabs also .#book.module, for example) will only create problems if you try to enable such module. It will most likely have the same function names etc. as the real book.module, so you get function redeclared errors etc. Removing the temporary file solves the problem.

Such removed modules (whether bogus or a real) will currently stay forever in the system table. I am attaching a patch (it includes the one I suggested above) to delete these bogus rows next time the admin enters modules configuration. It is actually exactly the same code snippet, just 'theme' is replaced with 'module'.

I grepped both core and contributions file trees, and there does not seem to be any calls to either of these methods that would cause problems with this patch - patched methods are never called on some other folder. However, if someone would actually create an alternative folder with some different themes or modules, and call the functions on that one manually, he could potentially lose some system table rows. Calling the patched functions on random folders (which do not contain themes or modules at all) would do nothing - bogus rows are deleted only when there are some real rows to insert as well.

Wouldn't it make more sense to avoid the files from being inserted, rather than having to delete them afterwards?

That is exactly the problem. If a file ends with ".module" and is in the modules folder, it will get inserted into the database. With xtemplate files it was easier because they can only have one name - "xtemplate.xhtml". Perhaps we can also change the call that collects "style" files as well to only care about exact matches.

But how to do this with modules? If the module has its own folder, allow only one .module file per folder, perhaps? But what about the main modules folder? Do something like my initial patch for xtemplate and skip files that start with a period? Or, before accepting a new module file, parse it for some standard function name that must be inside?

In any case, xtemplate files seems to be the only case where errors may happen without the administrator doing something silly (because bogus file gets used INSTEAD of the real template file.) In modules case, errors would only happen if you would enable a sweird-looking module that appeared out of nowhere. So, I think that jhriggs patch is enough to close this issue. Admins can take care of their own bogus files and records in other cases.

My understanding is that multiple .module files in one folder is intended to work for both core modules and contrib modules. Please do not try to break this. I use this functionality to group strongly related modules.

the most recent patch doesn't apply anymore.

This problem was introduced by moving files across several OSes with differen philosophies about line endings and stuff. Not Drupal's job to fix it.