[WP 5.8 beta 2] $_POST['sidebar'] is missing when saving a widget

[strategio]

It's a backward compatibility requirement because this data is available in the $_POST superglobal when running WP 5.7.2 and it can be used inside \WP_Widget::update.

Here's a simple example:

<?php
class My_Widget extends WP_Widget {

    public function update( $new_instance, $old_instance ) {
        $new_instance['sidebar'] = $_POST['sidebar'];

        return $new_instance;        
    }
}

I can see in \WP_REST_Widgets_Controller::save_widget that the $_POST superglobal is temporarily altered, so I assume we should be able to also include the 'sidebar'.

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

[spacedmonkey]

Why is this data needed? If it needed, then there is other data is missing. See this example.

This is example data.

widget-custom_html[2][title]: test
widget-custom_html[2][content]: Test!!
widget-id: custom_html-2
id_base: custom_html
widget-width: 400
widget-height: 350
widget_number: 2
multi_number:
add_new:
action: save-widget
savewidgets: 2d13e282bc
sidebar: sidebar-1

[strategio]

Replying to spacedmonkey:

Why is this data needed? If it needed, then there is other data is missing. See this example.

In our case, we have widgets that needs to know the assigned sidebar in order to set the initial settings. But there's no form element for our widget, the settings UI is shown in another place.

So to be honest, I didn't pay attention to the other form data, but I believe it should be passed to the superglobal to keep WP 5.8 backward compatible with legacy widgets.

For the 'sidebar', I believe it's legit to expose this information somewhere because \WP_Widget::update is called from \WP_REST_Widgets_Controller::save_widget which has this information as the second argument.

[noisysocks]

I'm OK with adding it for backwards compatibility. Could you please update the REST API controller's unit tests?

[strategio]

Replying to noisysocks:

I'm OK with adding it for backwards compatibility. Could you please update the REST API controller's unit tests?

Sure, I'll try to do this tomorrow (Friday).

[desrosj]

This is a pretty self contained change. I'm going to commit this without a unit test for the time being so that it can be included for testing in beta 4 later today.

@strategio We can add the test update when you have a chance to create the patch for it, but we should make sure to add it prior to RC1.

[desrosj]

In 51239:

REST API: Include the sidebar ID when saving a widget.

This ensures backwards compatibility for code expecting this field to be present.

Props strategio, noisysocks, spacedmonkey.
See #53452.

Merged into Core in https://core.trac.wordpress.org/changeset/51239.

https://core.trac.wordpress.org/ticket/53452

[strategio]

Hi @desrosj,

I added a test in ​https://github.com/WordPress/wordpress-develop/pull/1448. Please review it!

[desrosj]

Thanks @strategio! The test looks good to me, though I'm not sure about where the test class actually belongs.

It looks like we have a file in DIR_TESTDATA with some widgets, but there are only 2 and they are all tied to a specific data provider. I've also been trying to think of a way to refactor out the need for the class at all, but nothing comes to mind right now.

@SergeyBiryukov would you happen to have any suggestions here?

[JeffPaul]

Closing in favor of picking up the unit test for this in #53673. @strategio please feel free to drop your patch/PR on the subsequent ticket, though I did reference your comment in the description there.