Customizer changes to input field values after clicking Save not recorded

[mb5324897]

Using Twenty-Fifteen:

in the customizer sidebar, change the value of a text input, e.g. the site title to "abc". The changes will be reflected in the preview window, and the "Saved" button on top will change its text to "Save & Publish".

When clicking on "Save & Publish", a spinner will appear and the AJAX request for the updated value will be sent. If the connection to the server is really slow (which can be simulated with command line tools like tc), the user has time to modify the contents of the text input once more, e.g. to "123". After some time, the server's response will arrive and the client code will consider the current value "123" as "saved". It is now not possible to click "Save" again, the button is still disabled.

When reloading the page, the state resets and the client correctly considers "abc" to be the value stored on the server again.

The problem is in wp-admin/js/customize-controls.js, where after the "Save" button has been clicked, but before the response has returned, additional KeyUp events on the text input do not cause a state change.

[westonruter]

It sounds like we just need to disable the Save & Publish button while a save Ajax request is currently in progress. We may also want to disable controls from being modified while a save request is being made.

[westonruter]

This is closely related to the implementation of transactions (#30937). Updates to a transaction should be suspended when a save operation is happening.

[chandrapatel]

I disabled 'Save & Publish' button and controls after click on 'Save & Publish' button.

[chandrapatel]

Replying to westonruter:

It sounds like we just need to disable the Save & Publish button while a save Ajax request is currently in progress. We may also want to disable controls from being modified while a save request is being made.

Hi,

I have submitted patch to disabled 'Save & Publish' button and controls while a save Ajax request is currently in progress.

Currently, I have disabled input, textarea, select, button controls in submitted patch.

Its not possible to disabled some controls like color picker. Actually text box inside color picker will disabled but still we can use color picker. And many other controls which we can't disabled.

Question: Why we can't keep 'Save' button enabled?

Thanks.

[westonruter]

@chandrapatel A reason for why we can't keep the Save button enabled because if the user keeps trying to save new changes while other changes are being saved, there could be a race condition where one save request could finish in an unexpected order.

I'm concerned that your 32941.patch​ would have unexpected side effect of re-enabling inputs that were previously set to be disabled after a save completes. One possibility is to capture whether or not the input was initiallydisabled and skip enabling such elements after the save completes.

Alternatively, there could be a CSS solution where pointer-events:none or cursor:progress is added to the controls. Unfortunately there's no CSS property to make inputs inert yet.

Yet more alternatively, a fieldset element actually can take a disabled attribute and it disables all inputs inside. So that's another option, where a new fieldset could be injected as a child of the existing form and the disabled property could be set to true during save. It's too bad that the disabled attribute itself.

[chandrapatel]

I have added <fieldset></fieldset> element as child of form and then applied disabled attribute to this element which disabled all controls.

[chandrapatel]

Hi @westonruter,

Okay. Now I understood why we can't keep the Save button enabled. Thanks.

I have added fieldset element as child of form and then applied disabled attribute to this element which disabled all controls. I have also check whether disabled state enabled or not for fieldset. If it enabled then only I remove disabled state from fieldset.

I have uploaded new patch 32941.1.patch.

But I noticed some bad behavior for color picker and drag/drop widgets. To see this behavior add sleep(30) code at line #1032 in save() method in wp-includes/class-wp-customize-manager.php. Then use and check behavior of color picker and drag/drop widgets.

Another alternative solution is, We disable only Save & Publish button. User can change settings during ajax request in-progress and we set flag if any settings changed and once ajax request complete then we check if flag set then we enable Save & Publish. So user can save their settings again. If flag not set then save button in disabled mode.

Please let me know your suggestion.

Regards

[westonruter]

Replying to chandrapatel:

Another alternative solution is, We disable only Save & Publish button. User can change settings during ajax request in-progress and we set flag if any settings changed and once ajax request complete then we check if flag set then we enable Save & Publish. So user can save their settings again. If flag not set then save button in disabled mode.

That's a good idea. After the successful save response comes back, if any change events triggered on wp.customize during the save request, then the saved state should be reset to false instead of being true, and any of the settings that were modified during the save request should not have their _dirty state reset to false.

[chandrapatel]

I have set flag if any settings changed during the save request. Then I check flag is set or not after saving request completed. If flag set then keep save button enable otherwise in disable mode.

[chandrapatel]

Replying to westonruter:

That's a good idea. After the successful save response comes back, if any change events triggered on wp.customize during the save request, then the saved state should be reset to false instead of being true, and any of the settings that were modified during the save request should not have their _dirty state reset to false.

I have submitted new patch. Please check and let me know whether my implementation is correct or not.

Regards

[westonruter]

@chandrapatel instead of introducing a new is_dirty flag, what do you think of 32941.3.diff​? It keeps track of whether there were settings changed during the save request, and then re-sets the global dirty state if there were any.

[chandrapatel]

Replying to westonruter:

@chandrapatel instead of introducing a new is_dirty flag, what do you think of 32941.3.diff​? It keeps track of whether there were settings changed during the save request, and then re-sets the global dirty state if there were any.

Hi, I saw the diff. Sounds good. Thanks for correcting me. :)

[westonruter]

@chandrapatel thanks for your work!

[westonruter]

In 37346:

Customize: Ensure settings modified during an open save request remain dirty when save request completes.

Also disables Save & Publish button while save request is open. After the save request completes, any settings changed during the request can then be saved via an additional click to the button.

Props chandrapatel, westonruter.
Fixes #32941.

[westonruter]

In 40627:

Customize: Wait for processing state to clear before starting to captureSettingModifiedDuringSave.

See #32941.
Fixes #40729.