Somehow my site began printing this error message on posting a new node: Call to undefined function image_ncck_content_is_empty It gave cck.module at line 837.

This is in content_set_empty and the code constructs a function name and directly calls that function. It's not very robust because the function might not exist, and then what?

The below patch causes it to detect the function doesn't exist and simply not call it. This is better defensive coding.

diff -r 1813796c72a7 sites/all/modules/cck/content.module
--- a/sites/all/modules/cck/content.module	Fri Oct 17 08:24:18 2008 -0700
+++ b/sites/all/modules/cck/content.module	Sun Oct 19 20:56:33 2008 -0700
@@ -834,7 +834,7 @@
   $function = $field['module'] .'_content_is_empty';
   $max_delta = $field['multiple'] > 1 ? $field['multiple'] : 0;
   foreach ((array) $items as $delta => $item) {
-    if ($function($item, $field)) {
+    if (function_exists($function) && $function($item, $field)) {
       if ($delta <= $max_delta) {
         foreach (array_keys($field['columns']) as $column) {
           $items[$delta][$column] = NULL;

Comments

yched’s picture

yched commented
Status: Needs review » Postponed (maintainer needs more info)

This probably happens because the module that handles this field type ('image_ncck') is not enabled. image_ncck is part of emfield. I don't know about the status of emfield for D6, but emfield/contrib/image_ncck/readme.txt has :

"NOTICE: image_ncck has been replaced by emimage. If you follow the upgrade instructions properly,
i.e., you disable your modules in d5 before upgrading to d6, you should have no problems."

What's strange is that CCK should have marked those fields 'disabled' during it's own upgrade process, and should therefore not try to process them (so the patch should actually not be needed)

Are you sure you took all the required upgrade steps for CCK and emfield ?

reikiman’s picture

reikiman commented

On every module upgrade I run update.php ...

reikiman’s picture

reikiman commented

Regardless of whether I did the upgrade correctly, the function in question is not properly defensively coded .. but I do appreciate hints as to what might be the cause as to why it tried to use image_ncck when clearly that has been superseded by emfield. Oh.. I see.. the README talks about the d5-d6 upgrade. I think I did do that correctly.

mysql> select filename,name,type,status,throttle,bootstrap from system where filename like '%ncck%';
+-------------------------------------------------+------------+--------+--------+----------+-----------+
| filename                                                    | name | type   | status | throttle | bootstrap |
+----------------------------------------------------------------+------------+--------+--------+----------+-----------+
| sites/all/modules/emfield/contrib/image_ncck/image_ncck.module | image_ncck | module |   0 |  0 |  0 |
+----------------------------------------------------------------+------------+--------+--------+----------+-----------+
1 row in set (0.42 sec)
styro’s picture

styro commented
Version: 6.x-2.0-rc10 » 6.x-2.1
Status: Postponed (maintainer needs more info) » Active

I can confirm this issue. I'm not a CCK user, but was looking into this forum post ( http://drupal.org/node/345088 ), and you can reproduce it on a brand new 6.x site by:

Installing CCK and another CCK field module (eg Asset in this case),
Adding a field from the other module to a content type,
Disabling that module,
Adding a node of the previous content type.

Of course, this can be avoided by deleting any of those modules fields before disabling the module. But I think that is a trap for the unwary, and there could be a check for the *_content_is_empty() function before calling it.

Of course it is probably more the responsibility of the 3rd party modules to disable their fields when they get disabled.

markus_petrux’s picture

markus_petrux commented

Isn't it an issue with the module that implements de CCK field?

In CCK2, fields should notify the content module when they are installed, uninstalled, enabled or disabled. If the module does that, then I believe CCK disables the fields and invalidates de field cached data, so this situation would not happen.

styro’s picture

styro commented
Status: Active » Closed (works as designed)

Yeah probably, I don't don't enough about CCK internals at this stage.

It looks like Asset (in this case) notifies CCK for an uninstallation, but not for disabling.

I suppose the question then is, how much "defensiveness" does CCK want to implement? I guess that there are probably dozens of ways other modules could do things like this and it would just encourage sloppy coding by other modules and/or be a slippery slope to try and handle all scenarios.

I'll leave it to someone else to decide on whether this should be reactivated.