Add new filter: wpseo_sitemap_content_before_parse_html

[lots0logs]

Summary

Add new filter: wpseo_sitemap_content_before_parse_html to allow the post content to be filtered before it is parsed for images (for the xml sitemap). This will allow themes and plugins to process their own shortcodes if they wish to have the images properly included in the sitemap.

Relevant technical choices:

• N/A

Test instructions

• Once hook is implemented in Divi, images in page content of pages created with the Divi Builder will show up in the sitemap.

Fixes #4808
Fixes elegantthemes/Divi#2369
Related #6040

[stodorovic]

Yoast SEO already has filter for image parsing ( wpseo_sitemap_urlimages - inc/sitemaps/class-sitemap-image-parser.php#L99-L105 )

Thrive Visual Editor already uses this hook. I've seen more examples related to theme/plugins. I've developed similar system for VC (for my websites) - related to #6041.

[lots0logs]

Yes I realize that. However, using that hook requires implementing a lot of the logic that is already implemented in the WPSEO_Sitemap_Image_Parser class. This additional hook would avoid the code duplication and makes things easier for those who wish to customize the images that show up in the sitemap.

[stodorovic]

Just info from #4500:

We are querying less data than "normal" post object contains. We also tweak it with some hacks to speed up permalink generation.
This is de-facto not a WP_Post object at this point in code. At this time I am not considering rewriting calling code for the sake of improving this check alone.

Also, I had issues related to do_shortcode in sitemaps (and it's performance issue). I'm not sure that will works in all cases (maybe it works for Divi). It's my experience.

PS: It can't help for #6040 because URLS is part of background-image: url(...) which standard parser can't recognize.

[lots0logs]

It works in Divi 😃 Obviously it won't work in all cases, but that's okay because it's not intended to. Anyone hooking into this new hook would be doing so having made sure it works with their code.

[lots0logs]

Also, I don't think the concerns discussed on #4500 are relevant to this PR as the filter only deals with post content (not the post object being used by the parser).

[andizer]

I have done a review on your pull.

[andizer]

Can you add some docs about the $content`

@param string $content The post content.

It might be better to use this format for documenting the hook:

		/**
		 * Filter: 'wpseo_sitemap_content_before_parse_html' - Filters the post content before it is parsed for images.
		 *
		 * @api string $content The post content.
		 */

[andizer]

I think wpseo_sitemap_content_before_parse_html_images would be a more descriptive name.

[andizer]

Instead of introducing a new filter we might consider using the WordPress way. I think that way is handling shortcodes as well.

$content = apply_filters( 'the_content', $content );
$content = str_replace( ']]>', ']]>', $content );

[lots0logs]

@andizer

Instead of introducing a new filter we might consider using the WordPress way. I think that way is handling shortcodes as well.

I think that would make a lot of sense in this specific case. Honestly, all we would be doing with the new filter in Divi is applying the_content filter on the content (just like in your example).

[lots0logs]

@andizer I've incorporated your feedback.

[moorscode]

We are discussing the approach internally to make sure we are going forward with it in the most optimal way.

The decision is between introducing a performance heavy filter, with limited context awareness: the_content or providing a new filter which multiple plugins can hook into; possibly all adding the_content filter too it, thus causing more performance problems or unwanted behaviour.

As this is 'just' an image count for the sitemaps the impact should be a minimal as possible.

[jdevalk]

@moorscode can we make a decision on this one?

[jdevalk]

Been reading on this, it seems like a good addition but I'd also think that makes it a LOT slower... At the same time, most of this is usually cached, so I'm going to go ahead and approve this.

[jdevalk]

@andizer if you don't have further objections, I'd say merge!