7.x Port

I have not made any official benchmarks, however, it appears to be anecdotally the case that most servers can't really handle ~100 transformations per page load. The Drupal filter does have an aggressive caching policy which should alleviate this.

Here is an initial port of the module to Drupal 7.

Most things seem to behave themselves, but there are TODOs in the patch noting some areas that still need work (some might be existing bugs in D6 though). It also seems like integration with the non-Drupal form isn't working totally right in all cases yet.

Other notes:

• This patch moves the HTML Purifier cache settings from the per-format configuration pages to their own dedicated settings page. Because of the way the filter administration form works in D7, it would require some real gymnastics to keep making these available where they were before, and wouldn't really make sense to anyway - if they affect all text formats on the site, then they shouldn't be attached to a single text format's configuration form because it is confusing to save one text format and have the settings automatically applied to other text formats also.
• This currently requires a patch to D7 core in order to work correctly: #826380: Empty filter objects for new/unconfigured filters are incomplete

Thanks for your great work.

Re moving the cache settings, it's a good change that probably should have been in D6, except I couldn't figure out how to make a global settings page, so into the per-filter it went. It may still be useful to provide a link to the global settings page.

I'll take the patch for a spin soon; it might not be too early to set up a branch either. David, would you at all be interested in commit access?

Cheers,
Edward

Thanks, Edward!

I'm not sure I'll have a lot of time to work on this module beyond the initial effort of porting it for (possible use in) Drupal Gardens, so you might want to hold off on giving me commit access... or, rather, you're certainly welcome to give me commit access, but I'm not sure how useful it will wind up being to you in the long run :) It's a very cool module and a neat library, though.

Either way, a branch sounds like it might be a good idea if you feel this is getting close - that would help other people more easily test it out with D7 and help fix the remaining issues.

Here is an updated patch that adds a link to the global settings page as described in #4.

It also fixes a bug with the default filter settings - I was previously using something that made no sense at all (a full HTMLPurifier_Config object when we actually want a settings array). For now, I've fixed this by just removing the 'htmlpurifier_config' default setting altogether, since the module itself works fine without it (see the attached "interdiff" file for the change).

However, I ideally want to figure out how to do it correctly because it's needed for anyone else who wants to write code that cleanly modifies the filter's default behavior. The requirement is that any element which exists in the 'default settings' array in hook_filter_info() should have the same exact structure as what is saved to the database when you submit that part of the filter settings form. So in particular, if you manually save the filter settings form, you wind up with an array of data in 'htmlpurifier_config' with elements that look like this:

    [URI.AllowedSchemes] => http
https
mailto
ftp
nntp
news

This is the format expected by $config->mergeArrayFromForm(), and that is what we want. The problem is I can't figure out how to use the library to recreate this array structure automatically in 'default settings'. The closest I could get was with code like this:

    'default settings' => array(
      'htmlpurifier_help' => TRUE,
      'htmlpurifier_config' => _htmlpurifier_get_config()->getAll(),
    ),

However, that still isn't quite right. That method produces an array of settings with elements that look like this:

[URI] => Array
        (
            [AllowedSchemes] => Array
                (
                    [http] => 1
                    [https] => 1
                    [mailto] => 1
                    [ftp] => 1
                    [nntp] => 1
                    [news] => 1
                )

so not quite the same as above.

Does the library provide a method that I am not finding which outputs the configuration in an array like the first one - i.e. in an format that is suitable to feed back in to $config->mergeArrayFromForm()? Just wondering if I'm missing something obvious :)

Interesting question. mergeArrayFromForm is intended to deal with data supplied by an HTML form, so the closest thing you can get to that is the code that generates the HTML form, which is in HTMLPurifier_Printer_Config, but you still need a browser.

What you're probably actually looking for is HTMLPurifier_Config->loadArray(), which loads in the result of getAll, or perhaps even HTMLPurifier_Config::inherit($parent), which creates a config object that inherits the settings of another object, but can override them.

subscribing

Yep, I've branched out 7.x-1.x for dev. Patch as it stands doesn't apply cleanly to latest CVS HEAD.

Committed. There are some residual issues which I will be dealing with shortly.

Current known issues with the 7.x port:

• On filter config page comes up, we get the errors: Notice: Undefined property: stdClass::$name in _htmlpurifier_settings() (line 483 of htmlpurifier.module).
• Selecting basic filter results in display advanced config
• Styling of the config form is god awful; figure out why Drupal styles are overriding ours

Reclassified.

Bump and subscribing.

Subscribing

subscribe

subscribing!
good module for stupid users who want use html 3.0 ))

Other discovered issues:

* We need to write the upgrade script from 6 to 7
* #993274: Remove double caching logic needs to be completed

The stdclass error appears to have gone a way. I've fixed up the styling in my local copy. Displays seem fixed.

There's another bug where JS for the basic form doesn't work, because Drupal seems to put both the advanced config and the basic config forms on the same page, so the IDs are messed up.

Last bug fixed. Gonna roll a release.

Actually, no, we still need to write the upgrade script. Sigh.

JacobSingh: I don't think an upgrade hook is necessary, based on reviewing the filter code you cite, and looking at some other ported modules. Shouldn't the filter upgrade hook handle all the cases we're interested in? I guess I should run an upgrade and find out.

I'm pretty sure an upgrade is needed due to #685486: filter_update_7003() doesn't work for contrib modules.

Ok, that sounds complicated. I'll investigate it soon but I'm not sure when I'll be able to figure it out.

Patch for a D7 upgrade per http://drupal.org/update/modules/6/7#filter_update.

Closed #1359418: Schema issue upon upgrade to D7 as duplicate and merging into here the patch for the D6 upgrade. One difference is it checks if the schema fields actually exists before droping it for those who have native installed the module in D7.

Fixed in bba8868