Customizer: Widget settings are lost when activating a previewed theme

[ocean90]

Steps to reproduce:

• Go to wp-admin/themes.php and click "Live Preview" for a theme
• Change the header image
• Now add a widget to a sidebar
• Click "Save & Activate"
• Go to front end and you will notice that the header is changed, but the widgets are lost

[westonruter]

Likely due to Widget Customizer not incorporating logic from #17979 (Avoid losing widgets when switching themes)

[nacin]

Customizing widgets before activating a theme sounds like one of two primary use cases of widgets in the customizer. What am I missing here? Do customizing widgets just not work if you're doing "Save & Activate"?

[westonruter]

Replying to nacin:

Customizing widgets before activating a theme sounds like one of two primary use cases of widgets in the customizer. What am I missing here? Do customizing widgets just not work if you're doing "Save & Activate"?

When switching themes, changes made to widgets in the customizer will be lost when you click "Save & Update". But actually, the widget instances themselves get saved properly. The problem is specifically with the widget-sidebar associations. Those are what get lost.

If you go in to the _wp_sidebars_changed() function and comment out retrieve_widgets(true) then the newly-added widgets and new widget sort orders get saved when switching themes. So there is something here when switching themes that is not accounting for changes made to widget areas in the customizer.

[gcorne]

Since @DH-Shredder asked me to guess at how much effort was involved, I started digging in a bit and came up with an idea for how to fix the issues. 27767-02.patch​ seems to do it, although it definitely needs testing, review, and docs. The basic idea is:

1. When loading the customizer with a theme that is not active, we need to retrieve_widgets(), but without any writes occurring, so I added another argument that is used to control whether writes occur. This gets tricky because we also need to do the same when loading the iframe the first time, so the patch relies on the fact that customize_messenger_channel will be preview-0 the first time.

2. When saving the customizer and activating a theme, we need to capture that the action happened via the customizer so that we don't apply retrieve_widgets() again. I think the ideal is for this data to be incorporated into the theme_switched option to avoid races.

[westonruter]

Prevent getWidgetFormControls() from returning any undefined values when no form controls are found. ​https://github.com/x-team/wordpress-develop/compare/trac-27767

[westonruter]

I found that when I tried to click to add a new widget to a sidebar, I got a JS error due to sidebar.getWidgetFormControls() not being able to find the widget form controls, and returning undefined when it can't. I updated the patch to not ever return undefined, but this doesn't address the underlying problem. As soon as I added a new widget to the sidebar, all of a sudden a bunch of other widgets all of a sudden appeared in the sidebar, but they appeared faded-out indicating that they aren't shown in the preview.

[ocean90]

Tested 27767-03.patch​ and it works so far. I agree to gcorne , that we have to add some docs here. Will do also some more tests.

wp_get_sidebars_widgets () has a filter sidebars_widgets, I think for consistency we should have it in get_sidebars_widgets() too.

[gcorne]

So, I am still learning my way around some of this code, but I think that there are three possible causes for sidebar.getWidgetFormControls() not being able to find the widget form controls:

1. There is a case where sidebars_widgets needs to be overridden based on retrieve_widgets( true, false ) and it isn't. This could be either because it needs to be overridden earlier in the execution or that the conditions that control when the overriding occurs is missing a situation.

2. There is a scenario where sidebars_widgets is being overridden based on (retrieve_widgets( true, false ) when the global sidebar_widgets has already been passed through retrieve_widgets.

3. There is a scenario where sidebars_widgets is being overridden, but not in the context of the new theme causing a mismatch.

Because of the layers of filtering involved, getting this right is a bit tricky. 27767-04.patch​ tries to solve #2 and #3, which I think are the most likely causes. I also included a fix to customize-widgets.js to fix JQMIGRATE: $(html) HTML strings must start with '<' character. I left out the fix that @westonruter came up with for the javascript error so that the symptom of the problem is not suppressed.

More testing/review is definitely needed.

[gcorne]

27767-05.patch​ is a little more robust than 27767-04.patch​.

[gcorne]

While testing, I discovered an issue with previewing twentyeleven and the widget "Twenty Eleven Ephemera." I can add the widget just fine, but making changes to the widget doesn't seem to work. All I get is a spinner. This is true both with and without 27767-05.patch​ applied, so when previewing a theme that registers widgets and interacting with those widgets, the widget customizer doesn't seem to work quite work right.

[kirasong]

Notice (with traceback included) when previewing Twenty Fourteen and adding the Twenty Fourteen Ephemera Widget:
​http://screen.getsource.net/04-12-2014-13-15-25.png

[nacin]

Another angle: How bad does this break when the current theme doesn't support widgets?

[nacin]

If we can't get this fixed we'll either need to pull out the widget customizer or delay 3.9. I don't want to do either. DH-Shredder is going to try to coordinate this for the rest of the day. This is the highest priority ticket for 3.9 at the moment. Let's push through it.

[gcorne]

Replying to gcorne:

While testing, I discovered an issue with previewing twentyeleven and the widget "Twenty Eleven Ephemera." I can add the widget just fine, but making changes to the widget doesn't seem to work

This is caused by the ajax calls with action == 'update-widget' not including the theme parameter, so the customizer doesn't load in preview mode. Unfortunately, making that change exposes issues with the other work that I had done to override the sidebars_widgets to apply retrieve_widgets() under certain circumstances. Figuring out how to best handle that requires more thought.

[westonruter]

Changes from previous patches: ​https://github.com/x-team/wordpress-develop/commit/44e75cc9e0cd35ec5dcebcc62f420c5e5b53f095 ​https://github.com/x-team/wordpress-develop/pull/10 Cumulative changes: ​https://github.com/x-team/wordpress-develop/pull/10/files

[westonruter]

I believe I've nailed down the problem. In the latest patch 27767-07.patch​:

• Run retrieve_widgets() earlier, and only when initially opening customizer in a theme switch. (The results of this function get passed along subsequently by Customizer POSTs.)
• Likewise, initially filter sidebars_widgets option to be the retrieved widgets. This restores any widget area configuration previously used on the previewed theme.
• Store the old theme's sidebars_widgets in a new customizer setting which gets supplied into the sidebars_widgets theme mod after theme activation.

[gcorne]

Replying to westonruter:

I believe I've nailed down the problem. In the latest patch 27767-07.patch​

Everything is working well for me. Much better than my attempt to get sidebars_widgets initialized correctly. :)

One minor quibble...I am not sure that old_sidebars_widgets quite communicates what is being stored there. Some other ideas are preexisting_sidebars_widgets, current_sidebars_widgets or maybe active_theme_sidebars_widgets.

[ocean90]

Found an issue with 27767-07.patch​: After saving & activating the theme the widgets are in the right place, but after visiting wp-admin/widgets.php (where retrieve_widgets() is called too) the widget is moved to another sidebar, see screencast ​https://cloudup.com/cCQwwH0gVPQ.

[gcorne]

Replying to ocean90:

Found an issue with 27767-07.patch​: After saving & activating the theme the widgets are in the right place, but after visiting wp-admin/widgets.php (where retrieve_widgets() is called too) the widget is moved to another sidebar, see screencast ​https://cloudup.com/cCQwwH0gVPQ.

27767-08.patch​ fixes this.

[westonruter]

Replying to gcorne:

Replying to ocean90:

Found an issue with 27767-07.patch​: After saving & activating the theme the widgets are in the right place, but after visiting wp-admin/widgets.php (where retrieve_widgets() is called too) the widget is moved to another sidebar, see screencast ​https://cloudup.com/cCQwwH0gVPQ.

27767-08.patch​ fixes this.

Good one! Your ​most recent change to remove_theme_mod( 'sidebars_widgets' ) was what I was also working on. This also fixes the problem for me.

[westonruter]

Apply some formatting fixes requested ocean90 in ​https://github.com/x-team/wordpress-develop/pull/10

[ocean90]

27767-09.patch​ passes all my previous failed tests. Good job.

[westonruter]

​https://github.com/x-team/wordpress-develop/commit/af03194179956fdaa054b01db555baa4caf34d98

[westonruter]

In 27767-10.patch​ use wp_loaded instead of widgets_init action.

[westonruter]

​https://github.com/x-team/wordpress-develop/commit/58d5a11f1807ea723af7ffa3434908972754afb1?w=1

[westonruter]

In 27767-11.patch​ use WP_Customize_Manager::doing_ajax() instead of other less-elegant checks.

[nacin]

In 28124:

Customizer: Properly handle widget settings when activating a previewed theme.

props westonruter, ocean90, gcorne.
fixes #27767.

[westonruter]

The resulting commit r28124 did not fix the issue, due to a typo where the logic in retrieve_widgets('customize') should have been inverted. See #27897.