Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 82 additions & 1 deletion inc/classes/class-seo.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -1605,14 +1626,74 @@ 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 "&lt;/script&gt;"
// must not come back as a real tag.
$text = wp_strip_all_tags( html_entity_decode( $text, ENT_QUOTES | ENT_HTML5, 'UTF-8' ) );

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.
Expand Down
171 changes: 171 additions & 0 deletions tests/php/VideoSeoSchemaTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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 );
}
Expand Down Expand Up @@ -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( '<p>Plain page text with no shortcodes.</p>' ) );
}

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(
Expand Down
56 changes: 56 additions & 0 deletions tests/php/VideoSitemapTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 <video:description> 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';

Expand Down
23 changes: 23 additions & 0 deletions tests/stubs/hooks.php
Original file line number Diff line number Diff line change
Expand Up @@ -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' ) ) {

/**
Expand Down
Loading
Loading