hook_filter_info(): Remove $op from hook_filter() [#546336]

Introduce a new hook_filter_info to allow modules to declare input filters they provide. The input filters are declared using an associative array (hook_menu-alike registry structure).
Preparation, processing and configuration are invoked using the callbacks defined instead of calling hook_filter($op, ...).

Example:

function filter_filter_info() {
  $filters[0] = array(
    'name' => t('Limit allowed HTML tags'),
    'description' => t('Allows you to restrict the HTML tags the user can use. It will also remove harmful content such as...'),
    'process callback' => '_filter_html',
    'settings callback' => '_filter_html_settings',
    'tips callback'  => '_filter_html_tips'
  );
  $filters[1] = array(
    'name' => t('Convert line breaks'),
    'description' => t('Converts line breaks into HTML (i.e. &lt;br&gt; and &lt;p&gt;) tags.'),
    'process callback' => '_filter_autop',
    'tips callback' => '_filter_autop_tips'
  );
  $filters[2] = array(
    'name' => t('Convert URLs into links'),
    'description' => t('Turns web and e-mail addresses into clickable links.'),
    'process callback' => '_filter_url',
    'settings callback' => '_filter_url_settings',
    'tips callback' => '_filter_url_tips'
  );
  $filters[3] = array(
    'name' =>  t('Correct broken HTML'),
    'description' => t('Corrects faulty and chopped off HTML in postings.'),
    'process callback' => '_filter_htmlcorrector',
  );
  $filters[4] = array(
    'name' => t('Escape all HTML'),
    'description' => t('Escapes all HTML tags, so they will be visible instead of being effective.'),
    'process callback' => '_filter_html_escape',
    'tips callback' => '_filter_html_escape_tips'
  );
  return $filters;
}

Comment	File	Size	Author
#20	hook_filter_info-clenaup-20.patch	13.23 KB	dropcube
#20
#18	drupal.please-filter-this-more.patch	14.21 KB	sun
#18
#17	drupal.please-filter-this.patch	13.17 KB	sun
#17
#11	hook_filter_info-11.patch	21.57 KB	dropcube
#11
#7	hook_filter_info-7.patch	12.97 KB	dropcube
#7
#2	hook_filter_info-1.patch	11.64 KB	dropcube
#2
	hook_filter_info.patch	9.02 KB	dropcube

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

11 August 2009 at 19:22

Status:

Needs review

» Needs work

The last submitted patch failed testing.

Comment #2

dropcube CreditAttribution: dropcube commented 11 August 2009 at 19:35

Status:

Needs work

» Needs review

File	Size
hook_filter_info-1.patch	11.64 KB

Fixed tests that failed. Oh, seems to be a lot of confusion in filter.test, the term filter and format are being used indistinctly.

Comment #3

Dries CreditAttribution: Dries commented 11 August 2009 at 20:41

Could you provide a motivation for this patch? That helps us put it in context as well as understand why this is 'critical'. Thanks dropcube. :)

Comment #4

dropcube CreditAttribution: dropcube commented 11 August 2009 at 21:07

Dries, this issue comes from an old and ambitious one: #258939: Filter system revamp.

Motivations:

- Cleaning up the filter API to make it into an "info" hook, consistently with others info hooks in core (hook_menu, hook_theme, etc..)
- Benefits of registry-style info hooks is that they do NOT get re-called every time, which is a performance win and means less unused code loaded in every page request.
- If decided, filters definitions could be altered: hook_filter_info_alter(), so that this hook behaves like other "info" hooks.
- Code in nested switch for $op and $delta results is hard to maintain and debug.

Also see #546350: Remove hardcoded numeric deltas from hook_filter_info().

Comment #5

catch

he/him

English

CreditAttribution: catch commented 11 August 2009 at 22:09

subscribing. Looks good on a first pass, but don't have time to look in-depth at all now.

Comment #6

12 August 2009 at 08:02

Status:

Needs review

» Needs work

The last submitted patch failed testing.

Comment #7

dropcube CreditAttribution: dropcube commented 12 August 2009 at 23:46

Status:

Needs work

» Needs review

File	Size
hook_filter_info-7.patch	12.97 KB

Include implementation of php_filter_info(), to pass the tests.

Comment #8

Dries CreditAttribution: Dries commented 13 August 2009 at 11:46

Status:

Needs review

» Reviewed & tested by the community

This looks like a nice clean-up, and one I support. Filters are used a _lot_ so I wonder if there is any performance regressions due to this patch. I doesn't look like there would be one. I think this is RTBC, really.

Comment #9

Berdir

German

Switzerland

CreditAttribution: Berdir commented 13 August 2009 at 12:10

Status:

Reviewed & tested by the community

» Needs work

Looks nice, however, you need to update hook_filter() in filter.api.php too....

Comment #10

alexanderpas CreditAttribution: alexanderpas commented 13 August 2009 at 13:35

Comment #11

dropcube CreditAttribution: dropcube commented 13 August 2009 at 14:24

Status:

Needs work

» Needs review

File	Size
hook_filter_info-11.patch	21.57 KB

This patch updates filter.api.php as pointed by Berdir. However, the documentation can be improved in follow-up issues, as there are other issues pending, for example #546350: Remove hardcoded numeric deltas from hook_filter_info()

Comment #12

Dries CreditAttribution: Dries commented 13 August 2009 at 19:54

Priority:	Critical	» Normal
Status:	Needs review	» Needs work

Looks great. Committed to CVS HEAD.

Marking this as 'code needs work' because we need to update the upgrade instructions and follow-up on the remaining issues.

Comment #13

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 14 August 2009 at 17:15

Issue tags:

+Needs documentation

The upgrade doc definitely needs some work, see also #548308: Remove hook_filter_tips().

There's currently a section in the update guide that says the parameters for hook_filter() have changed - http://drupal.org/update/modules/6/7#hook_filter_params -- but hook_filter() no longer exists by that name at all, so that section needs to be removed or rewritten.

So it looks to me as though what needs to be documented is:
- hook_filter changed its name to hook_filter_info() (I think? not sure which issue that is)
- new parameter for hook_filter_info()
- No more hook_filter_tips()

Not all of that is related to this issue, but it all needs to be fixed, and possibly there are other things? I'm not sure. If someone could make a coherent list of the changes, that would be a good start.

Comment #14

dropcube CreditAttribution: dropcube commented 21 August 2009 at 05:22

Issue tags:

-Needs documentation

Added upgrade instructions: hook_filter() replaced by hook_filter_info(), and removed hook_filter_tips()

Comment #15

webchick

she/they

English

Vancouver 🇨🇦

CreditAttribution: webchick commented 21 August 2009 at 08:08

Issue tags:

+Needs documentation

Adding the tag back, pending jhodgdon's review.

Comment #16

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 21 August 2009 at 16:17

Assigned:

dropcube

» jhodgdon

This does still need some work. I will take care of it. In particular, the old upgrade guide section on hook_filter() needs to be removed or updated. Not sure of other problems, as I haven't reviewed in detal yet.

Comment #17

sun

German

Karlsruhe

CreditAttribution: sun commented 24 August 2009 at 05:46

Status:

Needs work

» Reviewed & tested by the community

File	Size
drupal.please-filter-this.patch	13.17 KB

I really should not be doing this...

Great, now let's fix the introduced coding-style issues and improper documentation.

Apparently, this patch is not guilty for all changes, but it seems like the other patch that changed the structure of hook_filter_info() is not tagged with FilterSystemRevamp.

Comment #18

sun

German

Karlsruhe

CreditAttribution: sun commented 24 August 2009 at 05:58

File	Size
drupal.please-filter-this-more.patch	14.21 KB

Of course, I didn't recognize the introduced code in the update function introduced by #546350: Remove hardcoded numeric deltas from hook_filter_info().

Comment #19

24 August 2009 at 06:40

Status:

Reviewed & tested by the community

» Needs work

The last submitted patch failed testing.

Comment #20

dropcube CreditAttribution: dropcube commented 25 August 2009 at 21:28

Status:

Needs work

» Needs review

File	Size
hook_filter_info-clenaup-20.patch	13.23 KB

This patch fixes some coding-style issues and improves/fixes the documentation of hook_filter_info().

Comment #21

sun

German

Karlsruhe

CreditAttribution: sun commented 26 August 2009 at 05:32

Status:

Needs review

» Reviewed & tested by the community

Comment #22

Dries CreditAttribution: Dries commented 26 August 2009 at 10:17

Status:

Reviewed & tested by the community

» Fixed

Committed to CVS HEAD. Thanks.

Comment #23

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 26 August 2009 at 16:52

Status:

Fixed

» Needs work

Doc still needs work here, as per comment #16 above. Assigned to me, I'll take care of it.

Comment #24

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 26 August 2009 at 17:21

Status:

Needs work

» Fixed

Taken care of now.

Comment #25

9 September 2009 at 17:30

Status:

Fixed

» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Comment #26

naught101 CreditAttribution: naught101 commented 7 April 2010 at 03:32

Status:

Closed (fixed)

» Active

Re-opening for a documentation question: What is $op="no cache" replaced with with the new hook_filter_info? Also, a module that I'm trying to upgrade has a

switch ($op) { 
  default: return $text;
}

Is that now completely irrelevant?

Also, it would be good to link the "Converting 6.x modules to 7.x" documentation back to this issue, for reference and context.

Comment #27

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 7 April 2010 at 14:32

Status:

Active

» Fixed

From http://api.drupal.org/api/function/hook_filter_info/7 it looks like the return value has a 'cache' element.

I have just added that to the upgrade guide.

I guess for your default op, you should see if you need to make a prepare/process function?

Comment #28

naught101 CreditAttribution: naught101 commented 7 April 2010 at 14:46

Thanks, there's already a process function, that was just shorthand. Anyway, I'll see how it goes, the default option probably wasn't previously used anyway...