Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Synced Pattern Overrides: Image metadata like data-permalink, data-image-title, keeps original image data #62886

Closed
perezcarreno opened this issue Jun 26, 2024 · 11 comments · Fixed by #63013
Assignees
Labels
[Feature] Block bindings [Type] Bug An existing feature does not function as intended

Comments

@perezcarreno
Copy link

Description

When you create a synced pattern and include an image override, the original image's metadata will be shown in the HTML even when you've chosen a new image in a pattern's instance. So if the image was called 'my-image-placeholder.jpg", that is the data that will be shown in the code, even if the displayed image is "bird-graphic.jpg".

For example:
<img decoding="async" data-attachment-id="28" data-permalink="https:/website.com/placeholder-image-11/" data-orig-file="https://website.com/wp-content/uploads/2024/03/woocommerce-placeholder.png" data-orig-size="800,800" data-comments-opened="1" data-image-meta="{&quot;aperture&quot;:&quot;0&quot;,&quot;credit&quot;:&quot;&quot;,&quot;camera&quot;:&quot;&quot;,&quot;caption&quot;:&quot;&quot;,&quot;created_timestamp&quot;:&quot;0&quot;,&quot;copyright&quot;:&quot;&quot;,&quot;focal_length&quot;:&quot;0&quot;,&quot;iso&quot;:&quot;0&quot;,&quot;shutter_speed&quot;:&quot;0&quot;,&quot;title&quot;:&quot;&quot;,&quot;orientation&quot;:&quot;0&quot;}" data-image-title="Placeholder Image" data-image-description="" data-image-caption="" data-medium-file="https://website.com/wp-content/uploads/2024/03/woocommerce-placeholder.png" data-large-file="https://website.com/wp-content/uploads/2024/03/woocommerce-placeholder.png" src="https://website.com/wp-content/uploads/2024/06/V3P100-129fig5.jpg" alt="Figure 5. A histogram plot showing distribution of missing death data according to age group" class="wp-image-28" style="object-fit:cover">

P.S. I might open another issue if I have time, but so I don't forget, the same thing happens if you had enabled a link in the image after creating the override. The best example being the "attachment" link which opens the image in a lightbox. It also shows the placeholder image in the lightbox instead of the current instance's image.

Thanks!

Step-by-step reproduction instructions

  1. Create a synced pattern that contains an image block.
  2. Enable overrides on the image block.
  3. Place the synced pattern on a post or page.
  4. Replace the image with another image.
  5. Load the page/post and view the HTML code.

Screenshots, screen recording, code snippet

Screenshot 2024-06-26 at 11 45 07 AM

Environment info

WordPress 6.5.5, Gutenberg 18.6.0

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@perezcarreno perezcarreno added the [Type] Bug An existing feature does not function as intended label Jun 26, 2024
@ndiego ndiego added the [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) label Jun 26, 2024
@ndiego
Copy link
Member

ndiego commented Jun 26, 2024

Are there any plugins active other than Gutenberg? In my testing, I do not see any of these data attributes in a local install. However, I so see them when Jetpack is enabled and the Media settings are enabled.

image

cc @jeherve

@perezcarreno
Copy link
Author

Yes, you are correct, I apologize. Jetpack is enabled. This particular client site is hosted on wp.com, not something I usually work with, and they silently force certain plugins on you. I forgot to make sure it was disabled.

@ndiego
Copy link
Member

ndiego commented Jun 26, 2024

No worries, this likely has implications for any third-party plugin that modifies block content before it's rendered.

@perezcarreno
Copy link
Author

Yes, I agree. It is not a bug in Gutenberg then, apologies. Thanks for looking into it, though!

@talldan talldan closed this as not planned Won't fix, can't repro, duplicate, stale Jun 27, 2024
@talldan
Copy link
Contributor

talldan commented Jun 27, 2024

Not sure if you created an issue already @perezcarreno, but the jetpack repo is here - https://github.com/Automattic/jetpack/issues.

I'd be curious to know if the issue only happens with the gutenberg plugin active, or if it still happens in the WP6.6 beta. There's a few small differences with the way the gutenberg plugin handles block bindings (the feature that pattern overrides is built on) compared to WordPress core. Gutenberg only overrides the block attributes like the image id later in the block rendering process, so it might be that jetpack doesn't have access to the correct image id.

@jeherve
Copy link
Contributor

jeherve commented Jun 27, 2024

Not sure if you created an issue already @perezcarreno, but the jetpack repo is here - https://github.com/Automattic/jetpack/issues.

I created an issue to keep track of this here:
Automattic/jetpack#38081

I'd be curious to know if the issue only happens with the gutenberg plugin active, or if it still happens in the WP6.6 beta.

The root issue is present with both as far as I can tell, but is only visible to visitors when Gutenberg is active. When Gutenberg is inactive, you cannot link an image inside a synced pattern to its attachment page:

image

As a result, Jetpack's Carousel will not be triggered.

it might be that jetpack doesn't have access to the correct image id.

That seems to be the root of the problem here. In the post content, the block with a replaced image would be saved like so:

<!-- wp:block {"ref":40,"content":{"original":{"id":44,"alt":"","url":"https:/mysite.com/wp-content/uploads/2024/06/replaced-1024x383.png"}}} /-->

Where 44 is the post ID of the new attachment (the replaced.png image).

However, some third-party plugins like Jetpack only parse the post content using a filter like the_content, instead of fetching the raw data saved in the db. By that point, the parsed content doesn't use the correct post ID:

<figure class="wp-block-image size-large"><img src="https://my-site.com/wp-content/uploads/2024/06/replaced-1024x383.png" alt="" class="wp-image-39"/></figure>

39 is the post ID of the original attachment.

Would there be a way to fix this in Core?

@talldan talldan added [Feature] Block bindings and removed [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) labels Jun 27, 2024
@talldan
Copy link
Contributor

talldan commented Jun 27, 2024

I'll reopen the issue as it seems worth discussing further, and I don't want it to get lost.

It's also something that could happen with any block binding, including if the user is binding an image block using post meta, so I'm re-labelling the issue.

When Gutenberg is inactive, you cannot link an image inside a synced pattern to its attachment page

That will land in the next release of the plugin as well, though I don't think that the image needs a link to be able to reproduce the issue.

39 is the post ID of the original attachment.

Does jetpack use the wp-image-39 class name to grab the image id? If so, then block bindings currently doesn't update that class name, it's set statically by the image block client side and so is not modified by block bindings (which is a process that happens server-side). A fix could be for the block to compute the classname dynamically on the server.

@talldan talldan reopened this Jun 27, 2024
@jeherve
Copy link
Contributor

jeherve commented Jun 27, 2024

Thanks for taking another look!

Does jetpack use the wp-image-39 class name to grab the image id?

Yes, that's how its Carousel feature currently works for single images.

A fix could be for the block to compute the classname dynamically on the server.

It seems like it would make sense. I think it's a fair expectation to have the class name matching the image displayed on the page.

@perezcarreno
Copy link
Author

perezcarreno commented Jun 27, 2024

The root issue is present with both as far as I can tell, but is only visible to visitors when Gutenberg is active. When Gutenberg is inactive, you cannot link an image inside a synced pattern to its attachment page:

image As a result, Jetpack's Carousel will not be triggered.

Thank you, @jeherve ! I haven't tested it on 6.6, but with Gutenberg the way that I added the link to the image was after creating the override. In other words, the "Overrides currently don't support image captions or links" legend is not shown to the user because the link isn't there yet. Then, once the override is set, I went in and added the link. I don't know if this helps, but I thought I'd mention it.

@talldan
Copy link
Contributor

talldan commented Jul 1, 2024

Similar to the issue with the wp-image-123 classname, there's also a data-id used in galleries that has the incorrect value when overriden using pattern overrides. The current version of the data-id was implemented in #35975 for back compat purposes. This one's a little different, the gallery block injects an extra data-id block attribute into the image block, which is something that block bindings doesn't know about, so it retains the original value.

I'll try to fix that at the same time, as I see from the PR it's used by some themes.

@talldan
Copy link
Contributor

talldan commented Jul 2, 2024

I've made a PR that should hopefully fix this - #63013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block bindings [Type] Bug An existing feature does not function as intended
4 participants