From 905aaa31b280db8f4b6043008bcbeba4786fe76c Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Wed, 5 May 2021 14:10:06 +1000 Subject: [PATCH 1/2] Legacy Widget: Ensure the control form's IDs have a consistent number --- lib/class-wp-rest-widget-types-controller.php | 10 ++-- .../src/legacy-widget/edit/control.js | 48 ++++++++++--------- 2 files changed, 33 insertions(+), 25 deletions(-) diff --git a/lib/class-wp-rest-widget-types-controller.php b/lib/class-wp-rest-widget-types-controller.php index 75b00e455cd59..bd6c0a42ee658 100644 --- a/lib/class-wp-rest-widget-types-controller.php +++ b/lib/class-wp-rest-widget-types-controller.php @@ -419,9 +419,13 @@ public function encode_form_data( $request ) { ); } - // Set the widget's number to 1 so that the `id` attributes in the HTML - // that we return are predictable. - $widget_object->_set( 1 ); + // Set the widget's number so that the id attributes in the HTML that we + // return are predictable. + if ( isset( $request['number'] ) && is_numeric( $request['number'] ) ) { + $widget_object->_set( (int) $request['number'] ); + } else { + $widget_object->_set( -1 ); + } if ( isset( $request['instance']['encoded'], $request['instance']['hash'] ) ) { $serialized_instance = base64_decode( $request['instance']['encoded'] ); diff --git a/packages/block-library/src/legacy-widget/edit/control.js b/packages/block-library/src/legacy-widget/edit/control.js index 1f49126f585ed..a6867771f7b29 100644 --- a/packages/block-library/src/legacy-widget/edit/control.js +++ b/packages/block-library/src/legacy-widget/edit/control.js @@ -47,6 +47,11 @@ export default class Control { this.onChangeHasPreview = onChangeHasPreview; this.onError = onError; + // We can't use the real widget number as this is calculated by the + // server and we may not ever *actually* save this widget. Instead, use + // a fake but unique number. + this.number = ++lastNumber; + this.handleFormChange = debounce( this.saveForm.bind( this ), 200 ); this.handleFormSubmit = this.handleFormSubmit.bind( this ); @@ -73,11 +78,6 @@ export default class Control { * @access private */ initDOM() { - // We can't use the real widget number as this is calculated by the - // server and we may not ever *actually* save this widget. Instead, use - // a fake but unique number. - const number = ++lastNumber; - this.element = el( 'div', { class: 'widget open' }, [ el( 'div', { class: 'widget-inside' }, [ ( this.form = el( 'form', { class: 'form', method: 'post' }, [ @@ -87,7 +87,7 @@ export default class Control { class: 'widget-id', type: 'hidden', name: 'widget-id', - value: this.id ?? `${ this.idBase }-${ number }`, + value: this.id ?? `${ this.idBase }-${ this.number }`, } ), el( 'input', { class: 'id_base', @@ -111,7 +111,7 @@ export default class Control { class: 'widget_number', type: 'hidden', name: 'widget_number', - value: this.idBase ? number.toString() : '', + value: this.idBase ? this.number.toString() : '', } ), ( this.content = el( 'div', { class: 'widget-content' } ) ), // Non-multi widgets can be saved via a Save button. @@ -179,21 +179,23 @@ export default class Control { const { form } = await saveWidget( this.id ); this.content.innerHTML = form; } else if ( this.idBase ) { - const { form, preview } = await encodeWidget( - this.idBase, - this.instance - ); + const { form, preview } = await encodeWidget( { + idBase: this.idBase, + instance: this.instance, + number: this.number, + } ); this.content.innerHTML = form; this.hasPreview = ! isEmptyHTML( preview ); // If we don't have an instance, perform a save right away. This // happens when creating a new Legacy Widget block. if ( ! this.instance.hash ) { - const { instance } = await encodeWidget( - this.idBase, - this.instance, - serializeForm( this.form ) - ); + const { instance } = await encodeWidget( { + idBase: this.idBase, + instance: this.instance, + number: this.number, + formData: serializeForm( this.form ), + } ); this.instance = instance; } } @@ -244,11 +246,12 @@ export default class Control { ] ); } } else if ( this.idBase ) { - const { instance, preview } = await encodeWidget( - this.idBase, - this.instance, - formData - ); + const { instance, preview } = await encodeWidget( { + idBase: this.idBase, + instance: this.instance, + number: this.number, + formData, + } ); this.instance = instance; this.hasPreview = ! isEmptyHTML( preview ); } @@ -338,12 +341,13 @@ async function saveWidget( id, formData = null ) { return { form: widget.rendered_form }; } -async function encodeWidget( idBase, instance, formData = null ) { +async function encodeWidget( { idBase, instance, number, formData = null } ) { const response = await apiFetch( { path: `/wp/v2/widget-types/${ idBase }/encode`, method: 'POST', data: { instance, + number, form_data: formData, }, } ); From 5bdb6dd8495c2dbfd68a91d94d4bf06e35545f8e Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Wed, 5 May 2021 14:27:37 +1000 Subject: [PATCH 2/2] Fix/add tests --- ...lass-rest-widget-types-controller-test.php | 51 +++++++++++++++---- 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/phpunit/class-rest-widget-types-controller-test.php b/phpunit/class-rest-widget-types-controller-test.php index 00529dd13d23e..5f65dfd8c14f4 100644 --- a/phpunit/class-rest-widget-types-controller-test.php +++ b/phpunit/class-rest-widget-types-controller-test.php @@ -260,8 +260,41 @@ public function test_encode_form_data_with_no_input() { $data = $response->get_data(); $this->assertEquals( "

\n" . - "\t\t\t\n" . - "\t\t\t\n" . + "\t\t\t\n" . + "\t\t\t\n" . + "\t\t

", + $data['form'] + ); + $this->assertStringMatchesFormat( + "
\n" . + "\t\t\t\t
\n" . + "\t\t\t\t\t\n" . + "\t\t\t\t\t\n" . + "\t\t\t\t\t\n" . + "\t\t\t\t
\n" . + "\t\t\t
", + $data['preview'] + ); + $this->assertEqualSets( + array( + 'encoded' => base64_encode( serialize( array() ) ), + 'hash' => wp_hash( serialize( array() ) ), + 'raw' => new stdClass, + ), + $data['instance'] + ); + } + + public function test_encode_form_data_with_number() { + wp_set_current_user( self::$admin_id ); + $request = new WP_REST_Request( 'POST', '/wp/v2/widget-types/search/encode' ); + $request->set_param( 'number', 8 ); + $response = rest_get_server()->dispatch( $request ); + $data = $response->get_data(); + $this->assertEquals( + "

\n" . + "\t\t\t\n" . + "\t\t\t\n" . "\t\t

", $data['form'] ); @@ -299,8 +332,8 @@ public function test_encode_form_data_with_instance() { $data = $response->get_data(); $this->assertEquals( "

\n" . - "\t\t\t\n" . - "\t\t\t\n" . + "\t\t\t\n" . + "\t\t\t\n" . "\t\t

", $data['form'] ); @@ -327,13 +360,13 @@ public function test_encode_form_data_with_instance() { public function test_encode_form_data_with_form_data() { wp_set_current_user( self::$admin_id ); $request = new WP_REST_Request( 'POST', '/wp/v2/widget-types/search/encode' ); - $request->set_param( 'form_data', 'widget-search[1][title]=Updated+title' ); + $request->set_param( 'form_data', 'widget-search[-1][title]=Updated+title' ); $response = rest_get_server()->dispatch( $request ); $data = $response->get_data(); $this->assertEquals( "

\n" . - "\t\t\t\n" . - "\t\t\t\n" . + "\t\t\t\n" . + "\t\t\t\n" . "\t\t

", $data['form'] ); @@ -373,8 +406,8 @@ public function test_encode_form_data_no_raw() { $data = $response->get_data(); $this->assertEquals( "

\n" . - "\t\t\t\n" . - "\t\t\t\n" . + "\t\t\t\n" . + "\t\t\t\n" . "\t\t

", $data['form'] );