KepaUrzelai / moodle-atto_multilang2

:globe_with_meridians: A plugin for Atto editor to make easier creation of multilingual contents.
0 stars 6 forks source link

content is not saved if modified in HTML view #8

Open taraghi opened 2 years ago

taraghi commented 2 years ago

Hi,

I think I have unfortunately discovered a somewhat annoying bug:

If I want to edit a text field and switch to the HTML view of the WYSIWYG editor, modify the text and then save the changes, then they are not applied. However, if I switch back to the "Preview" first, then the changes are saved. This happens only if this plugin is installed.

Could you guys please take a look at this?

izendegi commented 2 years ago

Hi @taraghi

We've been trying to replicate that behaviour but haven't been able to make it happen (it works as expected), please tell us more information about your installation (mostly Moodle version & plugin version) and a more detailed explanation of your steps and we'll try to replicate it.

taraghi commented 2 years ago

Hi

we have Moodle v3.11.10 and the last plugin version v3.9.1.12 running.

Assuming wie have a label activity where a teacher wants to edit the content: Label activity => Edit => Atto Editor

  1. Teacher switches directly to the html view in Editor
  2. Teacher Adds a Text, modify the text etc.
  3. Teacher triggers then directly the Moodle form save button at the bottom of the page, without switching back to the Editor visual view first

In this case his/her modifications in Editor are not saved in Moodle.

If the teacher first switches back from html view to the visual view and then clicks on the save button, the modifications would be saved.

I hope I could describe the issue more clearly now.

XiRouz commented 1 year ago

+1 here, if HTML view is not closed manually before saving any element (book, question, etc.), changes to HTML are lost My guess is, the problem lies in hooks to functions "updateOriginal" and\or "updateFromTextArea".

Moodle 3.8.9 Plugin version: master - Release v1.11.2 (Build 2019112700) for Moodle 3.6, 3.7 and 3.8 2019112700

KepaUrzelai commented 1 year ago

Hi @taraghi and @XiRouz,

I understand the issue but I'm unable to replicate it.

The tests I ran where...

...and each of them resulted in a correct behaviour when saving the content directly from Atto's html mode.

It would be helpful to know if you are using any aditional Atto editor plugin to see if there could be any colition or incompatibility.

I can also think about a special Moodle theme with some javascript making that happen.

Let me know if you discover any non-core plugin on your instances that could inpact on it.

Thanks

XiRouz commented 1 year ago

Good day, @KepaUrzelai

I've just checked out this plugin with clean Moodle v4.1 and got the same issue. Steps to recreate were described above by @taraghi, I can't add anything to that list.

Are you sure you didn't forget to include "lang = multilang2" string in atto config? At first I couldn't replicate this behaviour on a fresh Moodle too, until realized that I forgot to setup plugin properly and JS part of it isn't autoloaded until you change the Atto config.

I'm still getting my head around JS, Atto and other stuff, but I might've found the reason. Usually, on form submit, if editor is STILL in HTML mode, _hideCodeMirror() is called to hide HTML view and copy data (with updateFromTextArea()) from it to Atto editor. With this plugin, _cleanMlangTags() is called before _hideCodeMirror().

_cleanMlangTags calls markUpdated() and then updateOriginal() inside of it, where oldValue and newValue are actually mixed up in wrong way. Function updateFromTextArea() is never called so our older data from Atto editor "replaces" new data, which was written in HTML textarea editor.

image

As you can see on the screenshot, Atto editor textarea's value is set to old "editor content" on submit.

I made a simple workaround and created a pull request, although this solution might be a bit wanky.

Please let me know if you still can't replicate this behaviour, I can make a video for you.

Thank you.

KepaUrzelai commented 1 year ago

Hi all,

I checked the plugin on a clean Moodle 4.1.1 instance again (Postgresql v12) , with the only aditional plugins being: filter_multilang2 1.1.2 (build 2020101300) and atto_multilang2 1.12 (build 2021063000) and still can't replicate the bug.

But, I also tested @XiRouz's PR #10 and it keeps working as expected, so if the patch works for @taraghi we could mark it as valid, make some more tests and take it into integration.

Waiting for your review,

Regards

XiRouz commented 1 year ago

@KepaUrzelai Perhaps for some reason in your case functions that are listening for "submit" event, are called in other order, starting with atto_html which checks if editor is in HTML mode, and after hiding codeMirror + handles textarea update from HTML mode, and then _cleanMlangTags() function from atto_multilang2 is called. You can try to place breakpoints in functions hideCodeMirror() and _cleanMlangTags() to see, which one is called first in your Moodle. Those are located somewhere in theme/yui_combo.php?... files.

XiRouz commented 11 months ago

Hello @KepaUrzelai @izendegi, are there any plans on closing this issue? My requested solution works fine on our production LMS, but we are worried that if this atto plugin will updated or we upgrade our LMS at some point, our admins might forget to include this bug fix manually.

If no other options or fixes will be present, my pull request is still there, and I would appreciate if you would merge it. Thank you!