Smart Slider 3 conflict and code improvement suggestion

Resolved Nextendweb
(@nextendweb)

8 years, 3 months ago
Hi @hinjiriyo,
I’m the developer of Smart Slider 3. We use ob_start function like you do in your plugin. I started to collection usage of output buffering examples in different plugins and it seems like you use different method. Here you can check my collection: https://github.com/nextend/wp-ob-plugins-themes/blob/master/README.md

As you can see all of the popular plugins use template_redirect action to start their output buffers. Using template_include instead will result plugin conflict with other plugins.

The suggestion: you should change the template_include filter in class-speed-contact-bar.php
add_filter( 'template_include', array( $this, 'activate_buffer' ), 1 );
To:
add_action( 'template_redirect', array( $this, 'activate_buffer' ), 1 );

And you can change the activate_buffer function to:
```
public function activate_buffer( ) {
    // activate output buffer
    ob_start();
}
```
Thanks in advance,
Roland

Viewing 13 replies - 1 through 13 (of 13 total)

Plugin Contributor Martin Stehle
(@hinjiriyo)

8 years, 3 months ago

Thank you for your note. I am taking a look at it.

Thread Starter Nextendweb
(@nextendweb)

8 years, 3 months ago

@hinjiriyo, thank you. If you want to discuss the changes, you can reach me here or by email: roland@nextendweb.com

Plugin Contributor Martin Stehle
(@hinjiriyo)

8 years, 3 months ago

Your suggestion works well. That topic is new to me. I can not see the urge of changing the hook. Which conflicts would happen with template_include?
Thread Starter Nextendweb
(@nextendweb)

8 years, 3 months ago
@hinjiriyo,

template_include filter and template_redirect action both fired in template-loader.php

Run order:
```
do_action( 'template_redirect' );
...
if ( $template = apply_filters( 'template_include', $template ) ) {
```
Imagine that I open an output buffer on the template_redirect action:
```
add_action('template_redirect', function(){
    ob_start(); // ob started by Roland
});
```
You start yours on template_include:
```
add_filter('template_include', function($template){
    ob_start(); // ob started by Martin
    return $template;
});
```
PHP output buffers are nested which means if you can close only the last opened output buffer and so on.

Roland use the following:
```
add_action('shutdown', function(){
    echo ob_get_clean(); // ob closed by Roland
}, -100);
```
Martin use the following:
```
add_action('shutdown', function(){
    echo ob_get_clean(); // ob closed by Martin
}, 0);
```
Note the add_action priority value, as Roland’s priority lower, it will run first:
- Roland closes the ouput buffer which started by Martin.
- Then Martin closes the output buffer which started by Roland.
I think the best if you use the best if you use X for the priority of template_redirect and -1*X for the shutdown. The you will be safe, see my example:
```
add_action('template_redirect', function(){
    ob_start(); // ob started by Roland
}, 200);
add_action('shutdown', function(){
    echo ob_get_clean(); // ob closed by Roland
}, -200);
```
```
add_action('template_redirect', function(){
    ob_start(); // ob started by Martin
}, 300);
add_action('shutdown', function(){
    echo ob_get_clean(); // ob closed by Martin
}, -300);
```
```
add_action('template_redirect', function(){
    ob_start(); // ob started by Alice
}, 100);
add_action('shutdown', function(){
    echo ob_get_clean(); // ob closed by Alice
}, -100);
```
- Martin closes Martin’s ouput buffer
- Roland closes Roland’s ouput buffer
- Alice closes Alice’s ouput buffer
BTW.: shutdown is an action and not a filter so you should change the following also in your code:
add_filter( 'shutdown', array( $this, 'include_contact_bar' ), 0 );
=>
add_action( 'shutdown', array( $this, 'include_contact_bar' ), 0 );
Thread Starter Nextendweb
(@nextendweb)

8 years, 3 months ago
The conflict happens when a plugin closes all the opened output buffers before your plugin and you plugin will not be able to inject your codes with the include_contact_bar function.

For example JCH Optimise does it.

Smart Slider does almost the same thing. We use named output buffer and close every opened output buffer until we reach ours.
```
add_action('template_redirect', function(){
    ob_start('SmartSlider::ob_callback'); // ob started by Smart Slider
}, 10000);

add_action('shutdown', function(){
    $handlers = ob_list_handlers();
    if(in_array('N2Wordpress::platformRenderEnd', $handlers)){
	    for($i = count($handlers)-1; $i >= 0; $i--){
		    ob_end_flush();
		    if($handlers[$i] === 'SmartSlider::ob_callback'){
			    break;
		    }
	    }
    }
}, -10000);
```
The conflict lies here:
- Smart slider opened an ouput buffer
- Martin opened an output buffer
- Smart Slider closed Martin’s output buffer
- Smart Slider closed Smart Slider’s output buffer
- Martin tried to close an output buffer, but there wasn’t any opened. Was not able to inject the code.
Plugin Contributor Martin Stehle
(@hinjiriyo)

8 years, 3 months ago

Thank you very much for your detailed explanations, Roland. I implemented all your suggestions and the plugin works well. I will publish the upgrade soon.
Thread Starter Nextendweb
(@nextendweb)

8 years, 3 months ago
@hinjiriyo, thank you. If you need a more robust code which may never fail you can use ob_start with callback function:
```
add_action( 'template_redirect', array( $this, 'activate_buffer' ), 100 );
add_action( 'shutdown', array( $this, 'close_buffer' ), -100 );

....

public function activate_buffer() {
    // activate output buffer
    ob_start(array($this, 'include_contact_bar'));
}

public function close_buffer(){
    echo ob_get_clean();
}

public function include_contact_bar($content) {
     ...

    return $content;
}
```
In include_contact_bar method, you should remove $content = ob_get_clean(); as first parameter contains the content of the buffer. Then you should remove echo $content; and instead return it with return $content;
- This reply was modified 8 years, 3 months ago by Nextendweb.
Plugin Contributor Martin Stehle
(@hinjiriyo)

8 years, 3 months ago

Since I focus on robust code I am implementing that. Thank you for that note.

Plugin Contributor Martin Stehle
(@hinjiriyo)

8 years, 3 months ago

Hm, that does not work. No error but no bar is displayed. Maybe, as seen in the PHP documentation, ob_start() accepts only a string as a reference to the callback function, not an array.
Thread Starter Nextendweb
(@nextendweb)

8 years, 3 months ago
Ahh, well sorry for that. It should work:
```
add_action( 'template_redirect', 'Speed_Contact_Bar::static_include_contact_bar', 100 );

public static function static_include_contact_bar($content) {
    return self::$instance->include_contact_bar($content);
}
```
- This reply was modified 8 years, 3 months ago by Nextendweb.
Plugin Contributor Martin Stehle
(@hinjiriyo)

8 years, 3 months ago

That does not work, nor do ob_start( 'Speed_Contact_Bar::static_include_contact_bar' );. There is still both no error and no bar.

I will publish the upgrade without that. Maybe there will be a solution later.

Plugin Contributor Martin Stehle
(@hinjiriyo)

8 years, 3 months ago

In the currently published new version 6.0 your suggestions are implemented.
Thread Starter Nextendweb
(@nextendweb)

8 years, 3 months ago
@hinjiriyo, I’m sorry for the wrong code. It was a long day 🙂

Probably this would work, but I will have time to test it on Monday.
```
add_action( 'template_redirect', array( $this, 'activate_buffer' ), 100 );
add_action( 'shutdown', array( $this, 'close_buffer' ), -100 );

....

public function activate_buffer() {
    // activate output buffer
    ob_start('Speed_Contact_Bar:: static_include_contact_bar');
}

public static function static_include_contact_bar($content) {
    return self::$instance->include_contact_bar($content);
}

public function include_contact_bar($content) {
     ...

    return $content;
}

public function close_buffer(){
    ob_end_flush();
}
```

Viewing 13 replies - 1 through 13 (of 13 total)

The topic ‘Smart Slider 3 conflict and code improvement suggestion’ is closed to new replies.