Hook prefix error for 'the_content' filter

[surimohnot]

Plugin gives error for not prefixing 'the_content' filter hook (if it is invoked by a theme). There are many valid reasons for theme to use 'the_content' filter in themes and it obviously cannot be prefixed. Even twenty seventeen uses this filter.

This error is only visible if you have added theme prefix.

[dingo-d]

Hi @vedathemes thanks for the testing.

If you could provide some more details that would be great.

1. The standards that you've selected when this error happened
2. Example of the code, or the theme you tested
3. Exact error message

The prefix check is coming from WordPress Coding Standards, it would be great if you could open an issue there with a code example as well so this can be fixed in the proper place 🙂

[surimohnot]

1. I have selected "WPThemeReview" standard.
2. I checked it on my own theme https://wordpress.org/themes/bayleaf/
3. Error msg: Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "the_content".

I will check and open an issue as suggested by you. Thanks

[ernilambar]

I can confirm this. I think we should exclude core hooks from this prefixing check.

[dingo-d]

There is a

protected $whitelisted_core_hooks = array(
    'widget_title'   => true,
    'add_meta_boxes' => true,
);

In the sniff checking this rule. My guess is this should be extended to include the_content as well. Do you have any more examples of core hooks which should be in this list?

[ernilambar]

I am afraid I dont have the list exactly. I guess there would be dozens of filters.

[dingo-d]

Interesting comment: https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/1139

[joyously]

This is about apply_filters, right? There shouldn't be dozens, but there might be a few. When a theme has a nav_walker, it should still apply core filters. Same for image_size_names_choose.

the_content is a tricky one. If a theme is applying that filter, it's probably manipulating the content when it shouldn't. I don't think it should be white listed.

[surimohnot]

I think, there are a few valid cases where a theme can use 'the_content' filter.

One possible reason could be to take benefit of various functions that are hooked to this filter for processing raw data. Like, run_shortcode, autoembed, do_shortcode, do_blocks etc. (With future proof correct hook priority order). get_the_content() is not providing processed content and the_content() just echoing processed data. So, there is not way other than manually passing get_the_content() through this filter without echo.

one possible use case for this processed data (there could be many more) is to fetch video, audio or gallery from the content (If you want to display post video or audio on index pages).

[surimohnot]

This is about apply_filters, right? There shouldn't be dozens, but there might be a few. When a theme has a nav_walker, it should still apply core filters. Same for image_size_names_choose.

the_content is a tricky one. If a theme is applying that filter, it's probably manipulating the content when it shouldn't. I don't think it should be white listed.

I think it is add_filter where content is manipulated and not apply_filters.

[dingo-d]

This is about apply_filters, right? There shouldn't be dozens, but there might be a few. When a theme has a nav_walker, it should still apply core filters. Same for image_size_names_choose.

If there are examples of more hooks that a theme can use, could you write them down? It would be good to add them to the PR I've made to the WPCS 🙂

I think it is add_filter where content is manipulated and not apply_filters.

Correct

[joyously]

I think it is add_filter where content is manipulated and not apply_filters.

What I meant was that a filter is the correct way for a theme to manipulate content, done with add_filter, but this is checking for apply_filter which is only used when the content is built or retrieved outside the normal flow of the WordPress query. In other words, manipulating the content outside of a filter, which it shouldn't necessarily be doing (referring here to the_content) and then calling apply_filter on it so the rest of the normal filters are run. So I still feel that the_content should not be on the whitelist.

could you write them down?

Look in the core nav_walker and see which filters are there. Those should be in other nav_walkers.

[surimohnot]

I think it is add_filter where content is manipulated and not apply_filters.

What I meant was that a filter is the correct way for a theme to manipulate content, done with add_filter, but this is checking for apply_filter which is only used when the content is built or retrieved outside the normal flow of the WordPress query. In other words, manipulating the content outside of a filter, which it shouldn't necessarily be doing (referring here to the_content) and then calling apply_filter on it so the rest of the normal filters are run. So I still feel that the_content should not be on the whitelist.

I agree that for normally displaying post content, we should always use the_content() function rather than generating it manually. However, there are cases when we don't want to display the content but use it to fetch some data from it (as explained in my previous comment). 'the_content' filter is the most cleanest and future proof method to do this. I suggest you to check twenty seventeen theme for more on how a theme can use this.

[ernilambar]

Can anybody explain why this type of code is not good in theme? Just curious. $content = apply_filters( 'the_content', $some_content );

[surimohnot]

In my view, applying the_content filter should not be a problem. It will only pass $some_content to hooked functions. However, there are few things that should be taken care of,

1. If some theme author unknowingly recreate a core filter as a theme specific filter ( That's where theme prefixing is required ). Here, sniffer should create a Warning (not ERROR) that "a core filter is used and it should be checked that author is really intending to use the core filter".

2. If your $some_content is not get_the_content() and there is a hooked function which is updating some data in database or cache from that content (It should be possible, but I don't know if there is any function that is really doing that). It might pose a security or compatibility problem. Therefore, use of this filter should be restricted to get_the_content() only.

I think Sniffer should show a warning message that the_content core filter has been used and reviewer should manually check for above two things. But, in anyway it should not be an ERROR message (because there are many valid reasons to manually apply the_content filter on get_the_content()).

On a side note, I think this filter should be shifted to get_the_content() function.

[dingo-d]

@vedathemes Could you open an issue in the WPCS repo outlining the possible issue? As the prefixAllGlobalsSniff is pulled from there. Also any code examples will help, alongside any other allowed theme filters.

@ernilambar I think that it's considered 'not good' because it's a core filter. If this was

$content = apply_filters( 'plugin_name_content_filter', $content );

that would basically allow some plugin to hook to your theme filter which would be ok.

Setting it as a core filter means that any plugin that hooks to the_content filter may impact your output (and you may not want that).

At least that's the way I see it 🤷‍♂️

[joyously]

I see it as plugin functionality or content creation. The theme should not be gathering content to apply that filter to. It should only be affecting the content by adding a filter to the_content.

[justintadlock]

Here's a full description of why apply_filters( 'the_content', $content ) is often a problem and breaks plugins: https://themehybrid.com/weblog/how-to-apply-content-filters

Basically, it should only ever be called in The Loop context where a global $post object is available. Under that scenario is the only time a theme author should do it.

I can't even count the number of times I've had to contact plugin/theme authors to fix this because they were breaking a plugin of mine.

[surimohnot]

Here's a full description of why apply_filters( 'the_content', $content ) is often a problem and breaks plugins: https://themehybrid.com/weblog/how-to-apply-content-filters

Basically, it should only ever be called in The Loop context where a global $post object is available. Under that scenario is the only time a theme author should do it.

I can't even count the number of times I've had to contact plugin/theme authors to fix this because they were breaking a plugin of mine.

Thank you for this insight. It's a learning for me.

[dingo-d]

I have closed the upstream PR in the WPCS about this, and it seems like themes shouldn't mess with this filter, right? So I'll close this.

Feel free to ping me if this needs to be opened 🙂