[wp-trac] [WordPress Trac] #39693: Fix missing assignment of widgets on theme switch

[WordPress Trac]

#39693: Fix missing assignment of widgets on theme switch
--------------------------------------+--------------------------
 Reporter:  melchoyce                 |       Owner:  westonruter
     Type:  enhancement               |      Status:  reviewing
 Priority:  normal                    |   Milestone:  4.9
Component:  Widgets                   |     Version:
 Severity:  normal                    |  Resolution:
 Keywords:  has-patch has-unit-tests  |     Focuses:
--------------------------------------+--------------------------
Changes (by westonruter):

 * keywords:  has-patch has-unit-tests commit => has-patch has-unit-tests


Comment:

 In [attachment:39693.6.diff]:

 * Figured out why the mapping logic didn't appear to be working with WP-
 CLI. It actually just appeared to not work for the initial time you visit
 the frontend, but then after reloading the widgets would be in the sidebar
 as expected. The issue is just that the global variable containing the
 sidebars widgets wasn't getting reset after the sidebars widgets were
 updated. See [https://github.com/xwp/wordpress-
 develop/pull/251/commits/5312b269f7cbfd70dc7659d034860f1e3d4be586
 5312b26].
 * I was testing with a theme that had sidebars `sidebar-1` and `sidebar-2`
 and then another theme that had sidebars `primary` and `footer`. I was
 expecting when switching that they would get mapped correspondingly, but
 they were not. So I included `sidebar-1` and `sidebar-2` in the primary
 and footer groups respectively. See [https://github.com/xwp/wordpress-
 develop/pull/251/commits/2abe308cc4d1b42ffbb4520cb0d2a768b08e4ce0
 2abe308].
 * Renamed `_wp_map_sidebars()` to `wp_map_sidebars_widgets()` for better
 parity with `wp_map_nav_menu_locations()`.

 Some outstanding things that I see that need to be addressed:

 * The lost widgets logic appears to be blowing a way old single widgets
 that lack widget numbers. I think [https://github.com/xwp/wordpress-
 develop/pull/251/files/https://github.com/xwp/wordpress-
 develop/blob/206f97d434748ea4b113cf91446f1efc70d44b83/src/wp-
 includes/widgets.php#L1152-L1158 that logic] could probably be removed?
 * I was working on [https://github.com/xwp/wp-theme-nav-menu-widget-
 sidebar-permutations/blob/master/tests/widget-sidebars/scenarios.json
 scenarios] for testing the various sidebar [https://github.com/xwp/wp-
 theme-nav-menu-widget-sidebar-permutations theme permutations], and I
 found a [https://github.com/xwp/wp-theme-nav-menu-widget-sidebar-
 permutations/blob/c1f2074d060167f43e8eaa880a8e32859a00764a/tests/widget-
 sidebars/scenarios.json#L36-L45 scenario] where from a theme with 3
 populated sidebars to a theme with 2 sidebars and then switching back to
 the theme with 3 sidebars results in the one sidebar that was not
 initially mapped being ''empty'' after having switched back. This seems
 bad to me. I think this is part of the orphaned sidebar question.

 PR where I'm reviewing the patch and amending the patch and running Travis
 tests: https://github.com/xwp/wordpress-develop/pull/251

--
Ticket URL: <https://core.trac.wordpress.org/ticket/39693#comment:88>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform