WordPress.org

Make WordPress Core

Opened 14 months ago

Closed 13 months ago

Last modified 11 months ago

#50706 closed defect (bug) (wontfix)

Block Editor: Apply allowed_block_types filter to registered blocks

Reported by: Bernhard Reiter Owned by: youknowriad
Milestone: Priority: normal
Severity: normal Version:
Component: Editor Keywords: has-patch
Focuses: Cc:

Description

The `allowed_block_types` hook filters a list of blocks. Alternatively, it's possible to pass a bool to indicate that either all blocks are allowed (true), or none of them are (false).

It's currently applied in lib/edit-form-blocks.php to a default value of true -- meaning all blocks are allowed.

However, this means that the filter can only really be used for an allowlist, but not for a denylist: Since we don't really have access to the full list of available blocks from the filter (but just that true bool), we cannot choose to remove a set of blocks from that list.

I'm thus suggesting to pass the full list of registered blocks to the filter as the default value instead.

Change History (19)

This ticket was mentioned in PR #419 on WordPress/wordpress-develop by ockham.


14 months ago

  • Keywords has-patch added

The `allowed_block_types` hook filters a list of blocks. Alternatively, it's possible to pass a bool to indicate that either all blocks are allowed (true), or none of them are (false).

It's currently applied in lib/edit-form-blocks.php to a default value of true -- meaning all blocks are allowed.

However, this means that the filter can only really be used for an allowlist, but not for a denylist: Since we don't really have access to the full list of available blocks from the filter (but just that true bool), we cannot choose to remove a set of blocks from that list.

I'm thus suggesting to pass the full list of registered blocks to the filter as the default value instead.

Trac ticket: https://core.trac.wordpress.org/ticket/50706

#3 @SergeyBiryukov
14 months ago

  • Component changed from General to Editor

#4 @prbot
14 months ago

ockham commented on PR #419:

cc/ @TimothyBJacobs @youknowriad

#5 @prbot
14 months ago

TimothyBJacobs commented on PR #419:

Makes sense to me!

#6 @prbot
14 months ago

ockham commented on PR #419:

No objections from me either but curious about backward compatibility implications.
it also seems we'd have to wait for trunk to target 5.6 to land this.

I _think_ this should be mostly backward compatible, since the hook always accepted either an array of block names, or a bool, so filters attached to the hook would have to check with which of either they were dealing. In practice, they couldn't count on getting an array from which they could remove something, so the filter was only used for allow lists rather than deny lists. (As e.g. the docs show.) So I think we're adding functionality without really impacting existing usage.

Furthermore, the filter is applied right before the $settings var is passed to the client, so all server-side block registrations can be expected to have happened by this time.

LMK if you're thinking of a scenario that I'm missing :thinking:

#7 @youknowriad
14 months ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 5.6

#8 @youknowriad
14 months ago

  • Owner set to youknowriad
  • Status changed from new to accepted

#9 @prbot
14 months ago

ockham commented on PR #419:

Ping @TimothyBJacobs and @youknowriad -- Are we cool to merge this? :smile:

#10 @prbot
14 months ago

youknowriad commented on PR #419:

I've been thinking more here and I think we should not move forward with this. The reason being that all blocks are not registered on the server, so as implemented here, it means all the blocks registered on the frontend are not allowed by default.

#11 @prbot
13 months ago

vindl commented on PR #419:

@ockham sounds like we should close out this PR?

This ticket was mentioned in Slack in #core by helen. View the logs.


13 months ago

#13 @helen
13 months ago

  • Keywords commit removed

Changing keywords here just for tracking - @youknowriad please close if not moving in this direction.

#14 @helen
13 months ago

  • Milestone 5.6 deleted
  • Resolution set to wontfix
  • Status changed from accepted to closed

#15 @prbot
12 months ago

ockham commented on PR #419:

I've been thinking more here and I think we should not move forward with this. The reason being that all blocks are not registered on the server, so as implemented here, it means all the blocks registered on the frontend are not allowed by default.

@youknowriad That makes sense, but it also limits the potential use cases of the server-side filter significantly: It can basically never be used as a denylist. IMO, this raises the question of the general usefulness of this filter -- since there are also client-side mechanisms to _both_ register or unregister a block. Should we:

  1. prescribe that blocks _must_ be registered on the server side?
  2. deprecate the allowed_block_types hook?

#16 @prbot
12 months ago

TimothyBJacobs commented on PR #419:

Yeah the only times I've been able to use allowed_block_types is when there are a very specific set of blocks allowed which is rarely what you want. Maybe add a disabled_block_types filter?

#17 @prbot
12 months ago

ockham commented on PR #419:

Yeah the only times I've been able to use allowed_block_types is when there are a very specific set of blocks allowed which is rarely what you want. Maybe add a disabled_block_types filter?

Yeah, that could work :thinking: It might not perfectly cover cases where e.g. you'd like to remove all blocks in a certain namespace (rather than listing them explicitly, because e.g. they might come from a plugin that has added more blocks in more recent versions), but I guess it's a start...

#18 @prbot
12 months ago

TimothyBJacobs commented on PR #419:

We could support including a namespace in the list, so both my-plugin and other-plugin/block.

#19 @prbot
11 months ago

hellofromtonya commented on PR #419:

Closing this PR as the associate Trac ticket https://core.trac.wordpress.org/ticket/50706 was closed 7 weeks ago.

If any maintainer or committer feels this ticket and PR should be reopened, please update accordingly.

Note: See TracTickets for help on using tickets.