Change JComponentHelper filter usage to namespace version

[HLeithner]

Pull Request for Issue #41164 .

Summary of Changes

This commit changes all usages of JComponentHelper to \Joomla\CMS\Component\ComponentHelper. Additionally an exception is thrown when JComponehtHelper is used as filter but is not resolvable.

@richard67 we need a migration script for the database entries... at least for the plugins and the custom fields. Any idea what's the best way is for this?

Testing Instructions

Open Article add html save it.

Actual result BEFORE applying this Pull Request

HTML format get removed

Expected result AFTER applying this Pull Request

HTML format still here

Link to documentations

Please select:

• Documentation link for docs.joomla.org:

• No documentation changes for docs.joomla.org needed

• Pull Request link for manual.joomla.org: Needs documentation that JComponentHelper is not resolved anymore

• No documentation changes for manual.joomla.org needed

[richard67]

@HLeithner A migration function could be called in the "postflight" method in script.php similar to here:

https://github.com/joomla/joomla-cms/blob/5.0-dev/administrator/components/com_admin/script.php#L2202-L2205

It might need to modify the version check before that so it runs also when updating from a 5.0.0-alpha, i.e. anything lower than 5.0.0-beta1: https://github.com/joomla/joomla-cms/blob/5.0-dev/administrator/components/com_admin/script.php#L2198-L2200

The method function be a new private function similar to the one called in the example above https://github.com/joomla/joomla-cms/blob/5.0-dev/administrator/components/com_admin/script.php#L2217

[brianteeman]

I have tested this item ✅ successfully on d0a2509

tested the editor with the backward compatibility plugin disabled and the formatting was kept

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/41259.}

[HLeithner]

@HLeithner A migration function could be called in the "postflight" method in script.php similar to here:

https://github.com/joomla/joomla-cms/blob/5.0-dev/administrator/components/com_admin/script.php#L2202-L2205

It might need to modify the version check before that so it runs also when updating from a 5.0.0-alpha, i.e. anything lower than 5.0.0-beta1: https://github.com/joomla/joomla-cms/blob/5.0-dev/administrator/components/com_admin/script.php#L2198-L2200

The method function be a new private function similar to the one called in the example above https://github.com/joomla/joomla-cms/blob/5.0-dev/administrator/components/com_admin/script.php#L2217

can you help me writing this function please?

[Fedik]

I would suggest to make a filter class, instead.
Example like for filter="safehtml"

joomla-cms/libraries/src/Form/Filter/SafehtmlFilter.php

Lines 43 to 51 in e2170c8

	public function filter(\SimpleXMLElement $element, $value, $group = null, Registry $input = null, Form $form = null)
	{
	return InputFilter::getInstance(
	[],
	[],
	InputFilter::ONLY_BLOCK_DEFINED_TAGS,
	InputFilter::ONLY_BLOCK_DEFINED_ATTRIBUTES
	)->clean($value, 'html');
	}

That will call Joomla\CMS\Component\ComponentHelper::filterText internally:

public function filter($element, $value ...) {
  return Joomla\CMS\Component\ComponentHelper::filterText($value);
}

[wilsonge]

I actually agree with @Fedik here. I think given this is a very common pattern in core making lots of people hardcode php callbacks is asking for b/c breaks (because extensions will be using this too). Making it a static name will be more maintainable going forwards.

[HLeithner]

have you looked at the loadType how complicate it is to do the same thing then just setting the real name of the filter class+function.

It also uses J-prefix for classes, I'm not even sure which namespaces it considers, it might look only for Joomla core namespace? maybe I'm wrong. But this construct to just load one of our 5 filters is completely over engineered, if we allow such thing like shortcut to a filter object method then I think hardcoding that 5 classes makes more sense then expecting any 3rd party dev to reverse engineer this function...

never the less I'm happy for a shortcut for simple text filterText, but I would deprecate/remove this class autoloading voodoo

[richard67]

@HLeithner A migration function could be called in the "postflight" method in script.php similar to here:
https://github.com/joomla/joomla-cms/blob/5.0-dev/administrator/components/com_admin/script.php#L2202-L2205
It might need to modify the version check before that so it runs also when updating from a 5.0.0-alpha, i.e. anything lower than 5.0.0-beta1: https://github.com/joomla/joomla-cms/blob/5.0-dev/administrator/components/com_admin/script.php#L2198-L2200
The method function be a new private function similar to the one called in the example above https://github.com/joomla/joomla-cms/blob/5.0-dev/administrator/components/com_admin/script.php#L2217

can you help me writing this function please?

@HLeithner Don‘t expect any help or advise from me because when I give some you obviously ignore it.

[HLeithner]

ok no problem thanks

[bayareajenn]

In reading through the comments, would you like me to test this now or wait until more changes are made?

[HLeithner]

Yes please test it (with new installatons for now only, updates without b/c plugin are not handled yet)

[brianteeman]

@bayareajenn the code works - the question is if it is an appropriate solution

[bayareajenn]

I have tested this item ✅ successfully on d0a2509

Tested with b/c plugin enabled and disabled in articles, categories, and custom modules. Works in all cases. Thanks :)

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/41259.}

[richard67]

@HLeithner A migration function could be called in the "postflight" method in script.php similar to here:
https://github.com/joomla/joomla-cms/blob/5.0-dev/administrator/components/com_admin/script.php#L2202-L2205
It might need to modify the version check before that so it runs also when updating from a 5.0.0-alpha, i.e. anything lower than 5.0.0-beta1: https://github.com/joomla/joomla-cms/blob/5.0-dev/administrator/components/com_admin/script.php#L2198-L2200
The method function be a new private function similar to the one called in the example above https://github.com/joomla/joomla-cms/blob/5.0-dev/administrator/components/com_admin/script.php#L2217

can you help me writing this function please?

@HLeithner What needs to be migrated?

Update: Ah, I just see the difference in the base.sql. Just params of 2 plugins in the extensions table. Or is there more?

[HLeithner]

yes, at least com_fields needs to be updated JComponentHelper::filterText -> to the namespaced version

[richard67]

yes, at least com_fields needs to be updated JComponentHelper::filterText -> to the namespaced version

Will see if I can provide something tomorrow.

[HLeithner]

thanks

[richard67]

@HLeithner As the replacement would be a complete parameter value, it could be done easily with an update SQL script and string replacement. But that is not really nice with json strings, even if it would be safe, so the other way would be a conversion method in script.php like I suggested above. What do you prefer? Easy but theoretical risky way in SQL? Or safer but maybe a bit over engineered way in PHP?

[HLeithner]

Thats a good question... We could do it with SQL json functions...?

I'm happy with both

[richard67]

Thats a good question... We could do it with SQL json functions...?

I'm happy with both

I'm not familiar with json functions in SQL.

[richard67]

@HLeithner I've just made a new installation on a MySQL 8.0 database with the branch of this PR and then immediately after the installation I've checked the parameters in database. They lose the backslash, so for the editor field they are e.g.:
{"buttons":0,"width":"100%","height":"250px","filter":"JoomlaCMSComponentComponentHelper::filterText"}

The form shows "No" as selected filter for that field.

If I change the selection in the form to "Text" and save the form, the result in database is with double backslashes:
{"buttons":0,"width":"100%","height":"250px","filter":"\\Joomla\\CMS\\Component\\ComponentHelper::filterText"}

So we have to use double backslashes in the base.sql scripts for the params's filter values:

(0, 'plg_fields_editor', 'plugin', 'editor', 'fields', 0, 1, 1, 0, 1, '', '{"buttons":0,"width":"100%","height":"250px","filter":"\\Joomla\\CMS\\Component\\ComponentHelper::filterText"}', '', 4, 0),

I haven't tested PostgreSQL yet, but I expect it to be the same.

The same applies to PostgreSQL 14.9 and MariaDB 10.6, I've just tested.

The XML files are ok, no need for double backslashes there.

I've also experimented with an update SQL statement for an update script. On MySQL 8.0 and MariaDB 10.6, the following works with double backslashes:

UPDATE `#__extensions`
   SET `params` = JSON_REPLACE(`params`, '$.filter' , '\\Joomla\\CMS\\Component\\ComponentHelper::filterText')
 WHERE `type` = 'plugin'
   AND `folder` = 'fields'
   AND `params` <> ''
   AND JSON_EXTRACT(`params`, '$.filter') = 'JComponentHelper::filterText';

This would change it for all field plugins which have a filter property equal to "JComponentHelper::filterText" in their params, which would also include 3rd party field plugins.

Should we limit this to the editor and textarea plugins with an additional AND element in ('editor', 'textarea') so that it's limited to the core only?

For PostgreSQL this could be used:

UPDATE "#__extensions"
   SET "params" = jsonb_set("params"::jsonb, '{filter}' , '"\\Joomla\\CMS\\Component\\ComponentHelper::filterText"')
 WHERE "type" = 'plugin'
   AND "folder" = 'fields'
   AND "params" <> ''
   AND "params"::jsonb->>'filter' = 'JComponentHelper::filterText';

[richard67]

@HLeithner I've created PR #41417 with my changes suggested above for the base.sql scripts and an update SQL for migrating the parameter.

The update SQL currently would change it for all field plugins which have a filter property equal to "JComponentHelper::filterText" in their params, which would also include 3rd party field plugins.

Should we limit this to the editor and textarea plugins with an additional AND element in ('editor', 'textarea') so that it's limited to the core only?

[HLeithner]

I wouldn't limit it, because this would also convert 3rd party fields... Thinking about this might be a problem... Maybe you are right and we limit it to (relevant) core fields only

[richard67]

I wouldn't limit it, because this would also convert 3rd party fields... Thinking about this might be a problem... Maybe you are right and we limit it to (relevant) core fields only

@HLeithner Relevant core fields plugins are only 'editor', 'text' and 'textarea', is that right?

[richard67]

I wouldn't limit it, because this would also convert 3rd party fields... Thinking about this might be a problem... Maybe you are right and we limit it to (relevant) core fields only

@HLeithner Relevant core fields plugins are only 'editor', 'text' and 'textarea', is that right?

I've updated my PR to limit the migration to the 3 named plugins.

[richard67]

@HLeithner You wrote in the description of this PR:

@richard67 we need a migration script for the database entries... at least for the plugins and the custom fields. Any idea what's the best way is for this?

For the plugins we have my PR for you.

But what needs to be done for custom fields?

[richard67]

Ah, I see ... the fieldparams column of the #__fields table might need to be migrated, too. Will check for details.

[richard67]

I've updated my PR #41417 by the migration of the custom field's fieldparams on update. I haven't tested it yet, but it should work.

[laoneo]

Merging this for now to have the description fields accepting HTML content again. Thanks @richard67 for your patience on this.

This pr doesn't prevent to add a more sophisticated solution in the future, as @Fedik suggested.