Hooks aren't being called from block plugins

[Vitaliy-1]

Related to: https://forum.pkp.sfu.ca/t/add-a-css-to-block-plugin-in-3-2-0-1/59518/6

It seems impossible to hook in TemplateManager:: display with a Block Plugin, e.g., to add styling.

Steps to reproduce the behavior:

1. Create a block plugin
2. Register TemplateManager::display hook
3. Debugging shows that it's never been called.

Originally tested with OJS 3.2.0-1. I've reproduced with 3.2.0-3.
It's possible that other hooks are affected too.

[Vitaliy-1]

Starting from OJS 3.2.0 initialization of block plugins has moved from PKPTemplateManager::initialize to PKPTemplateManager::displaySidebar that is registered through Templates::Common::Sidebar hook, which is called after TemplateManager::display and other hooks.

[Vitaliy-1]

To make a behavior similar to OJS 3.1.2 (register block plugins earlier), it's possible to change this line:

pkp-lib/classes/template/PKPTemplateManager.inc.php

Line 442 in 51bf8bf

HookRegistry::register('Templates::Common::Sidebar', array($this, 'displaySidebar'));

to something like:

$plugins = PluginRegistry::loadCategory('blocks', true);
if (!empty($plugins)) {
		HookRegistry::register('Templates::Common::Sidebar', array($this, 'displaySidebar'));
}

with correspondent changes in displaySidebar method.
@asmecher, what do you think?

[Vitaliy-1]

Before making a PR I need additional confirmation that changes in 3.2 weren't made deliberately to limit the functionality of block plugins.

[NateWr]

I think I'd want to know what the use-case is for hooking into TemplateManager::display from a block plugin. These plugins are specifically designed to only support adding a block, so if something more is being done it may be worth considering whether it is appropriate for a block plugin.

(This is a common issue we've encountered with plugin categories. They were designed to support loading plugins only when required. Loading them on every request defeats the purpose but we continually find cases where people want to act outside of the category's limitations.)

[Vitaliy-1]

According to the forum post, the use-case is to add styling to the block.

[NateWr]

Ah, so to load a CSS stylesheet. That seems sensible. Someone might also want to load a JS file.

The problem with the following solution:

$plugins = PluginRegistry::loadCategory('blocks', true);
if (!empty($plugins)) {
		HookRegistry::register('Templates::Common::Sidebar', array($this, 'displaySidebar'));
}

Is that it will lead to the block plugins getting loaded twice, which can lead to situations where hooks are registered and fired twice as well. We'll need to also make sure that this "double load" situation doesn't happen on the reader-facing frontend, but also in the appearance settings where blocks are listed.

As part of work on #5865, I've separated out the code needed to load backend pages, and asked PageHandlers to opt into that. It'd be nice to do something similar on the reader-facing frontend and be able to determine when we want that code to run. Block plugins could be loaded early in the cycle for frontend pages since they're always used but aren't needed on the backend much.

[Vitaliy-1]

Is that it will lead to the block plugins getting loaded twice, which can lead to situations where hooks are registered and fired twice as well

Debugging shows that in this case if accessing front-end plugins are loaded once as TemplateManager is initialized only once.
But for the back-end, TemplateManager is initialized on every grid

[Vitaliy-1]

Or you mean this call inside the hook: https://github.com/Vitaliy-1/pkp-lib/blob/32621e78650deb20b9ccc698784d6c1911402be8/classes/template/PKPTemplateManager.inc.php#L1013 ?

[NateWr]

I'm talking about PluginRegistry::loadCategory('blocks', true). Calling that twice will lead to the register() method of the plugins being called twice. So we need to call that once, and from then on only call PluginsRegistry::getPlugins('blocks').

[Vitaliy-1]

It'd be nice to do something similar on the reader-facing frontend and be able to determine when we want that code to run

@NateWr, what about initiating block plugins in the PKPTemplatemanager::display, before the correspondent hook, and determine if it's front-end by template path?

[NateWr]

I don't want to determine it from the template path. The /frontend path is a convention not a requirement and someone could change a backend template into a frontend one if they wanted just by overwriting the template.

I think the best approach for now is to determine the best time to load the block plugins before the display hook, load them there, and refactor other places where they are loaded which cause problems. This may be in the TemplateManager::display() method itself, right before the hook is called.

An additional check could be added to only do so if the handler is a PageHandler (I think: is_a($request->getRouter()->getHandler(), 'PKPPageHandler')).

[Vitaliy-1]

You are right! All plugin categories are loaded twice when using Firefox. Google Chrome caches more data that's why I've missed it. Is it expected that TemplateManager initializes twice on the reader-facing front-end?

[Vitaliy-1]

Ah, now I understand, all plugin categories are also loaded on every component request. Sorry, haven't explored this part of the code before.

[Vitaliy-1]

Hmm, does anyone knows why plugin categories are loaded several times, e.g., I found that PluginRegistry::loadCategory('blocks') is mentioned 6 times in the code, why not check if it's loaded first?

While debugging PluginRegistry::loadCategory I see that categories are loaded suspiciously often per request per backend page. I'll need to take another look on this.

@NateWr, I see you already asked that question before: #1735. Did you find the reason?

[NateWr]

The first thing to check is that you're only identifying duplicate loads in the same request. Because a single request to the backend will often fire off five more ajax requests to components, the error log can make it seem like plugins are getting loaded several times. But it's actually only being loaded once per request.

To test this, you can add a define('UNIQUE_ID', uniqid()) in boostrap.inc.php and use that when logging the load calls. That will help you trace back whether the plugins are being loaded more than once per request.

In practice, I found that this accounted for most duplicate load issues. But if you find more, it's a good idea to locate and remove them if possible.

This is a challenge with the given plugin setup. We are moving towards eliminating the pluign categories and going all generic. But to do this we also need to find a way to minimize what actually gets loaded when a plugin is loaded (to minimize performance issues). Alec and I have discussed this but haven't prioritized a solution for the moment.

[NateWr]

Closing this as outdated. If you feel this is still important, please consider making a proposal in the feature request category of our community forum.