User tests: Successful: Unsuccessful:
Pull Request for Issues #17444, #26715, #26823.
This removes onBeforeExecute
event added in #12124 from CMS applications.
a) Enable System - Privacy Consent plugin. Edit your profile.
b) Set up a multilingual site. Browse around the frontend.
a) No untranslated strings.
b) Works like before.
a) Untranslated strings like PLG_SYSTEM_PRIVACYCONSENT_LABEL
.
IDK.
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries Front End Plugins |
Labels |
Added:
?
|
@brianteeman i agree... this is a hard b/c break given that it's no more possible to observe the onBeforeExecute event in a third-party extension during a CMS application execution. Are you sure to do this?
This was introduced in 4.0.
It is either leave this hook in and break language loading in plugins or remove the hook. Considering the hook is new to 4.0 whereas the language loading behavior dates back to 1.5, to me it’s an easy choice to remove the hook until the application architecture can be built in a way that the global Language instance does not have to be fully configured before importing plugins.
It is a harder B/C break to tell system plugin developers they have to manual load language files because their plugins are imported at a point that they can’t rely on the framework to do it for them as they had for the previous decade.
Trust me, I want the hook, that’s why I added it in the first place. But it has created too many problematic side effects for it to sanely go into a stable release, and I’ve been saying that basically ever since the first bug reports came in about those side effects. It should have been reverted much sooner.
Status | Pending | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2019-11-26 14:48:23 |
Closed_By | ⇒ | wilsonge |
Thanks
And for anyone reviewing this after the merge, the remaining onBeforeExecute
hooks are "safe" in the context of the existing APIs for various reasons:
Joomla\CMS\Application\CliApplication
is deprecated and shouldn't be used by the CMS itself by the time 4.0 ships (still gotta move the CLI Smart Search over to the console application), in addition to the class not importing CMS plugin groups, developers writing CLI applications would be able to cope with the language issue if they needed to (though in CLI context a lot of times languages aren't necessary)Joomla\CMS\Application\DaemonApplication
extends the deprecated Joomla\CMS\Application\CliApplication
(note the daemon really should be deprecated too), same logic applies hereJoomla\CMS\Application\WebApplication::execute()
is overridden by Joomla\CMS\Application\CMSApplication
, developers may choose though to build their own web application classes and the same logic as the CLI applies (basically, if you're building your own application classes expect to have to deal with some architecture quirks on your own)There IS one "before" event that is safe and expected to be used in the CMS environment, that being the Joomla\Application\ApplicationEvents::BEFORE_EXECUTE
event inside the console application. This one is essential to being able to allow plugins to add command classes to the application (either through the command lazy loader or the application itself) and is the only hook available before the application tries to resolve the command. A Joomla\CMS\Language\Language
instance isn't being explicitly set up in the console application, so if you have commands that rely on the language system then you're going to have to set that up (maybe core can offer a helper somewhere to do this?). Plugins in the "console" and "system" groups are imported (though it's really encouraged to use the new console group for this as it will only ever load in the CLI context, but since there are a large number of vendors that use system plugins for a lot of their functionality the system group is imported as well).
This is a hard b/c break ??