0

A 3rd party plugin that I use adds a shortcode that displays a list of adverts (CPT posts). The shortcode function includes (with an include statement) a template - the list.php file, that displays a search form and, in its turn, includes a second template - the list-item.php file, that displays the list of adverts itself.

Each entry of this list includes only a post thumbnail, the post title, the post date, and the price of the advertised item. By changing the list-item.php code I added also the excerpt of the post. The problem is that excerpts are displayed properly when we browse the main adverts list page and not displayed when we browse a category page.

How the adverts list is build when we browse a category page is partially influenced by a function applied to the the_content() filter. The question is: Why excerpts are not displayed on category pages?

This is the shortcode function that displays a list of adverts (and include the list.php template):

add_shortcode('adverts_list', 'shortcode_adverts_list');
/**
 * Generates HTML for [adverts_list] shortcode
 * 
 * @param array $atts Shorcode attributes
 * @since 0.1
 * @return string Fully formatted HTML for adverts list
 */
function shortcode_adverts_list( $atts ) {

    wp_enqueue_style( 'adverts-frontend' );
    wp_enqueue_style( 'adverts-icons' );

    wp_enqueue_script( 'adverts-frontend' );

    extract(shortcode_atts(array(
        'name' => 'default',
        'category' => null,
        'columns' => 2,
        'paged' => adverts_request("pg", 1),
        'posts_per_page' => 20,
    ), $atts));

    $taxonomy = null;
    $meta = array();

    $query = adverts_request("query");
    $location = adverts_request("location");

    if($location) {
        $meta[] = array('key'=>'adverts_location', 'value'=>$location, 'compare'=>'LIKE');
    }

    if($category) {
        $taxonomy =  array(
            array(
                'taxonomy' => 'advert_category',
                'field'    => 'term_id',
                'terms'    => $category,
            ),
    );
    }

    $loop = new WP_Query( array( 
        'post_type' => 'advert', 
        'post_status' => 'publish',
        'posts_per_page' => $posts_per_page, 
        'paged' => $paged,
        's' => $query,
        'meta_query' => $meta,
        'tax_query' => $taxonomy
    ) );

    $paginate_base = get_the_permalink() . '%_%';
    $paginate_format = stripos( $paginate_base, '?' ) ? '&pg=%#%' : '?pg=%#%';

    // adverts/templates/list.php
    ob_start();
    include_once ADVERTS_PATH . 'templates/list.php';
    return ob_get_clean();
}

This is the code of the list.php that includes the list-item.php template:

<div class="adverts-list">
    <?php if( $loop->have_posts() ): ?>
    <?php while ( $loop->have_posts() ) : $loop->the_post(); ?>
    <?php include ADVERTS_PATH . 'templates/list-item.php' ?>
    <?php endwhile; ?>
    <?php else: ?>
    <div class="adverts-list-empty"><em><?php _e("There are no ads matching your search criteria.", "adverts") ?></em></div>
    <?php endif; ?>
    <?php wp_reset_query(); ?>
</div>

This is the full code of customized list-item.php:

<div class="advert-item advert-item-col-<?php echo (int)$columns ?>">

    <?php $image = adverts_get_main_image( get_the_ID() ) ?>
    <div class="advert-img">
        <?php if($image): ?>
            <img src="<?php esc_attr_e($image) ?>" alt="" class="advert-item-grow" />
        <?php endif; ?>
    </div>

    <div class="advert-post-title">
        <span title="<?php esc_attr_e( get_the_title() ) ?>" class="advert-link"><?php the_title() ?></span>
        <a href="<?php the_permalink() ?>" title="<?php esc_attr_e( get_the_title() ) ?>" class="advert-link-wrap"></a>
    </div>

   <!-- THIS IS WHAT WAS CUSTOMIZED -->
    <div class="advert-post-excerpt">
        <span class="advert-excerpt"><?php echo get_the_excerpt(); ?></span>
    </div>
    <!-- END OF CUSTOMIZATION -->

    <div class="advert-published ">

        <span class="advert-date"><?php echo date_i18n( get_option( 'date_format' ), get_post_time( 'U', false, get_the_ID() ) ) ?></span>

        <?php $price = get_post_meta( get_the_ID(), "adverts_price", true ) ?>
        <?php if( $price ): ?>
        <div class="advert-price"><?php esc_html_e( adverts_price( get_post_meta( get_the_ID(), "adverts_price", true ) ) ) ?></div>
        <?php endif; ?>
    </div>fun

</div>

This is the function applied to the the_content() filter when we browse single posts and also category pages (in this case again the first shortcode function is called):

add_filter('the_content', 'adverts_the_content');

function adverts_the_content($content) {
    global $wp_query;

    if (is_singular('advert') && in_the_loop() ) {
        ob_start();
        $post_id = get_the_ID();
        include ADVERTS_PATH . 'templates/single.php';
        $content = ob_get_clean();
    } elseif( is_tax( 'advert_category' ) && in_the_loop() ) {
        $content = shortcode_adverts_list(array(
            "category" => $wp_query->get_queried_object_id()
        ));
    }

    return $content;
}

UPDATE

This is the adverts_request() function, called by the shortcode_adverts_list() function:

function adverts_request($key, $default = null) {
    if(isset($_POST[$key])) {
        return $_POST[$key];
    } elseif(isset($_GET[$key])) {
        return $_GET[$key];
    } else {
        return $default;
    }
}
11
  • debug this can show you a list of all filters attached to a certain filter hook on any frontend page. Looking at the list of filters on the_content on a category page might help you out.
    – tao
    Commented Nov 27, 2015 at 21:29
  • @andrei-gheorghiu I installed that plugin, but besides the function name (adverts_the_content) attached to the_content() I don't see anything helpful.
    – Yuri
    Commented Nov 27, 2015 at 21:45
  • The first condition is_singular('advert') is clearly not true on a category/tax page. Are you sure the elseif is true? Make an echo on that condition in the page template, to make sure.
    – tao
    Commented Nov 27, 2015 at 22:34
  • @andrei-gheorghiu Checked the elseif, is really true. Maybe the shortcode_adverts_list() needs a missing attribute?
    – Yuri
    Commented Nov 27, 2015 at 22:49
  • The only thing we don't have here is what the function adverts_request() does. Can you add that function, too? or, better yet, add $query = ''; inside the condition if($category) inside shortcode_adverts_list().
    – tao
    Commented Nov 27, 2015 at 22:53

2 Answers 2

4

I have reproduced your environment using the same theme and the same plugin. Out of the box, the plugin does not register the CPT with excerpt. You need to register it:

add_post_type_support('advert', array('excerpt'));

This should go in functions.php of your (child?) theme. My only question is: how and why were you getting excerpts on singular pages if the post type does not have excerpts enabled? I assume it's a WP feature/bug that is not the excerpt itself, but the trimmed/stripped version of content (55 chars by default) WP builds and I am guessing it does it without checking if excerpts are enabled for that particular post type.

Alternatively, you could just add excerpt to line 108 of wpadverts/wpadverts.php, but if you do this you will lose this mod if you ever update the plugin.


UPDATE

I think I finally understood what you asked for in the first place. My own testing of the plugin and theme is here. I do not know why the_excerpt() does not return a trimmed down version of the content on advert category pages for posts that do not have manual excerpts. But, to fix this problem, I made a "custom excerpt" for use in list-item.php. Here it is:

<div class="advert-post-excerpt">
    <span class="advert-excerpt">
        <?php
        global $post;
        $limit = 55;
        $content = strip_tags($post->post_content);
        echo has_excerpt($post->ID) ?
            $post->post_excerpt :
            ( strlen($content) > $limit ?
                substr($content, 0, $limit).'...' :
                $content
            )
        ;?>
    </span>
</div>

Change 55 to the desired excerpt length. The code above should replace your initial:

<div class="advert-post-excerpt">
    <span class="advert-excerpt"><?php echo get_the_excerpt(); ?></span>
</div>

I'm a practical guy, if it doesn't work one way, I find a way. However, if anyone could enlighten me on why the_excerpt() returns null in this case, I'd thank them.

5
  • OK, thank you very much! I will do that, but how in this case excerpts are displayed on the main adverts list? I mean without to be enabled?
    – Yuri
    Commented Nov 28, 2015 at 11:03
  • Probably this is an undeclared feature :).
    – Yuri
    Commented Nov 28, 2015 at 11:09
  • You show excerpts the same you did in your question, by altering list-item.php. I am assuming there is no way of overriding the templates of this plugin.
    – tao
    Commented Nov 28, 2015 at 11:12
  • My initial question was about excerpts not being displayed and Andrei helped me to resolve this. Thank you very much! But I want to say also a big thank you to Pieter Goosen (@pieter-goosen) that helped me with a very detailed analyzes of some important issues related to security and performance. Thank you, Pieter!
    – Yuri
    Commented Nov 28, 2015 at 22:05
  • I do not know the plugin, but there is something in the plugin, or another plugin or in the theme which is acting or breaking the excerpt. It would be nice to rather first look for the cause (bad filter for instance) and try to fix that than just simply applying a workaround to someone you do not know ;-) Commented Nov 29, 2015 at 6:06
4

There are a few issues with your code as far as I can see that might or might not have an effect on your output

Before I start, a few important point

  • All code is untested and needs PHP 5.4+. All code should be tested on a local install first with debug set to true

  • What I have given you is the bare minimum. Some pieces of code can be improved, and all can be modified to suite your exact needs

adverts_list SHORTCODE

You should never ever be using extract(). Just like query_posts(), it is an evil function than can set you of on a debugging hell for nothing. extract() was removed from core a couple of versions ago, and even the codex was updated to remove it. The use of globals in WordPress is already such a mess, and then extract() comes along and creates even more right left and centre. But anyways, I'm not going into details, you should read the following in your own time

and also do some research on the subject while you are at it

One other concern is using unsanitized and non validated values coming from attributes. This is dangerous and a big security loophole which can easily be exploited.

Just a sidenote, output buffering to return output from a shortcode should be avoided if you can. This should be your last resort. Rather use function counterparts that returns their output rather than echoing it, and concatenate everything to a variable and then return the variable. You can check this post for examples.

You can probably rewrite the shortcode as follow:

add_shortcode('adverts_list', 'shortcode_adverts_list');
/**
 * Generates HTML for [adverts_list] shortcode
 * 
 * @param array $atts Shorcode attributes
 * @since 0.1
 * @return string Fully formatted HTML for adverts list
 */
function shortcode_adverts_list( $atts ) {

    wp_enqueue_style( 'adverts-frontend' );
    wp_enqueue_style( 'adverts-icons' );

    wp_enqueue_script( 'adverts-frontend' );

    // Coorect way to set attribute defaults
    $attributes = shortcode_atts(
        [
            'name'           => 'default',
            'category'       => 0,
            'columns'        => 2,
            'paged'          => adverts_request("pg", 1),
            'posts_per_page' => 20,
        ], 
        $atts
    );

    // Now we can set our variables and sanitize/validate
    $name            = filter_var( $attributes['name'],            FILTER_SANITIZE_STRING );
    $category        = filter_var( $attributes['category'],        FILTER_VALIDATE_INT    );
    $columns         = filter_var( $attributes['columns'],         FILTER_VALIDATE_INT    );
    $paged           = filter_var( $attributes['paged'],           FILTER_VALIDATE_INT    );
    $posts_per_page  = filter_var( $attributes['posts_per_page'],  FILTER_VALIDATE_INT    );

    $query    = adverts_request("query");
    $location = adverts_request("location");

    $meta_query = [];
    if($location) {
        $meta_query[] = [
            'key'     => 'adverts_location', 
            'value'   => $location, 
            'compare' => 'LIKE'
        ];
    }

    $taxonomy = [];
    if($category) {
        $taxonomy =  [
            [
                'taxonomy' => 'advert_category',
                'terms'    => $category,
            ],
        ];
    }

    $loop = new WP_Query( [
        'post_type'      => 'advert', 
        'post_status'    => 'publish',
        'posts_per_page' => $posts_per_page, 
        'paged'          => $paged,
        's'              => $query,
        'meta_query'     => $meta_query,
        'tax_query'      => $taxonomy
    ] );

    $paginate_base   = get_the_permalink() . '%_%';
    $paginate_format = stripos( $paginate_base, '?' ) ? '&pg=%#%' : '?pg=%#%';

    // adverts/templates/list.php
    ob_start();
    include_once ADVERTS_PATH . 'templates/list.php';
    return ob_get_clean();
}

list.php

Get rid of all the php spam. Only open and clode php tags if one move between php and HTML. There is absolutely no use in using php tags when you move between lines that are still php

It should be noted, wp_reset_query() should be used with query_posts(), which I said you should never ever use. It can have unexpected results. The appropriate reset method with WP_Query is wp_reset_postdata(). You also do not want to reset anything when there are no posts as you did not alter the $post global. Always use wp_reset_postdata() between your endwhile and endif statements.

Just a small note, for the benefit of debugging and code editors, rather use curlies than : and endif/endwhile

You can probably rewrite the code as follow:

<div class="adverts-list">
    <?php 
        if( $loop->have_posts() ) {
            while ( $loop->have_posts() ) {
            $loop->the_post();

                include ADVERTS_PATH . 'templates/list-item.php'

            } //endwhile
            wp_reset_postdata();
        } else { 
    ?>
            <div class="adverts-list-empty"><em><?php _e("There are no ads matching your search criteria.", "adverts") ?></em></div>
    <?php 
        } // endif
    ?>
</div>

list-item.php

The only real issue here is that echo get_the_excerpt(); can be rewritten as the_excerpt() as it is exactly what the_excerpt() does

the_content filter

Unfortunely, your shortcode is a mess and the use of your shortcode

  • You cannot pass php variables to a shortcode. You will need to alter your shortcode to incorporate the variable part. As this is a plugin, you do not want to alter the plugin itself. So the best will be to remove the shorcode's callback function and replace it with your own.

    You can do the following with your shortcode:

    // First, we want to remove the original callback function
    remove_shortcode( 'adverts_list', 'shortcode_adverts_list', 11 );
    
    // Now we add our own callback, 'custom_shortcode_adverts_list' to the shortcode
    add_shortcode( 'adverts_list', 'custom_shortcode_adverts_list', 12 );
    /**
     * Generates HTML for [adverts_list] shortcode
     * 
     * @param array $atts Shorcode attributes
     * @since 0.1
     * @return string Fully formatted HTML for adverts list
     */
    function custom_shortcode_adverts_list( $atts ) {
    
        wp_enqueue_style( 'adverts-frontend' );
        wp_enqueue_style( 'adverts-icons' );
    
        wp_enqueue_script( 'adverts-frontend' );
    
        // Coorect way to set attribute defaults
        $attributes = shortcode_atts(
            [
                'name'           => 'default',
                'category'       => 0,
                'columns'        => 2,
                'paged'          => adverts_request("pg", 1),
                'posts_per_page' => 20,
            ], 
            $atts
        );
    
        // Now we can set our variables and sanitize/validate
        $name            = filter_var( $attributes['name'],            FILTER_SANITIZE_STRING );
        $category        = filter_var( $attributes['category'],        FILTER_VALIDATE_INT    );
        $columns         = filter_var( $attributes['columns'],         FILTER_VALIDATE_INT    );
        $paged           = filter_var( $attributes['paged'],           FILTER_VALIDATE_INT    );
        $posts_per_page  = filter_var( $attributes['posts_per_page'],  FILTER_VALIDATE_INT    );
    
        $query    = adverts_request("query");
        $location = adverts_request("location");
    
        $meta_query = [];
        if($location) {
            $meta_query[] = [
                'key'     => 'adverts_location', 
                'value'   => $location, 
                'compare' => 'LIKE'
            ];
        }
    
        // We need to set $category correctly according to page
        $category = ( is_tax( 'advert_category' ) ) ? get_queried_object_id() : $category;
        $taxonomy = [];
        if($category) {
            $taxonomy =  [
                [
                    'taxonomy' => 'advert_category',
                    'terms'    => $category,
                ],
            ];
        }
    
        $loop = new WP_Query( [
            'post_type'      => 'advert', 
            'post_status'    => 'publish',
            'posts_per_page' => $posts_per_page, 
            'paged'          => $paged,
            's'              => $query,
            'meta_query'     => $meta_query,
            'tax_query'      => $taxonomy
        ] );
    
        $paginate_base   = get_the_permalink() . '%_%';
        $paginate_format = stripos( $paginate_base, '?' ) ? '&pg=%#%' : '?pg=%#%';
    
        // adverts/templates/list.php
        ob_start();
        include_once ADVERTS_PATH . 'templates/list.php';
        return ob_get_clean();
    }
    
  • I really do not know what purpose does $post_id = get_the_ID(); have and it can probably be removed as I do not see any use for it

  • Just on the part of DRY, it will be a bonus to first do a check whether you are in the loop, and it not, return/stop executing your filter. This saves time and is a bit faster

  • Shortcodes are parsed and rendered when the_content filters are applied to get_the_content() before content is echoed via the_content(). So you can simply just add the shortcode as normal to the_content filter

You can try the following: (Adjust as needed as I do not know the exact use case)

add_filter( 'the_content', function ( $content )
{
    // Check if we are in the loop, if not, return
    if ( !in_the_loop() )
        return $content;

    // If this is a single page, execute the following
    if ( is_singular('advert') ) {
        ob_start();
        include ADVERTS_PATH . 'templates/single.php';
        $content = ob_get_clean();
    }

    // If this is a taxonomy page, run the following
    if ( is_tax( 'advert_category' ) ) {
        $content = '[adverts_list]';
    }

    return $content;
});

adverts_request()

You shouldn't be using unsantized values, specially those coming from $_GET and $_POST variables. This is a favorite spot which hackers use to inject malicious code into a site. ALWAYS ALWAYS sanitize and NEVER EVER trust any input from anywhere, do not even trust yourself

I make use of the filter_input filter which santize (if specified) and vaildate a key/value pair coming from a super global.

Also, set value to a parameter and make sure that a value is passed before executing something to avoid unnecessary bugs.

With this in mind, the following will do:

function adverts_request( $key = '', $default = null) 
{
    // Santize our $default value. I accept this is a string value, if not, adjust the filter
    if ( $default !== null )
        $default = filter_var( $default, FILTER_SANITIZE_STRING ); 

    // Check if we have a value, if not return $default
    if ( !$key )
        return $default; 

    // Sanitize $key, again I accept it to be a string, adjust filter accordingly if not
    $key = filter_var( $key, FILTER_SANITIZE_STRING ); 

    // Get our value from $_POST and $_GET. Again, I accepted that $key is a string
    $post_input = filter_input( INPUT_POST, $key, FILTER_SANITIZE_STRING );
    $get_input  = filter_input( INPUT_GET,  $key, FILTER_SANITIZE_STRING );

    if( $post_input ) {
        $output = $post_input;
    } elseif ( $get_input ) {
        $output = $get_input;
    } else {
        $output = $default
    }

    return $output
}
6
  • 1
    Very well spotted, Pieter. I have already notified the plugin author about the security issue. His plugin is pretty handy but he seems oblivious to security threats and coding standards. I also bet he's not familiar with WP itself quite yet. Let's see how he responds.
    – tao
    Commented Nov 28, 2015 at 13:00
  • Please see my the_content filter updates code, there was a bug, forgot to return the new content. Should work now Commented Nov 28, 2015 at 18:58
  • @pieter-goosen I added a return $content; to the the_content() filter and now it works, but I got a notice: Notice: Undefined variable: meta ... in the line 'meta_query' => $meta, of the shortcode_adverts_list( ) function.
    – Yuri
    Commented Nov 28, 2015 at 21:41
  • Resolved the notice by adding a $meta = array(); line before if($location) {.
    – Yuri
    Commented Nov 28, 2015 at 21:53
  • Thank you for the update. One do sometimes miss small stuff when coding large sections of code on small screen devices and one cannot test it. That is why I always state that my code is untested and it should always be tested with debug set to true to catch small bugs like this. I have update my code accordingly for futute reference. Quite strange that the excerpt does not work out of the box. I do not know the plugin, and although the accepted answer works (which is just a workaround to hide the problem), I would look for some filter in the plugin/any plugin or theme that acts on the... Commented Nov 29, 2015 at 6:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.