While looking into why wysiwyg_filter seemed to be mysteriously stripping out img tags when it shouldn't, I've come across the issue that the module is hard to configure in code (e.g. in an installation profile).

I'm setting a variable for my input format something like this in my install profile:

variable_set('wysiwyg_filter_valid_elements_raw_X', 'a[!href|target<_blank|title],
div[align<center?justify?left?right],
p[align<center?justify?left?right|style],
br,span,em,strong,cite,code,blockquote,ul,ol,li,dl,dt,dd,
h2,h3,h4,
img[!src|alt|title|style|width|height]',
);

However, it became apparent that this was not working. I tracked this down to ./wysiwyg_filter.inc:79

function wysiwyg_filter_get_valid_elements($format, $parsed = FALSE) {
  if ($parsed) {
    if (!($valid_elements = variable_get('wysiwyg_filter_valid_elements_parsed_'. $format, NULL))) {
      $valid_elements = wysiwyg_filter_default_valid_elements();
      $valid_elements = wysiwyg_filter_parse_valid_elements($valid_elements);
    }   
    return $valid_elements;

    // etc...

I'm not setting wysiwyg_filter_valid_elements_parsed_X so the defaults are being used instead (which in my case meant no img elements showing up despite what looked like okay settings in my profile).

Tinkering with the settings in the admin form and hitting submit made my images appear, but not because of any changes I'd made - it's because the admin form's submit function is saving the parsed settings into the variable:

function wysiwyg_filter_settings_filter_submit($form, &$form_state) {
  $format = $form_state['values']['format'];

  // Save trimmed version of user input.
  $valid_elements = trim($form_state['values']['wysiwyg_filter_valid_elements_raw_'. $format]);
  $form_state['values']['wysiwyg_filter_valid_elements_raw_'. $format] = $valid_elements;

  // Store an additional parsed version of valid_elements data.
  $valid_elements = wysiwyg_filter_parse_valid_elements($valid_elements);
  variable_set('wysiwyg_filter_valid_elements_parsed_'. $format, $valid_elements);

  // etc...

This means that it's harder to set up wysiwyg_filter in code than it might be. There are a couple of solutions:

1) save the parsed settings as well during the set up in code - however, the wysiwyg_filter_parse_valid_elements() function may not be available when the variables are being set, which is not very convenient.

2) tweak wysiwyg_filter_get_valid_elements to check whether there is a raw version of the settings when it finds there's no parsed version before falling back to defaults. It's calling wysiwyg_filter_parse_valid_elements() on the defaults anyway, so it wouldn't add much cost to do this check e.g.

    if (!($valid_elements = variable_get('wysiwyg_filter_valid_elements_parsed_'. $format, NULL))) {
      $valid_elements = variable_get('wysiwyg_filter_valid_elements_raw_'. $format, wysiwyg_filter_default_valid_elements());
      $valid_elements = wysiwyg_filter_parse_valid_elements($valid_elements);
    }  

3) go one step further; to avoid parsing the settings on the fly over and again, when this situation arises parse them once and store the variable. e.g.

    if (!($valid_elements = variable_get('wysiwyg_filter_valid_elements_parsed_'. $format, NULL))) {
      // There are no parsed settings for this format, 
      //  check if we have the raw settings, if so parse and store them
      if (is_string($raw_settings = variable_get('wysiwyg_filter_valid_elements_raw_'. $format, FALSE))) {
        $valid_elements = wysiwyg_filter_parse_valid_elements($raw_settings);
        variable_set('wysiwyg_filter_valid_elements_parsed_'. $format, $valid_elements);
      }
      else {
        $valid_elements = wysiwyg_filter_default_valid_elements();
        $valid_elements = wysiwyg_filter_parse_valid_elements($valid_elements);
      }      
    }  

However, whilst it's desirable not to be eating up CPU and slowing everything down parsing the settings over and again, I'm not sure whether doing a variable_set here could have performance implications (e.g. a race condition or at least clearing the whole variable cache).

On the positive side, this should only happen once (barring aforementioned race conditions) when the raw variable has been set up in code, and the parsed version not yet stored.

You could argue that the parsed version should be stored as a cache_set really rather than a variable, but as it's unlikely to change very often...

Perhaps a less risky approach would be to go with the simple (2) solution above, but implement hook_cron to check for raw settings without corresponding parsed versions stored, and remedy the situation.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mcdruid’s picture

patch attached which implements last approach outlined above - i.e. always check for raw settings before falling back to defaults, but only do a variable_set to store any missing parsed settings in hook_cron

mcdruid’s picture

Issue summary: View changes

typo

stefan.r’s picture

Version: 6.x-1.x-dev » 7.x-1.x-dev
Status: Active » Needs work

Needs to be ported to 7.x if applicable!

geek-merlin’s picture

Status: Needs work » Closed (won't fix)

The code does not seem sane to me.
Also features doployment seems to fix the issue.
Feel free to reopen if not.