From 6a843df867ab571926cabcc15a4a855dd4c93e5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1vid=20Szab=C3=B3?= Date: Fri, 14 May 2021 15:07:27 +0200 Subject: [PATCH] Check for allowed blocks recursively in patterns (#30366) Currently, patterns show up in the inserter even if they contain a block that's not on the allowed blocks list. Blocks aren't checked recursively in the patterns, only top level blocks. Even though they show up, we can't insert them since the blocks are disallowed. This PR checks recursively for disallowed blocks. --- package-lock.json | 1 + packages/block-editor/package.json | 1 + .../block-pattern-setup/use-patterns-setup.js | 16 ++- .../use-transformed-patterns.js | 16 ++- packages/block-editor/src/store/selectors.js | 103 +++++++++++++----- .../allowed-patterns-disable-blocks.php | 24 ++++ .../e2e-tests/plugins/allowed-patterns.php | 32 ++++++ .../editor/various/allowed-patterns.test.js | 74 +++++++++++++ 8 files changed, 233 insertions(+), 34 deletions(-) create mode 100644 packages/e2e-tests/plugins/allowed-patterns-disable-blocks.php create mode 100644 packages/e2e-tests/plugins/allowed-patterns.php create mode 100644 packages/e2e-tests/specs/editor/various/allowed-patterns.test.js diff --git a/package-lock.json b/package-lock.json index f4c169e392bef9..ffe15c5322cda4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13205,6 +13205,7 @@ "@babel/runtime": "^7.13.10", "@wordpress/a11y": "file:packages/a11y", "@wordpress/blob": "file:packages/blob", + "@wordpress/block-serialization-default-parser": "file:packages/block-serialization-default-parser", "@wordpress/blocks": "file:packages/blocks", "@wordpress/components": "file:packages/components", "@wordpress/compose": "file:packages/compose", diff --git a/packages/block-editor/package.json b/packages/block-editor/package.json index 5edd01ba2ab309..c8bbea7c3df215 100644 --- a/packages/block-editor/package.json +++ b/packages/block-editor/package.json @@ -34,6 +34,7 @@ "@babel/runtime": "^7.13.10", "@wordpress/a11y": "file:../a11y", "@wordpress/blob": "file:../blob", + "@wordpress/block-serialization-default-parser": "file:../block-serialization-default-parser", "@wordpress/blocks": "file:../blocks", "@wordpress/components": "file:../components", "@wordpress/compose": "file:../compose", diff --git a/packages/block-editor/src/components/block-pattern-setup/use-patterns-setup.js b/packages/block-editor/src/components/block-pattern-setup/use-patterns-setup.js index 1b6a90b87457a0..709e127ded6428 100644 --- a/packages/block-editor/src/components/block-pattern-setup/use-patterns-setup.js +++ b/packages/block-editor/src/components/block-pattern-setup/use-patterns-setup.js @@ -14,17 +14,23 @@ function usePatternsSetup( clientId, blockName, filterPatternsFn ) { const { getBlockRootClientId, __experimentalGetPatternsByBlockTypes, + __experimentalGetParsedPattern, __experimentalGetAllowedPatterns, } = select( blockEditorStore ); const rootClientId = getBlockRootClientId( clientId ); + let patterns = []; if ( filterPatternsFn ) { - return __experimentalGetAllowedPatterns( rootClientId ).filter( - filterPatternsFn + patterns = __experimentalGetAllowedPatterns( + rootClientId + ).filter( filterPatternsFn ); + } else { + patterns = __experimentalGetPatternsByBlockTypes( + blockName, + rootClientId ); } - return __experimentalGetPatternsByBlockTypes( - blockName, - rootClientId + return patterns.map( ( { name } ) => + __experimentalGetParsedPattern( name ) ); }, [ clientId, blockName, filterPatternsFn ] diff --git a/packages/block-editor/src/components/block-switcher/use-transformed-patterns.js b/packages/block-editor/src/components/block-switcher/use-transformed-patterns.js index 7f8c956ea69883..0ce374fbbc8cbc 100644 --- a/packages/block-editor/src/components/block-switcher/use-transformed-patterns.js +++ b/packages/block-editor/src/components/block-switcher/use-transformed-patterns.js @@ -3,11 +3,13 @@ */ import { useMemo } from '@wordpress/element'; import { cloneBlock } from '@wordpress/blocks'; +import { useSelect } from '@wordpress/data'; /** * Internal dependencies */ import { getMatchingBlockByName, getRetainedBlockAttributes } from './utils'; +import { store as blockEditorStore } from '../../store'; /** * Mutate the matched block's attributes by getting @@ -94,9 +96,19 @@ export const getPatternTransformedBlocks = ( */ // TODO tests const useTransformedPatterns = ( patterns, selectedBlocks ) => { + const parsedPatterns = useSelect( + ( select ) => + patterns.map( ( { name } ) => + select( blockEditorStore ).__experimentalGetParsedPattern( + name + ) + ), + [ patterns ] + ); + return useMemo( () => - patterns.reduce( ( accumulator, _pattern ) => { + parsedPatterns.reduce( ( accumulator, _pattern ) => { const transformedBlocks = getPatternTransformedBlocks( selectedBlocks, _pattern.blocks @@ -109,7 +121,7 @@ const useTransformedPatterns = ( patterns, selectedBlocks ) => { } return accumulator; }, [] ), - [ patterns, selectedBlocks ] + [ parsedPatterns, selectedBlocks ] ); }; diff --git a/packages/block-editor/src/store/selectors.js b/packages/block-editor/src/store/selectors.js index aa4f2d3c87b22e..8dabce62e33568 100644 --- a/packages/block-editor/src/store/selectors.js +++ b/packages/block-editor/src/store/selectors.js @@ -30,6 +30,7 @@ import { } from '@wordpress/blocks'; import { SVG, Rect, G, Path } from '@wordpress/components'; import { Platform } from '@wordpress/element'; +import { parse as parseBlocks } from '@wordpress/block-serialization-default-parser'; /** * A block selection object. @@ -1260,6 +1261,22 @@ export function getTemplateLock( state, rootClientId ) { return blockListSettings.templateLock; } +const checkAllowList = ( list, item, defaultResult = null ) => { + if ( isBoolean( list ) ) { + return list; + } + if ( isArray( list ) ) { + // TODO: when there is a canonical way to detect that we are editing a post + // the following check should be changed to something like: + // if ( list.includes( 'core/post-content' ) && getEditorMode() === 'post-content' && item === null ) + if ( list.includes( 'core/post-content' ) && item === null ) { + return true; + } + return list.includes( item ); + } + return defaultResult; +}; + /** * Determines if the given block type is allowed to be inserted into the block list. * This function is not exported and not memoized because using a memoized selector @@ -1278,22 +1295,6 @@ const canInsertBlockTypeUnmemoized = ( blockName, rootClientId = null ) => { - const checkAllowList = ( list, item, defaultResult = null ) => { - if ( isBoolean( list ) ) { - return list; - } - if ( isArray( list ) ) { - // TODO: when there is a canonical way to detect that we are editing a post - // the following check should be changed to something like: - // if ( list.includes( 'core/post-content' ) && getEditorMode() === 'post-content' && item === null ) - if ( list.includes( 'core/post-content' ) && item === null ) { - return true; - } - return list.includes( item ); - } - return defaultResult; - }; - let blockType; if ( blockName && 'object' === typeof blockName ) { blockType = blockName; @@ -1784,6 +1785,32 @@ export const __experimentalGetAllowedBlocks = createSelector( ] ); +const checkAllowListRecursive = ( blocks, allowedBlockTypes ) => { + if ( isBoolean( allowedBlockTypes ) ) { + return allowedBlockTypes; + } + + const blocksQueue = [ ...blocks ]; + while ( blocksQueue.length > 0 ) { + const block = blocksQueue.shift(); + + const isAllowed = checkAllowList( + allowedBlockTypes, + block.name || block.blockName, + true + ); + if ( ! isAllowed ) { + return false; + } + + block.innerBlocks?.forEach( ( innerBlock ) => { + blocksQueue.push( innerBlock ); + } ); + } + + return true; +}; + export const __experimentalGetParsedPattern = createSelector( ( state, patternName ) => { const patterns = state.settings.__experimentalBlockPatterns; @@ -1799,26 +1826,48 @@ export const __experimentalGetParsedPattern = createSelector( ( state ) => [ state.settings.__experimentalBlockPatterns ] ); +const getAllAllowedPatterns = createSelector( + ( state ) => { + const patterns = state.settings.__experimentalBlockPatterns; + const { allowedBlockTypes } = getSettings( state ); + const parsedPatterns = patterns.map( ( pattern ) => ( { + ...pattern, + // We only need the overall block structure of the pattern. So, for + // performance reasons, we can parse the pattern's content using + // the raw blocks parser, also known as the "stage I" block parser. + // This is about 250x faster than the full parse that the Block API + // offers. + blockNodes: parseBlocks( pattern.content ), + } ) ); + const allowedPatterns = parsedPatterns.filter( ( { blockNodes } ) => + checkAllowListRecursive( blockNodes, allowedBlockTypes ) + ); + return allowedPatterns; + }, + ( state ) => [ + state.settings.__experimentalBlockPatterns, + state.settings.allowedBlockTypes, + ] +); + /** - * Returns the list of allowed patterns for inner blocks children + * Returns the list of allowed patterns for inner blocks children. * * @param {Object} state Editor state. * @param {?string} rootClientId Optional target root client ID. * - * @return {Array?} The list of allowed block types. + * @return {Array?} The list of allowed patterns. */ export const __experimentalGetAllowedPatterns = createSelector( ( state, rootClientId = null ) => { - const patterns = state.settings.__experimentalBlockPatterns; - const parsedPatterns = patterns.map( ( { name } ) => - __experimentalGetParsedPattern( state, name ) - ); - const patternsAllowed = filter( parsedPatterns, ( { blocks } ) => - blocks.every( ( { name } ) => - canInsertBlockType( state, name, rootClientId ) - ) + const availableParsedPatterns = getAllAllowedPatterns( state ); + const patternsAllowed = filter( + availableParsedPatterns, + ( { blockNodes } ) => + blockNodes.every( ( { blockName } ) => + canInsertBlockType( state, blockName, rootClientId ) + ) ); - return patternsAllowed; }, ( state, rootClientId ) => [ diff --git a/packages/e2e-tests/plugins/allowed-patterns-disable-blocks.php b/packages/e2e-tests/plugins/allowed-patterns-disable-blocks.php new file mode 100644 index 00000000000000..16e0d83ceff407 --- /dev/null +++ b/packages/e2e-tests/plugins/allowed-patterns-disable-blocks.php @@ -0,0 +1,24 @@ +post_type ) { + return $allowed_block_types; + } + return array( 'core/heading', 'core/columns', 'core/column', 'core/image', 'core/spacer' ); +} + +add_filter( 'allowed_block_types', 'my_plugin_allowed_block_types', 10, 2 ); diff --git a/packages/e2e-tests/plugins/allowed-patterns.php b/packages/e2e-tests/plugins/allowed-patterns.php new file mode 100644 index 00000000000000..b598cb6c0b82e7 --- /dev/null +++ b/packages/e2e-tests/plugins/allowed-patterns.php @@ -0,0 +1,32 @@ + 'Test: Single heading', + 'content' => '

Hello!

', + ) +); + +register_block_pattern( + 'test-allowed-patterns/lone-paragraph', + array( + 'title' => 'Test: Single paragraph', + 'content' => '

Hello!

', + ) +); + +register_block_pattern( + 'test-allowed-patterns/paragraph-inside-group', + array( + 'title' => 'Test: Paragraph inside group', + 'content' => '

Hello!

', + ) +); diff --git a/packages/e2e-tests/specs/editor/various/allowed-patterns.test.js b/packages/e2e-tests/specs/editor/various/allowed-patterns.test.js new file mode 100644 index 00000000000000..449306f06d18d2 --- /dev/null +++ b/packages/e2e-tests/specs/editor/various/allowed-patterns.test.js @@ -0,0 +1,74 @@ +/** + * WordPress dependencies + */ +import { + activatePlugin, + createNewPost, + deactivatePlugin, + searchForPattern, + toggleGlobalBlockInserter, +} from '@wordpress/e2e-test-utils'; + +const checkPatternExistence = async ( name, available = true ) => { + await searchForPattern( name ); + const patternElement = await page.waitForXPath( + `//div[@role = 'option']//div[contains(text(), '${ name }')]`, + { timeout: 5000, visible: available, hidden: ! available } + ); + const patternExists = !! patternElement; + await toggleGlobalBlockInserter(); + return patternExists; +}; + +const TEST_PATTERNS = [ + [ 'Test: Single heading', true ], + [ 'Test: Single paragraph', false ], + [ 'Test: Paragraph inside group', false ], +]; + +describe( 'Allowed Patterns', () => { + beforeAll( async () => { + await activatePlugin( 'gutenberg-test-allowed-patterns' ); + await createNewPost(); + } ); + afterAll( async () => { + await deactivatePlugin( 'gutenberg-test-allowed-patterns' ); + } ); + + describe( 'Disable blocks plugin disabled', () => { + for ( const [ patternName ] of TEST_PATTERNS ) { + it( `should show test pattern "${ patternName }"`, async () => { + expect( await checkPatternExistence( patternName, true ) ).toBe( + true + ); + } ); + } + } ); + + describe( 'Disable blocks plugin enabled', () => { + beforeAll( async () => { + await activatePlugin( + 'gutenberg-test-allowed-patterns-disable-blocks' + ); + await createNewPost(); + } ); + afterAll( async () => { + await deactivatePlugin( + 'gutenberg-test-allowed-patterns-disable-blocks' + ); + } ); + + for ( const [ patternName, shouldBeAvailable ] of TEST_PATTERNS ) { + it( `should${ + shouldBeAvailable ? '' : ' not' + } show test "pattern ${ patternName }"`, async () => { + expect( + await checkPatternExistence( + patternName, + shouldBeAvailable + ) + ).toBe( shouldBeAvailable ); + } ); + } + } ); +} );