Fix adding and deleting legacy widgets

packages/customize-widgets/src/components/sidebar-block-editor/sidebar-adapter.js

@@ -173,6 +173,8 @@ export default class SidebarAdapter {
 			number = widgetModel.get( 'multi_number' );
 		}
 
+		widget.instance.is_widget_customizer_js_value = true;
+
 		const settingId = number
 			? `widget_${ widget.idBase }[${ number }]`
 			: `widget_${ widget.idBase }`;
@@ -200,6 +202,13 @@ export default class SidebarAdapter {
 
 	_removeWidget( widget ) {
 		const settingId = widgetIdToSettingId( widget.id );
+		const setting = this.api( settingId );
+
+		if ( setting ) {
+			const instance = setting.get();
+			this.widgetsCache.delete( instance );
+		}
+
 		this.api.remove( settingId );
 	}
 
@@ -277,7 +286,10 @@ export default class SidebarAdapter {
 			return widgetId;
 		} );
 
-		// TODO: We should in theory also handle delete widgets here too.
+		const deletedWidgets = this.getWidgets().filter(
+			( widget ) => ! nextWidgetIds.includes( widget.id )
+		);
+		deletedWidgets.forEach( ( widget ) => this._removeWidget( widget ) );
 
 		this.setting.set( nextWidgetIds );
 

packages/e2e-tests/plugins/class-test-widget.php

@@ -0,0 +1,79 @@
+<?php
+/**
+ * Plugin Name: Gutenberg Test Widgets
+ * Plugin URI: https://github.com/WordPress/gutenberg
+ * Author: Gutenberg Team
+ *
+ * @package gutenberg-test-widgets
+ */
+
+/**
+ * Test widget to be used in e2e tests.
+ */
+class Test_Widget extends WP_Widget {
+	/**
+	 * Sets up a new test widget instance.
+	 */
+	function __construct() {
+		parent::__construct(
+			'test_widget',
+			'Test Widget',
+			array( 'description' => 'Test widget.' )
+		);
+	}
+
+	/**
+	 * Outputs the content for the widget instance.
+	 *
+	 * @param array $args Display arguments including 'before_title', 'after_title',
+	 *                        'before_widget', and 'after_widget'.
+	 * @param array $instance Settings for the current Block widget instance.
+	 */
+	public function widget( $args, $instance ) {
+		$title = apply_filters( 'widget_title', $instance['title'] );
+		echo $args['before_widget'];
+		if ( ! empty( $title ) ) {
+			echo $args['before_title'] . $title . $args['after_title'];
+		}
+		echo 'Hello Test Widget';
+		echo $args['after_widget'];
+	}
+
+	/**
+	 * Outputs the widget settings form.
+	 *
+	 * @param array $instance Current instance.
+	 */
+	public function form( $instance ) {
+		?>
+		<p>
+			<label for="<?php echo $this->get_field_id( 'title' ); ?>">Title:</label>
+			<input class="widefat" id="<?php echo $this->get_field_id( 'title' ); ?>" name="<?php echo $this->get_field_name( 'title' ); ?>" type="text" value="<?php echo esc_attr( $instance['title'] ); ?>" />
+		</p>
+		<?php
+	}
+
+	/**
+	 * Handles updating settings for the current widget instance.
+	 *
+	 * @param array $new_instance New settings for this instance as input by the user via
+	 *                            WP_Widget::form().
+	 *
+	 * @return array Settings to save or bool false to cancel saving.
+	 * @since 4.8.1
+	 */
+	public function update( $new_instance ) {
+		$instance          = array();
+		$instance['title'] = ( ! empty( $new_instance['title'] ) ) ? strip_tags( $new_instance['title'] ) : '';
+		return $instance;
+	}
+}
+
+/**
+ * Register the widget.
+ */
+function load_test_widget() {
+	register_widget( 'Test_Widget' );
+}
+
+add_action( 'widgets_init', 'load_test_widget' );

packages/e2e-tests/specs/experiments/customizing-widgets.test.js

@@ -8,6 +8,7 @@ import {
 	visitAdminPage,
 	showBlockToolbar,
 	clickBlockToolbarButton,
+	createURL,
 } from '@wordpress/e2e-test-utils';
 
 /**
@@ -42,12 +43,14 @@ describe( 'Widgets Customizer', () => {
 		await deactivatePlugin(
 			'gutenberg-test-plugin-disables-the-css-animations'
 		);
+		await activatePlugin( 'gutenberg-test-widgets' );
 	} );
 
 	afterAll( async () => {
 		await activatePlugin(
 			'gutenberg-test-plugin-disables-the-css-animations'
 		);
+		await deactivatePlugin( 'gutenberg-test-widgets' );
 		await activateTheme( 'twentytwentyone' );
 	} );
 
@@ -510,6 +513,141 @@ describe( 'Widgets Customizer', () => {
 			"The page delivered both an 'X-Frame-Options' header and a 'Content-Security-Policy' header with a 'frame-ancestors' directive. Although the 'X-Frame-Options' header alone would have blocked embedding, it has been ignored."
 		);
 	} );
+
+	it( 'should handle legacy widgets', async () => {
+		const widgetsPanel = await find( {
+			role: 'heading',
+			name: /Widgets/,
+			level: 3,
+		} );
+		await widgetsPanel.click();
+
+		const footer1Section = await find( {
+			role: 'heading',
+			name: /^Footer #1/,
+			level: 3,
+		} );
+		await footer1Section.click();
+
+		const legacyWidgetBlock = await addBlock( 'Legacy Widget' );
+		const selectLegacyWidgets = await find( {
+			role: 'combobox',
+			name: 'Select a legacy widget to display:',
+		} );
+		await selectLegacyWidgets.select( 'test_widget' );
+
+		await expect( {
+			role: 'heading',
+			name: 'Test Widget',
+			level: 3,
+		} ).toBeFound( { root: legacyWidgetBlock } );
+
+		let titleInput = await find(
+			{
+				role: 'textbox',
+				name: 'Title:',
+			},
+			{
+				root: legacyWidgetBlock,
+			}
+		);
+
+		await titleInput.type( 'Hello Title' );
+
+		// Unfocus the current legacy widget.
+		await page.keyboard.press( 'Tab' );
+
+		// Disable reason: Sometimes the preview just doesn't fully load,
+		// it's the only way I know for now to ensure that the iframe is ready.
+		// eslint-disable-next-line no-restricted-syntax
+		await page.waitForTimeout( 1000 );
+		await waitForPreviewIframe();
+
+		// Expect the legacy widget to show in the site preview frame.
+		await expect( {
+			role: 'heading',
+			name: 'Hello Title',
+		} ).toBeFound( {
+			root: await find( {
+				name: 'Site Preview',
+				selector: 'iframe',
+			} ),
+		} );
+
+		// Expect the preview in block to show when unfocusing the legacy widget block.
+		await expect( {
+			role: 'heading',
+			name: 'Hello Title',
+		} ).toBeFound( {
+			root: await find( {
+				selector: 'iframe',
+				name: 'Legacy Widget Preview',
+			} ),
+		} );
+
+		await legacyWidgetBlock.focus();
+		await showBlockToolbar();
+
+		// Testing removing the block.
+		await clickBlockToolbarButton( 'Options' );
+		const removeBlockButton = await find( {
+			role: 'menuitem',
+			name: /Remove block/,
+		} );
+		await removeBlockButton.click();
+
+		// Add it back again using the variant.
+		const testWidgetBlock = await addBlock( 'Test Widget' );
+
+		titleInput = await find(
+			{
+				role: 'textbox',
+				name: 'Title:',
+			},
+			{
+				root: testWidgetBlock,
+			}
+		);
+
+		await titleInput.type( 'Hello again!' );
+		// Unfocus the current legacy widget.
+		await page.keyboard.press( 'Tab' );
+
+		// Expect the preview in block to show when unfocusing the legacy widget block.
+		await expect( {
+			role: 'heading',
+			name: 'Hello again!',
+		} ).toBeFound( {
+			root: await find( {
+				selector: 'iframe',
+				name: 'Legacy Widget Preview',
+			} ),
+		} );
+
+		const publishButton = await find( {
+			role: 'button',
+			name: 'Publish',
+		} );
+		await publishButton.click();
+
+		// Wait for publishing to finish.
+		await expect( publishButton ).toMatchQuery( {
+			disabled: true,
+		} );
+		await page.waitForResponse( createURL( '/wp-admin/admin-ajax.php' ) );
+
+		expect( console ).toHaveWarned(
+			"The page delivered both an 'X-Frame-Options' header and a 'Content-Security-Policy' header with a 'frame-ancestors' directive. Although the 'X-Frame-Options' header alone would have blocked embedding, it has been ignored."
+		);
+
+		await page.goto( createURL( '/' ) );
+
+		// Expect the saved widgets to show on frontend.
+		await expect( {
+			role: 'heading',
+			name: 'Hello again!',
+		} ).toBeFound();
+	} );
 } );
 
 /**
@@ -562,6 +700,20 @@ async function addBlock( blockName ) {
 	);
 	await addBlockButton.click();
 
+	const searchBox = await find( {
+		role: 'searchbox',
+		name: 'Search for blocks and patterns',
+	} );
+
+	// Clear the input.
+	await searchBox.evaluate( ( node ) => {
+		if ( node.value ) {
+			node.value = '';
+		}
+	} );
+
+	await searchBox.type( blockName );
+
 	// TODO - remove this timeout when the test plugin for disabling CSS
 	// animations in tests works properly.
 	//