[5.1] Implement onAfterInitialiseRouter event

[Hackwar]

Summary of Changes

We allow to modify the router by adding rules to it. So far often enough this was done by using a plugin and adding those rules on onAfterInitialise. This however poses a problem when we are not in the SiteApplication and want to create a link, since the router might not be instantiated properly. Often enough plugins are checking if the current application is the SiteApplication, and if not, they don't react. However that means when you are in the backend and create a link to the frontend for for example a mailing, the URL might be wrong, because the plugins with the custom behavior didn't attach that behavior to the router, since it thought it was in the wrong context.

This PR introduces the new onAfterInitialiseRouter event. This event is triggered each time the router object is created (which should only be once per process) and allows plugins to add their logic independent of the currently running application and especially independent of when the object is created. To show how this would be used in the future, the languagefilter plugin has been changed to use this new method.

I first was sure that the languagefilter plugin would suffer from the problem I described above and that this would be a hidden bug. However I then noticed that the plugin "gets around this", by forcing the SiteRouter object upon plugin instantiation. I consider this pretty bad style...

Testing Instructions

Setup a multi language site and apply the patch. Notice that the URLs/behavior did not change.

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:

• No documentation changes for manual.joomla.org needed

[Fedik]

Looks good. Only a couple notes.
Can we please place it here:

joomla-cms/libraries/src/Service/Provider/Router.php

Lines 42 to 50 in e2170c8

	$container->alias('SiteRouter', SiteRouter::class)
	->alias('JRouterSite', SiteRouter::class)
	->share(
	SiteRouter::class,
	function (Container $container) {
	return new SiteRouter($container->get(SiteApplication::class));
	},
	true
	);

kind of:

$router = new SiteRouter($container->get(SiteApplication::class));
$container->get(DispatcherInterface::class)->dispatch(
  'onAfterInitialiseRouter',
  new AfterInitialiseRouterEvent('onAfterInitialiseRouter', ['router' => $router])
);
return $router;

This way it will be more modular, and consistent when at some point we decide to extend it to another Routers.
And keep __constructor clean.

Another note. As @wilsonge already wrote.
Please remove PluginHelper::importPlugin it should be already imported outside (actualy it already done in App initialisation), if it not then it not a Router problem.

[Fedik]

@Hackwar please move the event dispatching out from the constructor, keep the constructor "light", then it is good to go 😉

It may looks a simple solution to place it there, but it will give a lot of headache to refactor this code later.

[Hackwar]

@Fedik While I disagree with the change, it's not the hill I'm going to die on. As requested, this has been implemented.

[Fedik]

Visiting a home page, got an error:
SiteRouter not set in Joomla\Plugin\System\LanguageFilter\Extension\LanguageFilter from

joomla-cms/plugins/system/languagefilter/src/Extension/LanguageFilter.php

Line 738 in 1cb8bc3

$currentInternalUrl = 'index.php?' . http_build_query($this->getSiteRouter()->getVars());

[Fedik]

I have tested this item ✅ successfully on 222e9ae

Works now.
Vielen Dank!

---

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

[Hackwar]

I'm not happy with https://github.com/joomla/joomla-cms/pull/42692/files#diff-efd8fc556b2cba4f15f336f73a194fd64fbb0b0a8fbb6c4478e87804b6f3ce48R740 yet.

[Fedik]

I see, but you already set the router at

joomla-cms/plugins/system/languagefilter/src/Extension/LanguageFilter.php

Line 216 in 222e9ae

$this->setSiteRouter($router);

So you probably can revert $this->getApplication()->getRouter() back to $this->getSiteRouter(), because this code runs only for site application, or?

[Hackwar]

I'm not happy that we have the get/setSiteRouter at all and I don't like the order in which this is set. I would either keep it the way it is with the getApplication()->getRouter() or get it directly from the DI container. But someone decided that we should deprecate the getRouter() and at the same time it is frowned upon to get this from the container directly. But the get/setSiteRouter also is nasty. Why do we have this for one specific application router? Why shouldn't we have this for all applications? And with this being called when the plugin is instantiated, the router is created before the system plugins are loaded, thus making it impossible to run a system plugin on the onAfterRouterInitialise event. It basically is just a matter of time when a third party extension uses this code and thus breaks this feature.

[laoneo]

You should not need an application to get the router from it. As only the SiteRouter is needed to create different urls, I decided to create that one first, if over time we need some other ones for more applications, then feel free to create new services.

[SniperSister]

I have tested this item ✅ successfully on 222e9ae

Fine for me. Did a core review and also tested the functionality of the new event in the depending PRs.

---

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

[chmst]

RTC

---

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

[brianteeman]

I have tested this item 🔴 unsuccessfully on 07c4e6a

---

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

[Hackwar]

Has been fixed.

[brianteeman]

It should be rendered the same as other descriptions eg

[Hackwar]

I would agree with you, however the global configuration code doesn't allow that. I'm going to revert the whole notice part and this apparently will have to go into an additional PR.

[Fedik]

I have tested this item ✅ successfully on eafa7fa

---

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

[SniperSister]

I have tested this item ✅ successfully on eafa7fa

I've re-tested the PR after the latest changes, works as described.

---

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

[richard67]

RTC

---

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

[LadySolveig]

Would appreciate feedback and testing on this proposal for the additional note in global configuration #42832 @Hackwar

[LadySolveig]

Thank you @Hackwar and also for testing and review @Fedik @laoneo @SniperSister @wilsonge @brianteeman