Pattern overrides: Button and Image blocks sharing the same name have incompatible attributes

[talldan]

Description

When pattern overrides are enabled for a block, the block name becomes the key that attribute data is stored under in instances of the pattern block. For example, a button block called "CTA" would store it's url attribute value in a pattern block's content like so:

content: { "CTA": { "url": "https://wordpress.org" } }

Another feature of pattern overrides is that two blocks can share the same name, so two paragraphs, or a paragraph and a heading can have their text content synced, with the data being stored in the same object.

A problem occurs when a user gives a button block and an image block the same name. The image block has the following attributes:

• url - used for the <img> tag's src attribute
• href - used for the optional <a> tag's href attribute

While the button block has:

• url - used for the optional <a> tag's href attribute

For image and button url means two different things. If an image and a button share the same override name, it can result in a broken image src when the user sets the button block's link. It's unfortunate that the button block's attribute isn't called href.

Another factor is that the image block's href isn't currently a supported attribute for pattern overrides / block bindings.

Step-by-step reproduction instructions

1. Add an image and a button to a post
2. Select both and create a pattern
3. Enable overrides for both the image and the button, and use the same block name.
4. Save the pattern and go back to the post
5. Set the button block's link to something that's not an image url (e.g. https://wordpress.org)

Expected: The link for the image is set to the same thing given the two blocks are named the same
Actual: The image block has a broken image

Screenshots, screen recording, code snippet

Environment info

No response

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

[talldan]

@youknowriad Not sure if you recall, but we discussed this issue in the past (though I can't remember which PR we discussed it on now).

I've put together a PR that renames the button block attributes to see exactly what it would entail - #62612.

It ends up being quite a big change. 😬

I'm not sure if there's anything else we can do to mitigate the issue before 6.6..

An alternative could be to prevent users from using the same name for button and image blocks. I can look into that. What do you think?

[youknowriad]

I've put together a PR that renames the button block attributes to see exactly what it would entail - #62612.

I'm not entirely sure we should be doing this (renaming the attribute) as the issue can come up with any block types really. And for these particular case, both of these are urls but it's not entirely clear why they should share the same name.

An alternative could be to prevent users from using the same name for button and image blocks. I can look into that. What do you think?

Maybe if it's not that complex but it could also be considered a bug to be solved later. I think ultimately we could offer a way to "map" to specific attributes in the actual binding configuration. Something like "bindingName": "href". (A way to rename the attribute at the binding level). We may want to offer this possibility later at the code level for third-parties/advanced use-cases but UI wise, it's not clear we should be doing anything.

[mtias]

I think it's fine to not address this right now. We could render a warning in list view or somewhere else if we detect two different block types with the same name. Maybe on save flow as well.

For more complex setups I think we can make it possible, but not super explicit (bound per attribute at the code level but not UI, as @youknowriad mentions).

This is a good example of the "Simple things should be easy and intuitive, and complex things possible."

The scenario described here is a complex one, we should find ways to make it possible, but the straightforward cases should be super easy, which is where the name as key comes into play — it's less concepts, more straightforward.

[talldan]

Thanks for the comments - that adds some confidence that it's ok to bump this to 6.7/later and think about more general solutions 👍