From 7701453dcd6bcc4e04477e071b2f2884b03d19ce Mon Sep 17 00:00:00 2001 From: Tetsuaki Hamano Date: Sat, 25 Feb 2023 12:18:23 +0900 Subject: [PATCH] Fix: duplicate markup on anchor attribute --- lib/block-supports/anchor.php | 25 +-------- packages/block-editor/src/hooks/anchor.js | 37 ++++++++++++-- packages/block-editor/src/hooks/test/utils.js | 51 +++++++++++++++---- .../src/server-side-render.js | 12 +++-- 4 files changed, 83 insertions(+), 42 deletions(-) diff --git a/lib/block-supports/anchor.php b/lib/block-supports/anchor.php index 8f704a5019b365..eb664687cd1e72 100644 --- a/lib/block-supports/anchor.php +++ b/lib/block-supports/anchor.php @@ -5,28 +5,6 @@ * @package gutenberg */ -/** - * Registers the anchor block attribute for block types that support it. - * - * @param WP_Block_Type $block_type Block Type. - */ -function gutenberg_register_anchor_support( $block_type ) { - $has_anchor_support = _wp_array_get( $block_type->supports, array( 'anchor' ), true ); - if ( ! $has_anchor_support ) { - return; - } - - if ( ! $block_type->attributes ) { - $block_type->attributes = array(); - } - - if ( ! array_key_exists( 'anchor', $block_type->attributes ) ) { - $block_type->attributes['anchor'] = array( - 'type' => 'string', - ); - } -} - /** * Add the anchor to the output. * @@ -61,7 +39,6 @@ function gutenberg_apply_anchor_support( $block_type, $block_attributes ) { WP_Block_Supports::get_instance()->register( 'anchor', array( - 'register_attribute' => 'gutenberg_register_anchor_support', - 'apply' => 'gutenberg_apply_anchor_support', + 'apply' => 'gutenberg_apply_anchor_support', ) ); diff --git a/packages/block-editor/src/hooks/anchor.js b/packages/block-editor/src/hooks/anchor.js index 65e227ab107ebf..0618dac9efc170 100644 --- a/packages/block-editor/src/hooks/anchor.js +++ b/packages/block-editor/src/hooks/anchor.js @@ -40,12 +40,39 @@ export function addAttribute( settings ) { if ( 'type' in ( settings.attributes?.anchor ?? {} ) ) { return settings; } + + // Gracefully handle if settings.attributes is undefined. if ( hasBlockSupport( settings, 'anchor' ) ) { - // Gracefully handle if settings.attributes is undefined. - settings.attributes = { - ...settings.attributes, - anchor: ANCHOR_SCHEMA, - }; + let saveElement; + try { + saveElement = + typeof settings.save === 'function' + ? settings.save() + : settings.save; + if ( saveElement === null || saveElement === undefined ) { + // If the save function returns null or undefined, save the anchor + // attribute as a block's comment delimiter without specifying + // the source. + settings.attributes = { + ...settings.attributes, + anchor: { + type: ANCHOR_SCHEMA.type, + }, + }; + } else { + settings.attributes = { + ...settings.attributes, + anchor: ANCHOR_SCHEMA, + }; + } + } catch ( error ) { + // If the save function returns an error, the anchor will be saved + // in the markup as an id attribute. + settings.attributes = { + ...settings.attributes, + anchor: ANCHOR_SCHEMA, + }; + } } return settings; diff --git a/packages/block-editor/src/hooks/test/utils.js b/packages/block-editor/src/hooks/test/utils.js index bfbced3195b7fb..9c685f42fd9521 100644 --- a/packages/block-editor/src/hooks/test/utils.js +++ b/packages/block-editor/src/hooks/test/utils.js @@ -9,8 +9,6 @@ import { applyFilters } from '@wordpress/hooks'; import '../anchor'; import { immutableSet } from '../utils'; -const noop = () => {}; - describe( 'immutableSet', () => { describe( 'handling falsy values properly', () => { it( 'should create a new object if `undefined` is passed', () => { @@ -116,7 +114,6 @@ describe( 'immutableSet', () => { describe( 'anchor', () => { const blockSettings = { - save: noop, category: 'text', title: 'block title', }; @@ -133,15 +130,49 @@ describe( 'anchor', () => { expect( settings.attributes ).toBe( undefined ); } ); - it( 'should assign a new anchor attribute', () => { - const settings = registerBlockType( { - ...blockSettings, - supports: { - anchor: true, + it( 'should assign a new anchor attribute depending on the value of the save property', () => { + const saveDefinitions = [ + // The anchor attribute should be stored as a comment delimiter. + { + value: null, + schema: { + type: 'string', + }, }, - } ); + { + value: () => null, + schema: { + type: 'string', + }, + }, + { + value: undefined, + schema: { + type: 'string', + }, + }, + // The anchor attributes should be saved as part of the markup. + { + value: () =>
, + schema: { + attribute: 'id', + selector: '*', + source: 'attribute', + type: 'string', + }, + }, + ]; - expect( settings.attributes ).toHaveProperty( 'anchor' ); + saveDefinitions.forEach( ( { value, schema } ) => { + const settings = registerBlockType( { + ...blockSettings, + save: value, + supports: { + anchor: true, + }, + } ); + expect( settings.attributes.anchor ).toEqual( schema ); + } ); } ); it( 'should not override attributes defined in settings', () => { diff --git a/packages/server-side-render/src/server-side-render.js b/packages/server-side-render/src/server-side-render.js index 69a2183036dd14..aee2921ecfa89b 100644 --- a/packages/server-side-render/src/server-side-render.js +++ b/packages/server-side-render/src/server-side-render.js @@ -112,9 +112,15 @@ export default function ServerSideRender( props ) { setIsLoading( true ); - let sanitizedAttributes = - attributes && - __experimentalSanitizeBlockAttributes( block, attributes ); + let sanitizedAttributes; + if ( attributes ) { + // Anchor attribute isn't supported on the server side and + // should be excluded from the parameters. + const { anchor, ...restAttributes } = attributes; + sanitizedAttributes = + restAttributes && + __experimentalSanitizeBlockAttributes( block, restAttributes ); + } if ( skipBlockSupportAttributes ) { sanitizedAttributes =