Widgets editor - prevent creation of multiple reference widgets

[adamziel]

Describe the bug
As discussed in #24905, it is possible to create multiple reference widgets by leveraging multiple legacy widget blocks. There should only ever be at most one instance of any reference widget. Unfortunately supports.multiple=false only works for the block, not for it's variations.

[noisysocks]

I guess we need to decide how we want supports.multiple to work with variations. Any preference, @WordPress/gutenberg-core?

[talldan]

Is the idea that supports.multiple would work for only some variations of the legacy widget block?

If so, I'm not sure supports.multiple could work, as a variation is only really a bunch of preset attributes for a block that's yet to be created. Once a variation is inserted it ceases to be a variation. It'd be hard to check whether an instance of that variation exists in order to prevent more than one using the built in supports.multiple system.

Consider a block where all the attributes can be changed from the values set initially by the variation, it'd be impossible to tell it was originally created using a variation.

Probably need to create something ad-hoc for widgets that checks for instances of reference widgets in the content and then unregisters the matching variations.

[adamziel]

Probably need to create something ad-hoc for widgets that checks for instances of reference widgets in the content and then unregisters the matching variations.

This unfortunately wouldn't cover block duplication or copying&pasting.

[adamziel]

getBlockSupport operates on the block name/type level, not on specific block instances:

https://github.com/WordPress/gutenberg/blob/master/packages/blocks/src/store/selectors.js#L212-L225

The way I see it is that we have three options:

1. Add a new set of checks tailored to this use-case to every place where supports.multiple is used.
2. Alter the way supports.multiple works so that it's able to reason about specific instances.
3. Register each reference widget as a separate block with supports.multiple = false.

1 and 2 could turn into rather large refactoring tasks, and while not impossible, I'm curious if 3 would work for us here.

[adamziel]

#25741 attempts to register each reference widget as a separate block.

[talldan]

This unfortunately wouldn't cover block duplication or copying&pasting.

Oh, good point. Multiple blocks makes sense, it's the way widgets used to work so there's precedence.

[youknowriad]

Creating blocks has several implications and for me sounds too heavy-handed to solve this particular issue which seems rather smallish. I fear the unintended consequences.

[adamziel]

@youknowriad although I had a feeling this might be the concern, I don't have a good understanding of all the implications - would you mind elaborating?

Also, I would appreciate your input about possible alternatives. hasBlockSupport( blockName, 'multiple', true ) is used in the following places:

gutenberg/packages/block-editor/src/store/selectors.js

Line 1445 in 1719dc2

if ( ! hasBlockSupport( blockType.name, 'multiple', true ) ) {

gutenberg/packages/block-editor/src/store/actions.js

Line 970 in 16e98d4

( blockName ) => ! hasBlockSupport( blockName, 'multiple', true )

gutenberg/packages/block-editor/src/components/block-actions/index.js

Line 38 in f7b9201

hasBlockSupport( block.name, 'multiple', true ) &&

gutenberg/packages/edit-post/src/hooks/validate-multiple-use/index.js

Line 36 in 4857ad5

const multiple = hasBlockSupport( block.name, 'multiple', true );

The options I considered were:

• Adding some additional, pluggable logic to each of these places. Like hasBlockInstanceSupport( block, 'multiple' ). This would effectively break the contract of hasBlockSupport as the true return value would no longer mean that the block really supports the feature.
• Refactoring hasBlockSupport to require block instance in addition to block type. Then use that instance to perform additional checks. I can see a few problems with that: 1) it would break the stable API 2) it would be a rather large refactoring with risk for errors as it would touch all hasBlockSupport checks.
• Making multiple something else than part of supports. It seems forced and again we'd be breaking stable API.

I don't like any of these options and I wasn't able to come up with anything better than registering one block per widget. I would really appreciate your thoughts and ideas.

Too heavy-handed to solve this particular issue

Just noting that we would have granular control over all supports options, not just multiple. Some folks requested the ability to edit widgets as html and supports.html makes sense for html/text widgets but not for e.g. calendar.

[talldan]

Looking at all the options, I still think multiple blocks is the easiest and safest option.

Although it'd be great to make supports.multiple work I can't see how it could be done easily. An instance of a variation can technically be changed completely from when it was inserted to no longer represent the variation. I think to make supports.multiple work at that level it'd have to be able to determine the variation name from the block markup, which means either including the variation name in the output somewhere, or making supports.multiple able to use a dependency list of attributes it can check for uniqueness, e.g.:

supports: {
  multiple: [ 'widgetName' ]
}

[adamziel]

After further discussion @youknowriad reaffirmed that registering a block per reference widget is not the right tool for the job and suggested an alternative of simply displaying an error message in duplicate instances. #26148 is an attempt at that.

The API part of the equation is a different question. I can see two ways to proceed here:

• Update the API such that saving each reference widget means removing it from other sidebars
• Do not update the API, acknowledge that saving multiple instances of reference widgets is a footgun, agree this is okay as the REST API relies on the PHP API which has the same footgun so developers have to be careful either way

[carlomanf]

Footgun? How so? The example you gave doesn't look like much of a footgun other than it overwrites the content in other instances of the widget. This could easily be addressed with a simple warning message. "Warning: the edits you make will take effect in all instances of the Marquee widget."

If you have a better example, please share it, but I'm yet to see any examples of multiple reference widgets causing side effects any more serious than that.

That said, even if you do deem #26148 to solve this, then it still calls #25371 into question. #25371 heavy-handedly sacrificed an important feature largely on the grounds of avoiding this "footgun" that #26148 now concedes can't be avoided.