`$sidebar_widgets` out of sync during batch updates to widgets through the REST API

[zieladam]

Continuation of #53657.

From @adamziel on ​https://github.com/WordPress/wordpress-develop/pull/1498:

retrieve_widgets() runs some logic to move "hidden/lost" widgets to wp_inactive_widgets sidebar. It does so based on the global $sidebars_widgets. Normally this is not a problem, but when processing batch requests, $sidebars_widgets isn't properly updated by a previous call to WP_REST_Widgets_Controller::create_item() – only global $_wp_sidebars_widgets has been changed. So, as far as retrieve_widgets() is concerned, the last created widget isn't assigned to any sidebar and so it is mistakenly moved to the wp_inactive_widgets sidebar.

[51432] provided a temporary, "RC friendly" fix to the issue for 5.8. But a better, more permanent solution should be explored for a future release.

See also:

• ​https://github.com/WordPress/gutenberg/issues/33335

[desrosj]

Changing reporter to reflect who originally reported the issue.

[zieladam]

This very relevant conversation happened on .org slack recently:

@zieladam
Does anyone know why we have both $_wp_sidebars_widgets AND $sidebars_widgets global variables?

@hellofromtonya
Great question @zieladam!
Core uses $_wp_sidebars_widgets private global as a cache of the sidebars_widgets option to initialize $sidebars_widgets global when not in the admin. (Note: The cached version can be modified in memory to switch from single to multi widget.)
$sidebars_widgets is the in memory working state of the widgets.

@zieladam
thank you @hellofromtonya! do you think we could refactor that to have just one? I just finished debugging ​https://github.com/WordPress/gutenberg/issues/33335 and a big part of it was that, after updating things, we refreshed only $_wp_sidebars_widgets but not $sidebars_widgets. I will patch it up and it will just work, but I think we’re in a non-future-proof situation and may introduce similar errors again (edited)

@hellofromtonya
Switching over now to look more deeply at the globals and managing their state.

@hellofromtonya
Hey @zieladam , the stack of handlers that have been added for inactive widgets and global states concerns me. 2 weird issues drove the need to add these. While it fixes both of them, I agree with you that fundamentally something is off. I agree that we should explore the 2 globals.
Caution: the globals and the code around them have existed a long time in Core. For those folks who choose to use the Classic Widgets, changes made in these functions and globals will need care to ensure backwards compatibility.

[desrosj]

With 5.8.1 RC due out in about 24 hours, I'm going to punt this one. I'm choosing to punt to 5.9, but if a fix is ready, it can be considered for a 5.8.x minor.

[zieladam]

Duplicate of #53816.

The fix is attached to and discussed in https://core.trac.wordpress.org/ticket/53816 so let's close this one as a duplicate – I got caught up in proliferation of tickets and PRs :-)