?
avatar wilsonge
wilsonge
10 Apr 2020

Steps to reproduce the issue

We need to have a think about how to deal with web assets on the error pages. For example in #28614 we have an issue with an early exception being thrown in onAfterInitialise). In this case we haven't loaded the registry file in the AdministratorApplication!

https://github.com/joomla/joomla-cms/pull/28581/files#diff-de2cf03053c95fda1cc845b63abf324dL64

cc @Fedik

avatar wilsonge wilsonge - open - 10 Apr 2020
avatar joomla-cms-bot joomla-cms-bot - change - 10 Apr 2020
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - labeled - 10 Apr 2020
avatar Fedik
Fedik - comment - 10 Apr 2020

We need to have a think about how to deal with web assets on the error pages

The error page is a ErrorDocument , in the end instance of Document, which should load WebAsset already, every time new ErrorDocument/Document instance are made

if (\array_key_exists('webAssetManager', $options))
{
$this->setWebAssetManager($options['webAssetManager']);
}
else
{
$webAssetManager = new WebAssetManager(\Joomla\CMS\Factory::getContainer()->get('webassetregistry'));
$this->setWebAssetManager($webAssetManager);
}

This will pull the AssetRegistry from Container, which already load all required assets:

function (Container $container)
{
$registry = new Registry;
// Add Core registry files
$registry
->addRegistryFile('media/vendor/joomla.asset.json')
->addRegistryFile('media/system/joomla.asset.json')
->addRegistryFile('media/legacy/joomla.asset.json');
return $registry;

avatar Fedik
Fedik - comment - 10 Apr 2020

hm, issue I see is that Registry does not load an asset file for active template,
that normally happen at Dispatch:

// Add Asset registry files
$document->getWebAssetManager()->getRegistry()
->addRegistryFile('media/' . $component . '/joomla.asset.json')
->addRegistryFile('templates/' . $template->template . '/joomla.asset.json');

What I can think, is to load registry file in the error.php explicitly:

$wa->getRegistry()->addRegistryFile('templates/blablatemplate/joomla.asset.json');

That may help to avoid such problem, what do you think?
It does not have performance issue, because file will be parsed only once and flagged as parsed in registry instance.

I also had in mind a helper methods, like:

// For template
$wa->getRegistry()->addTemplateFile('blabla-name', 'site');

// For component
$wa->getRegistry()->addComponentFile('blabla-name');
avatar Fedik
Fedik - comment - 10 Apr 2020

or, do it in ErrorDocument::render(), that maybe more nice

* Render the document
*
* @param boolean $cache If true, cache the output
* @param array $params Associative array of attributes
*
* @return string The rendered data
*
* @since 1.7.0
*/
public function render($cache = false, $params = array())
{

avatar wilsonge
wilsonge - comment - 10 Apr 2020

That may help to avoid such problem, what do you think?
It does not have performance issue, because file will be parsed only once and flagged as parsed in registry instance.

I thought about that but I couldn't actually see any code preventing it from being parsed a second time actually

avatar Fedik
Fedik - comment - 10 Apr 2020

but I couldn't actually see any code preventing it from being parsed a second time

ups, that actually a bug ?
it must be here:

foreach ($this->dataFilesNew as $path)
{
$this->parseRegistryFile($path);
// Mark as parsed (not new)
unset($this->dataFilesNew[$path]);
$this->dataFilesParsed[$path] = $path;

but I see it missed check for if (!empty($this->dataFilesParsed[$path])),
I will prepare a pull request, maybe at the weekend

avatar jwaisner jwaisner - change - 10 Apr 2020
Status New Confirmed
Build staging 4.0-dev
avatar Fedik
Fedik - comment - 11 Apr 2020

fun thing, the issue was already fixed in past, but because the path was hardcoded for the "site" it does not work for administrator

// Add registry file for the template asset
$this->getDocument()->getWebAssetManager()->getRegistry()
->addRegistryFile('templates/' . $template . '/joomla.asset.json');

I have made pull #28636

avatar wilsonge wilsonge - change - 11 Apr 2020
Status Confirmed Closed
Closed_Date 0000-00-00 00:00:00 2020-04-11 09:20:18
Closed_By wilsonge
avatar wilsonge
wilsonge - comment - 11 Apr 2020

ahh oops :) closing for the PR

avatar wilsonge wilsonge - close - 11 Apr 2020

Add a Comment

Login with GitHub to post a comment