htmlpurifier_doublecache_$format variable is not read correctly
Leonth - July 3, 2008 - 09:00
| Project: | HTML Purifier |
| Version: | 6.x-1.0 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | ezyang |
| Status: | closed |
Jump to:
Description
It seems that when Drupal invokes hook_filter with $op 'no cache', it does NOT pass $format. It means that "htmlpurifier_doublecache_$format" used in the call to variable_get() will become "htmlpurifier_doublecache_-1" (-1 is the default value of $format) which is obviously meaningless.
A workaround for this misfeature would be to make the option global, i.e. one option is used for every formats. This is a simple patch for that:
--- /media/data/htdocs/Drupal 6 modules/htmlpurifier/htmlpurifier (copy).module 2008-05-13 08:18:30.000000000 +0700
+++ /media/data/htdocs/Drupal 6 modules/htmlpurifier/htmlpurifier.module 2008-07-03 14:50:59.000000000 +0700
@@ -76,7 +76,7 @@
case 'no cache':
// Since HTML Purifier implements its own caching layer, having filter
// cache it again is wasteful. Returns FALSE if double caching is permitted.
- return !variable_get("htmlpurifier_doublecache_$format", FALSE);
+ return !variable_get('htmlpurifier_doublecache', FALSE);
case 'description':
$common = t(
@@ -294,11 +294,11 @@
} else {
$title = t('Advanced configuration options');
$allowed = TRUE;
- $form['htmlpurifier']["htmlpurifier_doublecache_$format"] = array(
+ $form['htmlpurifier']['htmlpurifier_doublecache'] = array(
'#type' => 'checkbox',
'#title' => t('Allow double caching'),
- '#default_value' => variable_get("htmlpurifier_doublecache_$format", FALSE),
- '#description' => t('If enabled, HTML Purifier will tell filter that its output is cacheable. This is not usually necessary, because HTML Purifier maintains its own cache, but may be helpful if you have later filters that need to be cached.'),
+ '#default_value' => variable_get('htmlpurifier_doublecache', FALSE),
+ '#description' => t('If enabled, HTML Purifier will tell filter that its output is cacheable. This is not usually necessary, because HTML Purifier maintains its own cache, but may be helpful if you have later filters that need to be cached.') . '<strong>' . t('This option applies to every HTML Purifier filter in every input formats') . '</strong>',
);
}
Should I also report this misfeature to Drupal core? (I've never done that before)
#1
As a long term solution, I think it would be a good idea to forward this along to the Drupal devs. See if you can cook up a patch for it; the fix is then much more likely to get put in.
As for HTML Purifier, I don't think setting the property globally is much of an option: I specifically put in doublecache to deal with certain filter combinations, and I would prefer that it not be used unless absolutely necessary. I'm going to dig around the code and see if there's a workaround I can add to detect the filter.
#2
This is the issue for Drupal core, along with the patch (turns out to be a one-liner!): #279090: Invoking hook_filter, $op 'no cache' with proper $format argument
#3
Fixed in HEAD, insofar as noting that the property is a global attribute. Will be fully fixed in 7.x port.
#4
Automatically closed -- issue fixed for 2 weeks with no activity.