Media & Text block: Remove the link option when the featured image is used

[carolinan]

What & Why & How?

When the Media & Text block is linked and uses the featured image, the image is not linked on the front of the site.

Originally, this PR was meant to make the link work.
The preferred way to solve the bug, as expressed in the reviews below, would be to update the LinkControl component and replace ImageURLInputUI.

It was then recommended to solve the bug short-term by not showing the link option if the featured image is used, so that
the user can not use the broken control.
The PR was updated to reflect this.

Updated testing instructions:

1. Activate Gutenberg
2. Create a new post and add a featured image.
3. Add a Media & Text block and select the option "Use featured image".
4. In the block toolbar, confirm that there is no link option.
5. Select another image from the media library.
6. In the block toolbar, confirm that there is a link option, set a link. Save and view the front, confirm that the link is working.
7. Select the featured image again, save and view the front. confirm that there is no link being output. This step is meant to to test that the link that was set in step 6 was properly unset (reset) when the featured image was selected.

Old description

The pull request updates where in the block markup the `img` tag is inserted:

1. It checks wether the block $content contains an <a> tag.
2. If there is an <a> tag, it inserts the <img> tag by replacing the closing </a> tag with <img></a>.
   By replacing the closing </a> instead of the opening tag, the class names and attributes on the link are preserved.
3. If the block $content does not contain a link, the <img> tag is inserted directly inside the <figure>.

Closes #60497

Limitations

The PR does not address the problem that users can not set individual links on the block when it is placed inside a query loop.

Testing Instructions

1. Activate Gutenberg
2. Create a new post and add a featured image.
3. Add a Media & Text block and select the option "Use featured image".
4. In the block toolbar, select the link option and add a link.
5. Save and view the front.
6. Confirm that the image is linked.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: carolinan <poena@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: draganescu <andraganescu@git.wordpress.org>
Co-authored-by: jasmussen <joen@git.wordpress.org>
Co-authored-by: richtabor <richtabor@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Co-authored-by: fabiankaegy <fabiankaegy@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[carolinan]

With this PR, if the featured image is linked but has no alt text, VoiceOver on Mac will read the URL is the link text.

I think that is the best that can be done unless we want to add the "Link to post" setting (the same setting that the featured image block has).

[t-hamano]

Thanks for the PR!

I think the difficult part of this problem is finding the link of the featured image when the block contains multiple a elements.

For example, when "Show media on the right" is enabled and a link exists in the content, the screenshot and the markup will look like this:

<!-- wp:media-text {"mediaPosition":"right","linkDestination":"custom","mediaType":"image","imageFill":false,"useFeaturedImage":true} -->
<div class="wp-block-media-text has-media-on-the-right is-stacked-on-mobile">
	<div class="wp-block-media-text__content">
		<!-- wp:paragraph {"placeholder":"Content…"} -->
		<p>Hello <a href="#">World</a></p>
		<!-- /wp:paragraph -->
	</div>
	<figure class="wp-block-media-text__media">
		<a href="#test"></a>
	</figure>
</div>
<!-- /wp:media-text -->

On the front end side, the following invalid markup is output:

<div class="wp-block-media-text has-media-on-the-right is-stacked-on-mobile">
	<div class="wp-block-media-text__content">
	  <p>Hello 
	    <a href="#">World
	      <img decoding="async" width="1024" height="683" alt="" class="wp-image-8 size-full" src="http://localhost:8888/wp-content/uploads/2024/04/test_1.jpg" srcset="http://localhost:8888/wp-content/uploads/2024/04/test_1.jpg 1024w, http://localhost:8888/wp-content/uploads/2024/04/test_1-300x200.jpg 300w, http://localhost:8888/wp-content/uploads/2024/04/test_1-768x512.jpg 768w" sizes="(max-width: 1024px) 100vw, 1024px" />
	    </a>
	  </p>
	</div>
	<figure class="wp-block-media-text__media">
	  <a href="#test">
	    <img>
	  </a>
	</figure>
</div>

I'm wondering how many users will want to take advantage of the custom link when they enable the featured image on this block. For example, when this block is inserted inside a Query Loop block and the link is enabled, I expect the featured image to automatically link to that post.

In any case, we need to find a way to detect the correct link. Ideally, it would be nice if we could specify the a element inside the .wp-block-media-text__media class, but as far as I know, WP_HTML_Tag_Processor does not allow us to specify the parent element 😅

[carolinan]

Thank you for lending me your brain!
I'll try this option:

I think that is the best that can be done unless we want to add the "Link to post" setting (the same setting that the featured image block has).

[carolinan]

• The link control in the toolbar is hidden when the featured image is selected.
• A new toggle option in the sidebar, "link to (Post type name)" is available when the featured image is selected.
• There is a new attribute called "featuredImageLink" that is true when the option is enabled. This name could be improved.

index.php:

• Use the post title as the alternative text when the featured image is linked, if a post title is set.
• Use the alternative text from the media library if the image is not linked and the text is set.
• Use the WP_HTML_Tag_Processor to find the HTML tag with the class name wp-block-media-text__media instead of finding just any <figure> tag in the block content.
  This is because the content part of the block can include an image block that also uses a figure element, and the src and class names etc would override the first image block. 🙀
  There may be more scenarios like that, that I have not found yet.

What is still missing:

While I could add conditional options for changing the linkTarget and rel, when the link to post option is enabled, I was not able to figure out how to pass those two attributes to the index.php file.
They don't show up in $attributes so they were probably not updated correctly.

[t-hamano]

Thanks for the update!

While I could add conditional options for changing the linkTarget and rel, when the link to post option is enabled, I was not able to figure out how to pass those two attributes to the index.php file.
They don't show up in $attributes so they were probably not updated correctly.

This is because those attributes are not saved as HTML comments, but as part of the block HTML. If you remove the source, selector, and attribute from those attributes, the data will be saved as a HTML comment and can be referenced in the render_callback() function. If ywe do this we will need to add a deprecation. Alternatively, we could parse the media link element using the HTML Tag Processor and retrieve the attributes via get_attribute() mothod.

---

BTW, if I think about this problem again, it may be more difficult than I imagine.

@draganescu @aaronrobertshaw I have summarized the problems and issues that I understand below, so if you have a good approach, please let me know 🙏

• Originally reported in #60497, an issue where the custom link was not applied correctly on the front end when "Use Featured Image" was enabled for the Media & text block.
• In this PR, if "Use Featured Image" is enabled, the custom link will be disabled and a "Link to Post" control will be added to the block sidebar. This is because the ImageURLInputUI component used in this block does not accept custom settings.
• The custom link supports "Open in new tab", "Link Rel", and "Link CSS Class" in addition to the URL. These UIs need to be added to the block sidebar so that these settings can still be controlled when the featured image is linked to a post.

Considering these, I think the UI will probably look something like the following, what do you think? I'm concerned that the UI location for setting the link changes depending on whether the featured image is enabled or disabled.

54554e69874d98c8b21bc0be51ca3764.mp4

[carolinan]

I have the same concerns.
There is more though, - what if we eventually want to add the lightbox? It will be even more confusing with separate settings.

[carolinan]

I opened this issue, but I'm not sure I described the problem clearly enough:
Link control: Make it possible to link to the current post when in a query loop
#60499

[carolinan]

If all blocks that use the featured image link option could move it to the link setting in the toolbar, it would be more consistent with how the "lightbox linking exception" works. I mean the exception where you can only choose one link or the other.

But short term, what can we do to fix this rather large bug as soon as possible?

[draganescu]

I agree w/ @jasmussen about definitely not moving linking to the inspector - I think it would be a very isolated UX (an exception from how we generally link). I would also add that if we link to post what is the point of rel and target which make sense for external links?

[richtabor]

In #50893 I was thinking how link editing would consolidate across WordPress.

It could be appended to the list in the link UI as pictured above, but I'd like to move forward on using LinkControl throughout the experience, to reduce ad hoc fixes/implementations.

[carolinan]

Yes I have intentionally avoided creating yet another custom and temporary link control.

I still do think that this bug needs to be fixed and not get blocked.

[github-actions]

This pull request changed or added PHP files in previous commits, but none have been detected in the latest commit.

Thank you! ❤️

[github-actions]

Flaky tests detected in e0d8001.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9205311072
📝 Reported issues:

• [Flaky Test] Clicking on inserter while on zoom-out should open the patterns tab on the inserter #61806 in /test/e2e/specs/site-editor/zoom-out.spec.js

[carolinan]

Is this solution acceptable until someone is able to re-work the LinkControl? What other options are there besides disabling the link settings or reverting the featured image option? I don't want this to go broken into 6.6.

[carolinan]

If the link option is disabled when the featured image is used, using the featured image in the media and text block inside a query is kind of pointless.
It would still need to manage the broken links already added by users who have had Gutenberg active.

[draganescu]

Given the feedback that ideally we move everything to the linking UI and since this is a bug fix what do y'all think if we just hide for now the linking option if the featured media image is used in the media text block?

I understand this reduces the usefulness of the featured image prop in the media text block, but it'd be a temporary solution that avoids yet another deprecation, and hundreds of documents updated when linking will happen in the toolbar.

I say this PR should only fix the bug and we then open a separate issue and PR to introduce link to post to link UI?

[carolinan]

I'm not fully convinced that having the link option in the sidebar is more disruptive than not having a link option at all, but since this is the only feedback received I will try to update the PR so that this can be solved, short term.

[draganescu]

@carolinan I am not convinced either 100% 😁 but I lean towards removing linking altogether because the featured image prop of the media text block is new (will land in WP next), and we found a limitation of our linking system, so I generally default to less instead of more to undo later.

[carolinan]

I still want to test that there is no "weirdness" for people who has used Gutenberg and added links.
For those users, the block attributes related to the link won't update unless they toggle the featured image off...

[carolinan]

For blocks that had the link set before this PR, the link is still output in the wrong position on the front:

<div class="wp-block-media-text is-stacked-on-mobile"><figure class="wp-block-media-text__media">
<img decoding="async" width="295" height="295" alt="" class="wp-image-201 size-full" src="..." 
sizes="(max-width: 295px) 100vw, 295px"><a href="https://wordpress.org"></a></figure>
<div class="wp-block-media-text__content">
<p>This Media &amp; Text block was linked on Gutenberg trunk.</p>
</div></div>

Do I need to try to remove it?

[draganescu]

For blocks that had the link set before this PR, the link is still output in the wrong position on the front [...] Do I need to try to remove it?

I would lean into no - using the plugin in production is precisely subject to these kinds of issues. #51491 introduced using the featured image for the media and text block which is a feature only available starting WP 6.6.

If the bug landed in a WP release (minor or major) a minor release to remove this from where it was output (via some filter) would be justified. But since the bug existed only in the plugin, adding some logic to strip that out would be then shipped forever for a tiny fraction of cases.

cc @Mamaduka is my reasoning above correct?

[Mamaduka]

@draganescu, yes, the reasoning makes sense. There is no need to introduce backward compatibility handers for code that has only ever existed in plugins.

Also, if the content had been saved in DB, it would've been a different story, but IIRC, markup is dynamic for featured images.

[draganescu]

This LGTM. Let's make an issue if it's not made to add "link to current post" the link UI somehow.