Skip to content

Commit 6d91e38

Browse files
joedolsonjoedolson
committed
Media: Ensure figcaption/figure IDs are unique.
Using the same image multiple times with a `figure` and `fig caption` would result in duplicate IDs, creating unclear relationships and breaking 3rd party expectations for anchor targets. Check id attributes and append a counter to ensure each attribute is unique, while retaining the current expectation of a counter-less ID attribute for the first instance. Update tests to verify. Developed in WordPress#11940 Props steelwagstaff, westonruter, jamesbregenzer, joedolson, mukesh27. Fixes #65315. git-svn-id: https://develop.svn.wordpress.org/trunk@62698 602fd350-edb4-49c9-b593-d223f7449a82
1 parent 1d693f8 commit 6d91e38

2 files changed

Lines changed: 138 additions & 12 deletions

File tree

‎src/wp-includes/media.php‎

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2656,6 +2656,16 @@ function img_caption_shortcode( $attr, $content = '' ) {
26562656
return $output;
26572657
}
26582658

2659+
/**
2660+
* @var array{
2661+
* id: string,
2662+
* caption_id: string,
2663+
* align: string,
2664+
* width: string,
2665+
* caption: string,
2666+
* class: string,
2667+
* } $atts
2668+
*/
26592669
$atts = shortcode_atts(
26602670
array(
26612671
'id' => '',
@@ -2675,24 +2685,30 @@ function img_caption_shortcode( $attr, $content = '' ) {
26752685
return $content;
26762686
}
26772687

2678-
$id = '';
2679-
$caption_id = '';
2680-
$describedby = '';
2688+
$id = '';
2689+
$caption_id = '';
2690+
$describedby = '';
2691+
$unique_id_value = '';
2692+
$caption_id_value = '';
26812693

26822694
if ( $atts['id'] ) {
2683-
$atts['id'] = sanitize_html_class( $atts['id'] );
2684-
$id = 'id="' . esc_attr( $atts['id'] ) . '" ';
2695+
$atts['id'] = sanitize_html_class( $atts['id'] );
2696+
$unique_id_value = (string) preg_replace( '/-1$/', '', wp_unique_prefixed_id( $atts['id'] . '-' ) );
2697+
$id = 'id="' . esc_attr( $unique_id_value ) . '" ';
26852698
}
26862699

26872700
if ( $atts['caption_id'] ) {
2701+
// User explicitly provided a caption_id - make it unique.
26882702
$atts['caption_id'] = sanitize_html_class( $atts['caption_id'] );
2689-
} elseif ( $atts['id'] ) {
2690-
$atts['caption_id'] = 'caption-' . str_replace( '_', '-', $atts['id'] );
2703+
$caption_id_value = preg_replace( '/-1$/', '', wp_unique_prefixed_id( $atts['caption_id'] . '-' ) );
2704+
} elseif ( $unique_id_value ) {
2705+
// Derive from the already-unique figure ID - guaranteed unique, no need for second call.
2706+
$caption_id_value = 'caption-' . str_replace( '_', '-', $unique_id_value );
26912707
}
26922708

2693-
if ( $atts['caption_id'] ) {
2694-
$caption_id = 'id="' . esc_attr( $atts['caption_id'] ) . '" ';
2695-
$describedby = 'aria-describedby="' . esc_attr( $atts['caption_id'] ) . '" ';
2709+
if ( $caption_id_value ) {
2710+
$caption_id = 'id="' . esc_attr( $caption_id_value ) . '" ';
2711+
$describedby = 'aria-describedby="' . esc_attr( $caption_id_value ) . '" ';
26962712
}
26972713

26982714
$class = trim( 'wp-caption ' . $atts['align'] . ' ' . $atts['class'] );

‎tests/phpunit/tests/media.php‎

Lines changed: 112 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ public function test_img_caption_shortcode_with_old_format_id_and_align() {
269269
)
270270
);
271271
$this->assertSame( 1, substr_count( $result, 'wp-caption &myAlignment' ) );
272-
$this->assertSame( 1, substr_count( $result, 'id="myId"' ) );
272+
$this->assertSame( 1, preg_match( '/id="myId(?:[0-9]+)?"/', $result ) );
273273
$this->assertSame( 1, substr_count( $result, self::CAPTION ) );
274274
}
275275

@@ -365,7 +365,117 @@ public function test_img_caption_shortcode_has_aria_describedby() {
365365
self::IMG_CONTENT . self::HTML_CONTENT
366366
);
367367

368-
$this->assertSame( 1, substr_count( $result, 'aria-describedby="caption-myId"' ) );
368+
$this->assertMatchesRegularExpression( '/aria-describedby="caption-myId(?:-[0-9]+)?"/', $result );
369+
}
370+
371+
/**
372+
* Tests that both figure and figcaption IDs are unique for multiple caption instances.
373+
*
374+
* When the same image with the same or different captions appears multiple
375+
* times on a page, each figure and figcaption should receive a unique ID to
376+
* maintain HTML validity and accessibility.
377+
*
378+
* @ticket 65315
379+
*
380+
* @covers ::img_caption_shortcode
381+
*/
382+
public function test_img_caption_shortcode_unique_ids_per_instance(): void {
383+
/*
384+
* Important: The id part of this must be unique among unit tests, or else the test will fail due to
385+
* wp_unique_prefixed_id() not returning the expected increment.
386+
*/
387+
$id = 'attachment_65315';
388+
$caption_id = 'caption_attachment_65315';
389+
390+
// First instance with caption "My caption".
391+
$result_1 = img_caption_shortcode(
392+
array(
393+
'width' => 20,
394+
'id' => $id,
395+
'caption' => 'My caption',
396+
'caption_id' => $caption_id,
397+
),
398+
self::IMG_CONTENT . 'My caption'
399+
);
400+
401+
// Second instance - identical to first.
402+
$result_2 = img_caption_shortcode(
403+
array(
404+
'width' => 20,
405+
'id' => $id,
406+
'caption' => 'My caption',
407+
'caption_id' => $caption_id,
408+
),
409+
self::IMG_CONTENT . 'My caption'
410+
);
411+
412+
// Third instance - same image, different caption.
413+
$result_3 = img_caption_shortcode(
414+
array(
415+
'width' => 20,
416+
'id' => $id,
417+
'caption' => 'Different caption',
418+
'caption_id' => $caption_id,
419+
),
420+
self::IMG_CONTENT . 'Different caption'
421+
);
422+
423+
// Extract the figure (caption wrapper) and caption text IDs from each instance.
424+
$figure_id_1 = $this->get_id_of_first_tag_with_class( $result_1, 'wp-caption' );
425+
$figure_id_2 = $this->get_id_of_first_tag_with_class( $result_2, 'wp-caption' );
426+
$figure_id_3 = $this->get_id_of_first_tag_with_class( $result_3, 'wp-caption' );
427+
$caption_id_1 = $this->get_id_of_first_tag_with_class( $result_1, 'wp-caption-text' );
428+
$caption_id_2 = $this->get_id_of_first_tag_with_class( $result_2, 'wp-caption-text' );
429+
$caption_id_3 = $this->get_id_of_first_tag_with_class( $result_3, 'wp-caption-text' );
430+
431+
// Figure IDs should all exist.
432+
$this->assertNotEmpty( $figure_id_1, 'First figure should have an ID' );
433+
$this->assertNotEmpty( $figure_id_2, 'Second figure should have an ID' );
434+
$this->assertNotEmpty( $figure_id_3, 'Third figure should have an ID' );
435+
436+
// Figure IDs should all be different (each instance gets unique ID).
437+
$this->assertNotSame( $figure_id_1, $figure_id_2, 'First and second figures should have different IDs even with identical content' );
438+
$this->assertNotSame( $figure_id_2, $figure_id_3, 'Second and third figures should have different IDs' );
439+
$this->assertNotSame( $figure_id_1, $figure_id_3, 'First and third figures should have different IDs' );
440+
441+
// The first Figure ID should be identical to the supplied ID, and the others should have unique suffixes.
442+
$this->assertSame( $id, $figure_id_1 );
443+
$this->assertSame( "$id-2", $figure_id_2 );
444+
$this->assertSame( "$id-3", $figure_id_3 );
445+
446+
// Caption IDs should all exist.
447+
$this->assertNotEmpty( $caption_id_1, 'First caption should have an ID' );
448+
$this->assertNotEmpty( $caption_id_2, 'Second caption should have an ID' );
449+
$this->assertNotEmpty( $caption_id_3, 'Third caption should have an ID' );
450+
451+
// Caption IDs should all be different (each instance gets unique ID).
452+
$this->assertNotSame( $caption_id_1, $caption_id_2, 'First and second captions should have different IDs even with identical content' );
453+
$this->assertNotSame( $caption_id_2, $caption_id_3, 'Second and third captions should have different IDs' );
454+
$this->assertNotSame( $caption_id_1, $caption_id_3, 'First and third captions should have different IDs' );
455+
456+
// The first Caption ID should have no.
457+
$this->assertSame( $caption_id, $caption_id_1 );
458+
$this->assertSame( "$caption_id-2", $caption_id_2 );
459+
$this->assertSame( "$caption_id-3", $caption_id_3 );
460+
}
461+
462+
/**
463+
* Returns the `id` attribute of the first tag bearing the given class name.
464+
*
465+
* @param string $html Markup to search.
466+
* @param string $class_name Class name to locate the tag by.
467+
* @return string|null The tag's `id` value, or null if no matching tag or `id` is found.
468+
*/
469+
private function get_id_of_first_tag_with_class( string $html, string $class_name ): ?string {
470+
$processor = new WP_HTML_Tag_Processor( $html );
471+
472+
if ( ! $processor->next_tag( array( 'class_name' => $class_name ) ) ) {
473+
return null;
474+
}
475+
476+
$id = $processor->get_attribute( 'id' );
477+
478+
return is_string( $id ) ? $id : null;
369479
}
370480

371481
public function test_add_remove_oembed_provider() {

0 commit comments

Comments
 (0)