When Drupal invokes hook_filter with $op 'no cache', it does NOT pass $format. I caught this bug when trying out HTML Purifier module, which needs to turn on/off caching for individual formats using the filter (issue for htmlpurifier: #278079: htmlpurifier_doublecache_$format variable is not read correctly ).

To reproduce the bug:

function foo_filter($op, $delta = 0, $format = -1, $text = '') {
  switch ($op) {
    case 'no cache':
      drupal_set_message($format);
      return FALSE;
  }
}

should output the format ID, e.g. 1, 2, ... It instead outputs -1.

Fixing this would be very easy (just pass the proper variable when invoking hooks), the documentation is ambiguous about this ("no cache: Return true if caching should be disabled for this filter."), can the patch go for Drupal 6.x? The documentation needs updates too.

CommentFileSizeAuthor
#1 invoke-filter-nocache-args.patch589 bytesLeonth

Comments

Leonth’s picture

Leonth commented
StatusFileSize
new invoke-filter-nocache-args.patch589 bytes

Seems that the patch got lost when I submitted the issue. It is a one-liner!

--- modules/filter/filter.admin.inc	2008-07-06 16:56:29.000000000 +0700
+++ modules/filter/filter.admin-invoke-filter-nocache-args.inc	2008-07-06 16:58:52.000000000 +0700
@@ -199,7 +199,7 @@ function filter_admin_format_form_submit
       db_query("INSERT INTO {filters} (format, module, delta, weight) VALUES (%d, '%s', %d, %d)", $format, $module, $delta, $weight);
 
       // Check if there are any 'no cache' filters.
-      $cache &= !module_invoke($module, 'filter', 'no cache', $delta);
+      $cache &= !module_invoke($module, 'filter', 'no cache', $delta, $format);
     }
   }
damien tournoud’s picture

damien tournoud commented
Status: Needs review » Closed (won't fix)

Well, no. The hook_filter() documentation [1] clearly states:

$format Which input format the filter is being used in (applies to 'prepare', 'process' and 'settings').

The caching/no caching of a filter does not depend of the form it is in, at least on Drupal 6. A discussion on implementing more filters that can be aware of their format context is going on for Drupal 7, thou.

[1] http://api.drupal.org/api/function/hook_filter

Leonth’s picture

Leonth commented
Version: 6.2 » 7.x-dev
Category: bug » feature
Status: Closed (won't fix) » Active

Sorry for not reading the docs clearly... So... can this go for Drupal 7? At least there is one use case for this feature...

Leonth’s picture

Leonth commented
Version: 7.x-dev » 6.2
Category: feature » bug
Status: Active » Closed (won't fix)

Digging into that link, it seems to be exactly what I am requesting... sorry