Updated text widget do not save text (when using paste)

[rinkuyadav999]

Hi

I was testing updated text widget in 4.8. It saves text normally but when i switch visual to text 2-3 time and then

when i past text in visual and switch to text and press save button. it does not save text. Screenshot: ​https://i.gyazo.com/1e760893e6f830a3747204e2772a2407.mp4

[ketuchetan]

Hi @rinkuyadav999,

I have tried to reproduce the issue as per your video. But, I am not facing this kind of issue.

Could you please give me instruction to reproduce the issue?

Thanks,

[rinkuyadav999]

Okay i have created a video for this issue : ​https://www.youtube.com/watch?v=q2ZeW2IxotI

System: Window 10 (64 bit), Firefox 53.0.3 (32 bit), WordPress 4.8, No plugin, default twenty seventeen theme.

[westonruter]

@rinkuyadav999 I notice you're pasting the text into the widget. What about when you actually type text into the widget? It could be an issue with TinyMCE not picking up paste events properly to trigger a change.

[westonruter]

Actually, I can reproduce this issue. If you switch switch to the Text tab too quickly (in under a second specifically) after entering text into the Visual editor, then the text won't get synced into the underlying hidden due to the 1 second debouncing. That being said, it should also be syncing the TinyMCE value upon blur as well.

Here is the underlying code: ​https://github.com/WordPress/wordpress-develop/blob/4.8.0/src/wp-admin/js/widgets/text-widgets.js#L194-L198

[westonruter]

For some reason the blur event is not triggering when I click the Text tab from Visual.

[rinkuyadav999]

Yes, when i change changeDebounceDelay = 1000 to 1, it works but i think it creates other similar issues.

Past some text to text area in visual mode and click on text mode tab and save. it will save. Go back to Visual, remove pasted text, go to text tab and save. there will be removed text.

[westonruter]

The key to reproduce is to switch the tabs under the 1 second debounced NodeChange event handling.

[westonruter]

@rinkuyadav999 I think I see what is going on. When a user switches from the Visual to the Text tab, the TinyMCE editor is hidden and apparently in the process the blur event never triggers. I can see however that when the editor's hide method is called, it will do a save and trigger a SaveContent event. So if we switch from blur to SaveContent this seems to fix the issue. Please test 40974.0.diff​.

@azaozz Does this make sense?

[rinkuyadav999]

Hi @westonruter

I have applied above patch. when i past text in visual and switch to text and press save button. it adds p tag and save value for first time. Now remove text in text mode and save. now go back to visual, past some text, go to text mode and save. there will be not pasted text (main issue again).

[westonruter]

@rinkuyadav999 I'm having a very hard time reproducing this bug. It's hard for me to follow the steps. Could you create a video that shows the issue when you have the patch applied?

[rinkuyadav999]

Hi @westonruter

I have created a new video after applying patch 40974.0.diff : ​https://youtu.be/KxDlBAG0QsQ

First time, it adds p tag and second time same problem.

[westonruter]

​https://github.com/xwp/wordpress-develop/pull/237/files/b0fc12e..032a888

[westonruter]

@rinkuyadav999 thanks, I can reproduce that. So it seems specifically to be a problem when pasting content into the widget. It seems when paste happens, the editor's dirty state is not changed. So I've manually implemented that in 40974.1.diff​. Please test.

@azaozz Please review, as there may be a TinyMCE defect here.

PR: ​https://github.com/xwp/wordpress-develop/pull/237

[rinkuyadav999]

Hi @westonruter

Yes, normally it works fine. But when we past text in visual and then switch to text, it creates issue.

I have applied 40974.1.diff and now it is working fine.

Thanks

[westonruter]

Please add commit keyword if the changes here check out.

[azaozz]

Hm, this doesn't look right. There is also editor.on( 'hide', ... ) which AFAIK is used internally to save the content to the textarea. That's how it works in the "main" editor.

​When testing editor.isDirty() also seems to work properly after pasting. I'll play with this a bit more.

[azaozz]

In 40974.2.diff: use editor.on( 'hide', ... ) to run triggerChangeIfDirty() when the Text tab is clicked.

Seems to work well here. The editor.isDirty() is not yet set when the paste event fires, can pass the event and check it instead of setting it manually.

@rinkuyadav999 can you test if that works for you too please.

[rinkuyadav999]

Hi @azaozz

This is working but when i past text in visual and then quickly switch to text and press save button, it remove text from text area and data does not save.

When i past text in visual and wait for 2-3 seconds and then switch to text and press save button, it saves data perfectly.

Thanks

[rinkuyadav999]

Testing video: ​https://youtu.be/ReV6eZZhkp4

[westonruter]

@azaozz do you have a recommendation on a way forward to resolve this?

[azaozz]

Replying to rinkuyadav999:

This is working but when i past text in visual and then quickly switch to text and press save button, it remove text from text area and data does not save.

Yeah, can reproduce it too. Happens only on the Widgets screen not in the Customizer, and with any input type, typing does it too. It doesn't happen in the Customizer as it blocks saving. You cannot do a fast save there as the button is disabled. Have to wait for it to sync...

Happens because the editor is added on "fake" input fields, the "real" fields are hidden and sync from the fake fields on a delay. Not sure why the Text widget needs that, all seems to work better if the "real" fields are used directly.

If the fake fields are indeed needed, the way to fix this is to reduce the syncing delay or to introduce a way to sync with no delay.

[westonruter]

@rinkuyadav999 would you test 40974.4.diff​?

[azaozz]

40974.4.diff looks good.

[westonruter]

@azaozz I was still finding issues with previewing changes to the Text widget in the Customizer, namely when making a change in TinyMCE and then switching to the Text tab before the 1s debounced delay for checking if dirty.

Here's the change that fixes that issue: ​https://github.com/xwp/wordpress-develop/pull/240/commits/366c20afa49acfea1f0c34bae9a366718b6a7c69

Essentially we just keep around a second flag, in addition to the dirty state, to keep track of whether or not we need to trigger the change event for the sake of syncing.

[westonruter]

In 41094:

Widgets: Improve Text widget's reliability in syncing Visual tab's contents with Text tab and with hidden sync inputs.

Amends [40631].
Props azaozz, westonruter, rinkuyadav999 for testing.
See #35243.
Fixes #40974 for trunk.

[westonruter]

Opening for 4.8.1.

[westonruter]

In 41095:

Widgets: Improve Text widget's reliability in syncing Visual tab's contents with Text tab and with hidden sync inputs.

Merges [41094] onto 4.8 branch.
Amends [40631].
Props azaozz, westonruter, rinkuyadav999 for testing.
See #35243.
Fixes #40974 for 4.8.1.