WordPress.org

Make WordPress Core

Opened 3 weeks ago

Last modified 2 weeks ago

#53660 new defect (bug)

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

Reported by: zieladam Owned by:
Milestone: 5.8.1 Priority: normal
Severity: normal Version: 5.8
Component: Widgets Keywords: needs-patch
Focuses: rest-api Cc:

Description

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:

Change History (2)

#1 @desrosj
3 weeks ago

  • Reporter changed from desrosj to zieladam

Changing reporter to reflect who originally reported the issue.

#2 @zieladam
2 weeks ago

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.

Note: See TracTickets for help on using tickets.