filter_xss*() functions live in filter.module.

The reason for countless exceptions and hard-coded loading of filter.module throughout core.

Possible solutions:

a) Move it as-is into common.inc.

b) Move into common.inc and rename into check_xss*().

c) Move into sanitize.inc and rename into check_xss*() or clean_xss*() or similar.

Agreement first.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Anonymous’s picture

+1.

way back when i thought we could use the registry to remove module_load_all from _drupal_bootstrap_full, this was one of the first functions that broke that naive idea.

kill it, kill it dead.

sun’s picture

Given that you're one of folks who seem to work on registry a lot, it would be good to provide at least one opinion on the choices mentioned in the OP.

catch’s picture

I vote (a) but don't have particular objections against (b) or (c).

Anonymous’s picture

i'd vote a), because it solves the problem at hand without having to work out whether we should rename.

Damien Tournoud’s picture

Vote (a) also. I've been willing to do so for a while now ;)

sun’s picture

ok. Now, let's get a confirmation from chx, webchick, and Dries.

Site note: After moving those functions, we can

1) remove required = TRUE from filter.info and

2) no longer load filter.module in maintenance mode (install/update/regular offline) and remove it everywhere else we've hard-coded it.

Which means: check_markup() and filter_form() will be available in DRUPAL_BOOTSTRAP_FULL only. We need to check whether any hook_update_N() tries to invoke other functions of filter.module and fix accordingly.

sun’s picture

#455724: Rename "check_markup()" made this decision simpler: Renaming (and/or possible replacement with PHP's filter functions) will be left for a separate issue.

sun’s picture

Status: Active » Needs review
FileSize
19.57 KB

required = TRUE serves a different purpose and cannot be removed. It means that filter.module must always be enabled, so modules can invoke filter_form() and check_markup() without testing whether filter.module is installed.

sun’s picture

FileSize
19.57 KB

Meh. Damn Windows.

Damien Tournoud’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
2.02 KB

This makes a lot of sense.

Overly cautious, checked this patch very carefully. There is no code change at all (see attached patch), only a couple of whitespace fix and the reformatting of two doxygen comments.

Status: Reviewed & tested by the community » Needs work

The last submitted patch failed testing.

sun’s picture

Status: Needs work » Reviewed & tested by the community

Last attachment shouldn't have been named .patch ;)

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to HEAD. I agree that is the right thing to do. I do think renaming the functions would be good.

Status: Fixed » Closed (fixed)

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

sun’s picture