Skip to content

Use the alt text that clients send - #335

Merged
pfefferle merged 1 commit into
trunkfrom
fix/alt-text-is-discarded
Aug 26, 2026
Merged

pfefferle merged 1 commit into
trunkfrom
fix/alt-text-is-discarded

Conversation

@pfefferle

Copy link
Copy Markdown
Member

Summary

insert_attachment() has always taken a third argument and never used it. Both of its callers pass something, and both were losing it.

media_sideload_url() gets the alt of a media value. default_file_handler() reads it off the request:

$desc = is_array( $val ) ? \mp_get( $val, 'alt', null ) : null;
$media_controller->media_sideload_url( $url, $post_id, $desc );

so a client that describes its photo has that description carried through two calls and then dropped. I noticed this while writing the test for the alt is optional fix in #330: there was nothing to assert on, because alt does not reach anything.

The media endpoint passes $request->get_param( 'name' ), which is a title rather than a description, and lost it the same way.

These are two different things sharing one argument, so they are now two arguments. $title sets post_title, after the EXIF title so that an explicit name wins, and before the filename fallback. $alt goes to _wp_attachment_image_alt, which is where WordPress looks for alternative text.

media_sideload_url()'s third parameter is renamed from $title to $alt and forwarded as alt. It is a public method, but the argument has been inert since it was written, so nothing can be relying on the old meaning.

The mf2_photo meta still holds a plain URL rather than the { value, alt } object. That seems right to me: the alt now lives on the attachment, which is the WordPress-native place to look it up, and changing the shape of that meta would break anything reading it today.

Tests

Six tests in class-test-media.php. Four fail without the change, two are guards that pass either way.

  • A sideload stores the alt text
  • A sideload strips tags from it, as core does when saving alt
  • A sideload without alt stores no alt text (guard)
  • The alt survives the whole way from a create request to the attachment, not just the last call
  • An upload with name uses it as the attachment title
  • An upload without name still falls back to the filename (guard)

serve_test_image() moves to Micropub_UnitTestCase so it is not written a third time. #334 added a copy in class-test-endpoint-media.php.

Test plan

  • vendor/bin/phpunit in wp-env: 116 tests, the 11 failures and 2 errors are the pre-existing example.org ones
  • Each new test checked against trunk
  • composer lint
  • CI green on 7.4, 8.3 and 8.5

@pfefferle
pfefferle requested a lite review from Copilot August 24, 2026 18:39
@pfefferle pfefferle self-assigned this Aug 24, 2026
@pfefferle
pfefferle requested a review from a team August 24, 2026 18:39

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR ensures media metadata sent by Micropub clients is actually persisted: attachment alt text provided in media objects now reaches WordPress’ _wp_attachment_image_alt, and the media endpoint’s name parameter is used as the attachment title.

Changes:

  • Extend Media_Controller::insert_attachment() to accept and save both a $title (to post_title) and an $alt (to _wp_attachment_image_alt), and repurpose media_sideload_url()’s third argument to mean alt text.
  • Add/expand PHPUnit coverage to assert alt text storage (including tag stripping) and correct title behavior for uploads with/without name.
  • Deduplicate test infrastructure by moving serve_test_image() into Micropub_UnitTestCase.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
includes/rest/class-media-controller.php Persist attachment title and alt text during upload/sideload flows; rename/repurpose sideload parameter to represent alt text.
tests/phpunit/includes/class-micropub-unit-test-case.php Move serve_test_image() helper to shared base test case for reuse.
tests/phpunit/tests/class-test-endpoint-media.php Remove duplicated helper now provided by the base test case.
tests/phpunit/tests/class-test-media.php Add tests covering alt persistence/tag stripping and upload title behavior driven by name.
Suppressed comments (2)

includes/rest/class-media-controller.php:423

  • $alt is currently gated by a truthy check. In PHP, the string '0' is falsey, so a client-provided alt text of '0' would not be saved. Consider checking for null instead so all valid strings are stored when the parameter is provided.
		if ( $alt ) {
			\update_post_meta( $id, '_wp_attachment_image_alt', \wp_slash( \wp_strip_all_tags( $alt ) ) );
		}

includes/rest/class-media-controller.php:515

  • When attachment_url_to_postid() finds an existing attachment, the function returns early after setting post_parent, and the new $alt parameter is ignored. That means clients still can't set alt text for already-sideloaded media (including attachments created before this fix). Consider applying $alt here too; to avoid overwriting an existing curated alt, only set it when the attachment currently has no alt.
		$id = \attachment_url_to_postid( $url );
		if ( $id ) {
			\wp_update_post(
				array(
					'ID'          => $id,

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

includes/rest/class-media-controller.php:516

  • When attachment_url_to_postid() finds an existing attachment, media_sideload_url() returns early without applying the provided alt text. If callers pass alt, it should be persisted for existing attachments too (at least when $alt !== null).
		if ( $id ) {
			\wp_update_post(
				array(
					'ID'          => $id,
					'post_parent' => $post_id,

@pfefferle
pfefferle merged commit 80627b1 into trunk Aug 26, 2026
6 checks passed
@pfefferle
pfefferle deleted the fix/alt-text-is-discarded branch August 26, 2026 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants