CMS MADE SIMPLE FORGE

CMS Made Simple Core

 

[#11552] Core::StylesheetPostRender hook run twice

avatar
Created By: Mathieu Muths (airelibre) (airelibre)
Date Submitted: Tue Aug 29 09:36:26 -0400 2017

Assigned To:
Version: 2.2.3.1
CMSMS Version: 2.2.3.1
Severity: Minor
Resolution: Fixed
State: Open
Summary:
Core::StylesheetPostRender hook run twice
Detailed Description:
Hi,

I'm trying to make the CSSPreprocessor module work in cmsms 2.2.3.1 but even if
the "do_hook" function is called only once, the "add_hook"line 215 in
class.HookManager.php is called, and the foreach at the end loops 2 times.

It worked fine before this modification: 
http://viewsvn.cmsmadesimple.org/diff.php?repname=cmsmadesimple&path=%2Ftrunk%2Flib%2Fclasses%2Fclass.HookManager.php&rev=11474&peg=11474

The hook is done in {cms_stylesheet} line 269, and the CSSPreprocessor modules
adds the hook in its constructor

Thanks


History

Comments
avatar
Date: 2017-08-29 18:48
Posted By: Robert Campbell (calguy1000)

The HookManager also sends events.  Sending events is done by adding another
hook handler with a closure.
So because 'StylesheetPostRender' is an event there will always be at least one
handler.

Your adding a hook creates a second one.
      
avatar
Date: 2017-08-29 19:27
Posted By: Robert Campbell (calguy1000)

do_hook is being called twice.    Once for each stylesheet, before they are
combined.
The bug  is not in the Hookmanager it is in cms_stylesheet

      
avatar
Date: 2017-08-29 19:30
Posted By: Robert Campbell (calguy1000)

Actually,   in my test the hook is called twice legitimately because two css
files are generated.
      
avatar
Date: 2017-08-30 04:30
Posted By: Mathieu Muths (airelibre) (airelibre)

Thanks for testing.

In my case, I've 2 stylesheets in the DesignManager but I only generate 1 .css
file / 1 <link> tag (which is OK)

If I look at cms_stylesheet, the do_hook is called just before the file is
written in /tmp/cache, and after all the stylesheets have been combined - not
before the combination

So in my case, do_hook is called once, but class.HookManager.php  :: do_hook 
the "add_hook" line 215 is executed which makes the foreach line 234 run twice

I'll make more tests to better undersrtand the logic and see where and if this
is a bug
      
avatar
Date: 2017-08-30 04:55
Posted By: Mathieu Muths (airelibre) (airelibre)

I going deeper - The problem is not that the foreach is run twice, but that the
core event handler seems to not return the CSS content passed in params

I continue
      
avatar
Date: 2017-08-30 05:27
Posted By: Mathieu Muths (airelibre) (airelibre)

I think I got it

\Events::SendEvent does not return any value, but the foreach uses
call_user_func_array with a return value.

In fact, in my case :
1. the SendEvent is run with the correct content params
2. it does not return a value
3. the null return value is affected to $value which is then given to the next
hook, and this causes the problem (the css is not given in params)

So, if the aim of the hook (like events) is to handle passed-by-reference
variables only, I would suggest removing the return value of call_user_func in :

foreach( self::$_hooks[$name]->handlers as $obj ) {
                if( !is_array( $value ) ) $value = [ $value ];
                if( empty($value) ) {
                    call_user_func($obj->callable);
                } else {
                    call_user_func_array($obj->callable,$value);
                }
            }


Or use do_hook_accumulate in {cms_stylesheet} ?
      
avatar
Date: 2017-09-08 12:50
Posted By: Robert Campbell (calguy1000)

This I fixed.
the closure that sends the events now returns the input parameters.
      
Updates

Updated: 2017-09-23 10:52
resolution_id: 9 => 7

Updated: 2017-08-29 18:48
resolution_id: => 9