Loading assets in Packages breaks Sitemap

Permalink
Sometimes you want a package to add some Assets that need to be available all over the site. In our case it's a simple Javascript.
I'm using the on_start() method to add my .js to the AssetList and would expect it to be output somewhere within the header or footer on the site in order for me to use.:

public function on_start(){
        $this->pkg = Package::getByHandle($this->pkgHandle);
        $al = AssetList:: getInstance();
        $al->register('javascript', 'mesch/support/video', 'js/support_video.js', array('minify' => true), $this->pkg);
}


I don't really understand how/ when exactly the Assets are included but what happens now is that the HTML Markup for the loading of the Script is added to some AJAX Requests as well. For example the one that fetches the Sitemap.
Witch leads to a parse Error because a stray script tag is added to the otherwise clean json Response.

The Question is: Is that behaviour by design and there is a better way to do this? Or is it an Issue worth mentioning on GitHub?

I think there should be some mechanism in place to avoid assets beeing output on AJAX Responses.
Or is there any reliable way to detect AJAX Requests within the on_start() method of my package? Then i could simply forgo registering the Asset in cases like that.

1 Attachment

haeflimi
 
Remo replied on at Permalink Reply
Remo
I just ran into a bug where Page::getCurrentPage() didn't return an object. A bit hacky, but I guess if we'd check the return value we could determine whether we're in an AJAX call or not, unless the AJAX call executes a page controller method.

I don't think that $al->register is responsible for the JavaScript in the AJAX call, imho the requireAsset causes that, wherever it is called..
Remo replied on at Permalink Reply
Remo
mnakalay replied on at Permalink Reply
mnakalay
Are you sure the problem is not within your code in support_video.js?
haeflimi replied on at Permalink Reply
haeflimi
Of course the snippet i posted dind't show the issue... it couldn't have been that simple :-/ . After testing a bit more i could register some Assets without the error occuring.
I could reproduce the problem however after registering a Asset of the type "javascript-localized'.

Of course Remo is right. register() itself doesn't cause the problem. It's more like registered Assets seem to be required and output in places they should not.

I am fairly shure it has nothing to do with the content of the required Asset however.
haeflimi replied on at Permalink Reply
haeflimi
I solved it the way Remo suggested.: only require the Asset when Page::getCurrentPage() can be determined. That seems to work although it would be nice having a better way to determine if we're handling a AJAX Request or not.
Remo replied on at Permalink Reply
Remo
There's a method which checks that:https://github.com/concrete5/concrete5/blob/develop/web/concrete/src...

I don't think it's in the right place though, something which is accessibly from a controller method or event the request class would be nice, wouldn't it?