diff --git a/inc/classes/class-seo.php b/inc/classes/class-seo.php
index 63c8c39ea..409f32539 100644
--- a/inc/classes/class-seo.php
+++ b/inc/classes/class-seo.php
@@ -31,6 +31,27 @@ class Seo {
*/
const DESCRIPTION_MAX_LENGTH = 160;
+ /**
+ * Most unregistered builder-style shortcode names removed from one page
+ * text when building its fallback description. Together with the name
+ * length limit below, it keeps the pattern core builds from them far below
+ * PCRE's size limit, whatever a page contains.
+ *
+ * @since 2.5.0
+ */
+ const FALLBACK_UNREGISTERED_SHORTCODES_LIMIT = 100;
+
+ /**
+ * Which unregistered tag names the fallback description treats as page-builder
+ * shortcodes (`vc_row`, `et_pb_section`, `fusion_builder_container`,
+ * `contact-form-7`): 64 characters at most, starting with a letter, with an
+ * underscore or hyphen in them. Plain bracketed words are common in ordinary
+ * text and stay.
+ *
+ * @since 2.5.0
+ */
+ const FALLBACK_SHORTCODE_NAME_PATTERN = '/^(?=.{1,64}$)(?=[A-Za-z0-9]*[_-])[A-Za-z][A-Za-z0-9_-]*$/D';
+
/**
* Seconds before a failed transcript fetch is retried.
*
@@ -1605,7 +1626,7 @@ private function get_post_excerpt_fallback( $post_id ) {
$text = has_excerpt( $post ) ? $post->post_excerpt : $post->post_content;
$text = preg_replace( '//s', '', (string) $text );
- $text = wp_strip_all_tags( strip_shortcodes( $text ) );
+ $text = wp_strip_all_tags( $this->strip_all_shortcodes( $text ) );
// Decode entities, then strip again: text stored as "</script>"
// must not come back as a real tag.
$text = wp_strip_all_tags( html_entity_decode( $text, ENT_QUOTES | ENT_HTML5, 'UTF-8' ) );
@@ -1613,6 +1634,66 @@ private function get_post_excerpt_fallback( $post_id ) {
return self::trim_to_length( $text, self::DESCRIPTION_MAX_LENGTH );
}
+ /**
+ * Remove shortcodes from a text, including page-builder shortcodes that are
+ * not registered yet, together with the text they enclose.
+ *
+ * Core's strip_shortcodes() only removes the shortcodes registered at the
+ * moment it runs. The video sitemap is served at parse_request priority 1,
+ * before a page builder such as WPBakery or Divi registers its own, so
+ * their tags would stay in the description as raw `[vc_row]` text while a
+ * front-end page (wp_head, after the builder loaded) drops them. Adding
+ * the unregistered builder-style tag names found in the text to core's list,
+ * through its own `strip_shortcodes_tagnames` filter, gives the same result
+ * at both points.
+ *
+ * Only names that look like a builder tag are added (see
+ * {@see self::FALLBACK_SHORTCODE_NAME_PATTERN}): ordinary bracketed text
+ * such as `[sic]`, `[1]` or `[b]bold[/b]` is left alone, so the description
+ * of a page whose shortcodes are all registered does not change.
+ *
+ * @since 2.5.0
+ *
+ * @param string $text Page text.
+ * @return string Text without shortcodes.
+ */
+ private function strip_all_shortcodes( $text ) {
+ $add_found_tag_names = static function ( $tags, $content ) {
+ global $shortcode_tags;
+
+ $tags = (array) $tags;
+ $registered = is_array( $shortcode_tags ) ? $shortcode_tags : array();
+ $found = array();
+
+ preg_match_all( '@\[([^<>&/\[\]\x00-\x20=]++)@', (string) $content, $matches );
+
+ foreach ( array_unique( $matches[1] ) as $name ) {
+ if ( count( $found ) >= self::FALLBACK_UNREGISTERED_SHORTCODES_LIMIT ) {
+ break;
+ }
+
+ // A tag another filter took off the list is registered: leave it be.
+ if ( array_key_exists( $name, $registered ) || ! preg_match( self::FALLBACK_SHORTCODE_NAME_PATTERN, $name ) ) {
+ continue;
+ }
+
+ $found[] = $name;
+ }
+
+ return array_merge( $tags, $found );
+ };
+
+ add_filter( 'strip_shortcodes_tagnames', $add_found_tag_names, 10, 2 );
+
+ try {
+ $stripped = strip_shortcodes( $text );
+ } finally {
+ remove_filter( 'strip_shortcodes_tagnames', $add_found_tag_names, 10 );
+ }
+
+ return (string) $stripped;
+ }
+
/**
* Whether the current request is a front-end page render where render-time
* video SEO should be collected and emitted.
diff --git a/tests/php/VideoSeoSchemaTest.php b/tests/php/VideoSeoSchemaTest.php
index a7637c2a7..04d323411 100644
--- a/tests/php/VideoSeoSchemaTest.php
+++ b/tests/php/VideoSeoSchemaTest.php
@@ -45,6 +45,14 @@ class VideoSeoSchemaTest extends TestCase {
*/
private $temp_dirs = array();
+ /**
+ * Registered shortcodes as they were before register_shortcodes() replaced
+ * them, put back in tearDown().
+ *
+ * @var array|null
+ */
+ private $saved_shortcode_tags = null;
+
protected function setUp(): void {
parent::setUp();
$GLOBALS['rtgodam_stub'] = array();
@@ -53,6 +61,10 @@ protected function setUp(): void {
}
protected function tearDown(): void {
+ if ( null !== $this->saved_shortcode_tags ) {
+ $GLOBALS['shortcode_tags'] = $this->saved_shortcode_tags;
+ $this->saved_shortcode_tags = null;
+ }
foreach ( $this->temp_dirs as $dir ) {
$this->remove_dir( $dir );
}
@@ -239,6 +251,165 @@ public function test_page_content_fallback_strips_blocks_and_shortcodes() {
$this->assertSame( 'Hello & welcome.', $schema['description'] );
}
+ /**
+ * Replace the registered shortcodes, as they stand at a given point of a
+ * request. The sitemap is built at parse_request priority 1, before a page
+ * builder registers its shortcodes; a front-end page (wp_head) is built after.
+ *
+ * @param string[] $tags Shortcode tags that are registered.
+ * @return void
+ */
+ private function register_shortcodes( array $tags ) {
+ if ( null === $this->saved_shortcode_tags ) {
+ $this->saved_shortcode_tags = $GLOBALS['shortcode_tags'];
+ }
+
+ $GLOBALS['shortcode_tags'] = array_fill_keys( $tags, '__return_empty_string' );
+ }
+
+ /**
+ * Description a page with the given content gets, or '' when it has none.
+ *
+ * @param string $content Page content.
+ * @return string
+ */
+ private function page_description( $content ) {
+ $GLOBALS['rtgodam_stub']['posts'][7] = (object) array(
+ 'post_type' => 'page',
+ 'post_excerpt' => '',
+ 'post_content' => $content,
+ );
+
+ $schema = $this->seo->build_video_object_schema( $this->legacy_entry(), 7 );
+
+ return $schema['description'] ?? '';
+ }
+
+ public function test_page_fallback_does_not_show_builder_shortcodes_that_are_not_registered_yet() {
+ // The sitemap runs before WPBakery registers vc_row, vc_column and vc_column_text.
+ $this->register_shortcodes( array( 'gallery', 'godam_video' ) );
+
+ $description = $this->page_description( '[vc_row][vc_column][vc_column_text]Learn to brew filter coffee at home.[/vc_column_text][godam_video id="265"][/vc_column][/vc_row]' );
+
+ $this->assertStringNotContainsString( '[', $description );
+ $this->assertStringNotContainsString( 'vc_', $description );
+ }
+
+ public function test_page_fallback_is_the_same_before_and_after_the_builder_registers_its_shortcodes() {
+ $content = 'Intro text. [vc_row][vc_column][vc_column_text]Inside the builder.[/vc_column_text][/vc_column][/vc_row] Outro text.';
+
+ $this->register_shortcodes( array( 'gallery', 'godam_video' ) );
+ $at_sitemap_time = $this->page_description( $content );
+
+ $this->register_shortcodes( array( 'gallery', 'godam_video', 'vc_row', 'vc_column', 'vc_column_text' ) );
+ $at_page_time = $this->page_description( $content );
+
+ $this->assertSame( $at_page_time, $at_sitemap_time );
+ $this->assertSame( 'Intro text. Outro text.', $at_sitemap_time );
+ }
+
+ public function test_page_fallback_removes_divi_shortcodes_that_are_not_registered() {
+ $this->register_shortcodes( array( 'gallery' ) );
+
+ $description = $this->page_description( 'Before. [et_pb_section][et_pb_row][et_pb_text]Divi copy.[/et_pb_text][/et_pb_row][/et_pb_section] After.' );
+
+ $this->assertSame( 'Before. After.', $description );
+ }
+
+ public function test_page_fallback_removes_a_self_closing_shortcode_that_is_not_registered() {
+ $this->register_shortcodes( array( 'gallery' ) );
+
+ $this->assertSame( 'Hello world.', $this->page_description( 'Hello [unknown_tag foo="bar" /]world.' ) );
+ }
+
+ public function test_page_fallback_leaves_text_outside_shortcodes_alone_when_none_are_used() {
+ $this->register_shortcodes( array( 'gallery' ) );
+
+ $this->assertSame( 'Plain page text with no shortcodes.', $this->page_description( 'Plain page text with no shortcodes.
' ) );
+ }
+
+ public function test_page_fallback_leaves_ordinary_bracketed_text_alone() {
+ $this->register_shortcodes( array( 'gallery' ) );
+
+ $text = 'He said [sic] hello, see [1] and [citation needed], [b]bold[/b] and a[0].';
+
+ $this->assertSame( $text, $this->page_description( $text ) );
+ }
+
+ public function test_page_fallback_removes_a_builder_tag_with_a_hyphen_in_its_name() {
+ $this->register_shortcodes( array( 'gallery' ) );
+
+ $this->assertSame( 'Hello world.', $this->page_description( 'Hello [contact-form-7 id="1"]world.' ) );
+ }
+
+ public function test_page_fallback_leaves_a_tag_another_filter_took_off_the_strip_list() {
+ $this->register_shortcodes( array( 'gallery', 'my_plugin_tag' ) );
+ add_filter(
+ 'strip_shortcodes_tagnames',
+ static function ( $tags ) {
+ return array_diff( $tags, array( 'my_plugin_tag' ) );
+ },
+ 5
+ );
+
+ $this->assertSame( 'See [my_plugin_tag id="1"] done.', $this->page_description( 'See [my_plugin_tag id="1"] done.' ) );
+ }
+
+ public function test_page_fallback_survives_a_page_full_of_distinct_builder_style_tags() {
+ $this->register_shortcodes( array( 'gallery' ) );
+
+ // Far more names than fit in one pattern: core's regex fails to compile past a few thousand.
+ $content = 'Start. ';
+ for ( $i = 0; $i < 20000; $i++ ) {
+ $content .= '[tag_' . $i . '] ';
+ }
+
+ $description = $this->page_description( $content );
+
+ $this->assertStringStartsWith( 'Start.', $description );
+ $this->assertLessThanOrEqual( Seo::DESCRIPTION_MAX_LENGTH, mb_strlen( $description ) );
+ }
+
+ public function test_page_fallback_survives_a_very_long_tag_name() {
+ $this->register_shortcodes( array( 'gallery' ) );
+
+ // A name this long would make the pattern too large for PCRE and the whole description empty.
+ $content = 'Start. [long_' . str_repeat( 'x', 70000 ) . '] End.';
+
+ $description = $this->page_description( $content );
+
+ $this->assertStringStartsWith( 'Start.', $description );
+ }
+
+ public function test_page_fallback_does_not_leave_its_filter_registered() {
+ $this->register_shortcodes( array( 'gallery' ) );
+
+ $this->page_description( '[vc_row]Text.[/vc_row]' );
+
+ $this->assertEmpty( array_filter( $GLOBALS['rtgodam_hooks']['strip_shortcodes_tagnames'] ?? array() ) );
+ }
+
+ public function test_page_fallback_does_not_leave_its_filter_registered_when_stripping_throws() {
+ $this->register_shortcodes( array( 'gallery' ) );
+ add_filter(
+ 'strip_shortcodes_tagnames',
+ static function () {
+ throw new \RuntimeException( 'another plugin failed' );
+ },
+ 20
+ );
+
+ try {
+ $this->page_description( '[vc_row]Text.[/vc_row]' );
+ $this->fail( 'The exception from the other filter should have reached the caller.' );
+ } catch ( \RuntimeException $e ) {
+ $this->assertSame( 'another plugin failed', $e->getMessage() );
+ }
+
+ // Only the throwing callback (priority 20) is left; the one this fix added (priority 10) is gone.
+ $this->assertEmpty( $GLOBALS['rtgodam_hooks']['strip_shortcodes_tagnames'][10] ?? array() );
+ }
+
public function test_page_text_cannot_close_the_json_ld_script_tag() {
// An Author's paragraph, stored with entities (kses allows this).
$GLOBALS['rtgodam_stub']['posts'][7] = (object) array(
diff --git a/tests/php/VideoSitemapTest.php b/tests/php/VideoSitemapTest.php
index 23435d37e..a4682659f 100644
--- a/tests/php/VideoSitemapTest.php
+++ b/tests/php/VideoSitemapTest.php
@@ -257,6 +257,62 @@ public function test_description_falls_back_to_title_because_google_requires_one
$this->assertSame( 'Video 1', $video['description'] );
}
+ /**
+ * The sitemap's texts for a page whose content is the
+ * given string and whose video has no description of its own, with only the
+ * given shortcodes registered (the sitemap is served at parse_request
+ * priority 1, before WPBakery registers vc_row, vc_column and the rest).
+ *
+ * @param string $content Page content.
+ * @param string[] $registered_shorts Shortcode tags registered at that point.
+ * @return string[]
+ */
+ private function sitemap_descriptions_for_page( $content, array $registered_shorts ) {
+ $saved = $GLOBALS['shortcode_tags'];
+ $GLOBALS['shortcode_tags'] = array_fill_keys( $registered_shorts, '__return_empty_string' );
+
+ try {
+ $GLOBALS['rtgodam_stub']['posts'][10] = (object) array(
+ 'post_type' => 'page',
+ 'post_excerpt' => '',
+ 'post_content' => $content,
+ );
+
+ $sitemap = $this->sitemap_with_pages( array( 10 => array( $this->video( 1, array( 'description' => '' ) ) ) ) );
+ $xml = Video_Sitemap::build_urlset_xml( $sitemap->get_url_entries( 1 ) );
+ } finally {
+ $GLOBALS['shortcode_tags'] = $saved;
+ }
+
+ $xpath = $this->parse( $xml );
+ $descriptions = array();
+ $count = $xpath->query( '//video:description' )->length;
+ for ( $position = 1; $position <= $count; $position++ ) {
+ $descriptions[] = $xpath->evaluate( "string(//video:description[{$position}])" );
+ }
+
+ return $descriptions;
+ }
+
+ public function test_wpbakery_page_does_not_leak_shortcode_text_into_the_description() {
+ $descriptions = $this->sitemap_descriptions_for_page(
+ '[vc_row][vc_column][vc_column_text]Learn to brew filter coffee at home, step by step.[/vc_column_text][godam_video id="265"][/vc_column][/vc_row]',
+ array( 'gallery', 'godam_video' )
+ );
+
+ // Nothing is left of the page text, so the sitemap uses the title, as it does when the builder is active.
+ $this->assertSame( array( 'Video 1' ), $descriptions );
+ }
+
+ public function test_page_text_outside_the_builder_shortcodes_is_used_as_the_description() {
+ $descriptions = $this->sitemap_descriptions_for_page(
+ 'Learn to brew filter coffee at home. [vc_row][vc_column][godam_video id="265"][/vc_column][/vc_row]',
+ array( 'gallery', 'godam_video' )
+ );
+
+ $this->assertSame( array( 'Learn to brew filter coffee at home.' ), $descriptions );
+ }
+
public function test_missing_thumbnail_falls_back_to_featured_image() {
$GLOBALS['rtgodam_stub']['featured_image'][10] = 'https://example.test/featured.jpg';
diff --git a/tests/stubs/hooks.php b/tests/stubs/hooks.php
index 8780aac74..f307cedfc 100644
--- a/tests/stubs/hooks.php
+++ b/tests/stubs/hooks.php
@@ -36,6 +36,29 @@ function add_filter( $hook, $callback, $priority = 10, $accepted_args = 1 ) { //
}
}
+if ( ! function_exists( 'remove_filter' ) ) {
+
+ /**
+ * Remove a callback that add_filter() recorded.
+ *
+ * @param string $hook Hook name.
+ * @param callable $callback Callback, compared by identity.
+ * @param int $priority Priority it was added at.
+ *
+ * @return bool Whether a callback was removed.
+ */
+ function remove_filter( $hook, $callback, $priority = 10 ) { // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedFunctionFound -- stub mirroring the WP function.
+ foreach ( $GLOBALS['rtgodam_hooks'][ $hook ][ $priority ] ?? array() as $index => $registered ) {
+ if ( $registered === $callback ) {
+ unset( $GLOBALS['rtgodam_hooks'][ $hook ][ $priority ][ $index ] );
+ return true;
+ }
+ }
+
+ return false;
+ }
+}
+
if ( ! function_exists( 'add_action' ) ) {
/**
diff --git a/tests/stubs/video-seo-functions.php b/tests/stubs/video-seo-functions.php
index a196d8bdb..7c1c49f75 100644
--- a/tests/stubs/video-seo-functions.php
+++ b/tests/stubs/video-seo-functions.php
@@ -14,6 +14,9 @@
namespace {
+ // get_shortcode_regex(), used by the strip_shortcodes() stub below.
+ require_once __DIR__ . '/shortcode-functions.php';
+
/*
* The HLS URL is read through rtgodam_get_hls_transcoded_url_from_attachment()
* when admin/godam-transcoder-functions.php is loaded (EarlyTranslationHooksTest
@@ -124,14 +127,76 @@ function has_excerpt( $post = 0 ) {
}
}
+ /*
+ * Registered shortcodes. Core fills $shortcode_tags as plugins call
+ * add_shortcode(); strip_shortcodes() below only removes tags found in it,
+ * which is the behaviour the sitemap description depends on. These are the
+ * shortcodes core registers itself, plus GoDAM's. A test that needs another
+ * set assigns $GLOBALS['shortcode_tags'] and restores it in tearDown().
+ */
+ if ( ! isset( $GLOBALS['shortcode_tags'] ) ) {
+ $GLOBALS['shortcode_tags'] = array_fill_keys(
+ array( 'wp_caption', 'caption', 'gallery', 'playlist', 'audio', 'video', 'embed', 'godam_video' ),
+ '__return_empty_string'
+ );
+ }
+
+ if ( ! function_exists( 'strip_shortcode_tag' ) ) {
+
+ /**
+ * WordPress core's strip_shortcode_tag(), copied.
+ *
+ * @param array $m Regex match.
+ * @return string
+ */
+ function strip_shortcode_tag( $m ) {
+ // Allow [[foo]] syntax for escaping a tag.
+ if ( '[' === $m[1] && ']' === $m[6] ) {
+ return substr( $m[0], 1, -1 );
+ }
+
+ return $m[1] . $m[6];
+ }
+ }
+
if ( ! function_exists( 'strip_shortcodes' ) ) {
/**
+ * WordPress core's strip_shortcodes(): removes the registered shortcodes
+ * found in $content, together with the text they enclose, and leaves
+ * every other [tag] as plain text. Copied from wp-includes/shortcodes.php
+ * except for do_shortcodes_in_html_tags() / unescape_invalid_shortcodes(),
+ * which only matter for shortcodes inside HTML attributes.
+ *
* @param string $content Content.
* @return string
*/
function strip_shortcodes( $content ) {
- return preg_replace( '/\[[^\]]*\]/', '', (string) $content );
+ global $shortcode_tags;
+
+ $content = (string) $content;
+
+ if ( false === strpos( $content, '[' ) ) {
+ return $content;
+ }
+
+ if ( empty( $shortcode_tags ) || ! is_array( $shortcode_tags ) ) {
+ return $content;
+ }
+
+ // Find all tag names in $content.
+ preg_match_all( '@\[([^<>&/\[\]\x00-\x20=]++)@', $content, $matches );
+
+ $tags_to_remove = apply_filters( 'strip_shortcodes_tagnames', array_keys( $shortcode_tags ), $content );
+ $tagnames = array_intersect( $tags_to_remove, $matches[1] );
+
+ if ( empty( $tagnames ) ) {
+ return $content;
+ }
+
+ $pattern = get_shortcode_regex( $tagnames );
+
+ return preg_replace_callback( "/$pattern/", 'strip_shortcode_tag', $content );
}
}