Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Editing document not working second time with Plone 5.1 #36

Open
gp54321 opened this issue Feb 7, 2018 · 6 comments
Open

Editing document not working second time with Plone 5.1 #36

gp54321 opened this issue Feb 7, 2018 · 6 comments

Comments

@gp54321
Copy link

gp54321 commented Feb 7, 2018

After fixing issue #35 there is a remaining problem, after editing a document to add a term covered in the glossary it is not longer possible to edit the document, the editor window shows up empty.
This is caused by the Tinymce implementation in Plone 5 having a hidden window as a buffer between Plone and the javascript implementation, the actual editing window being generated dynamically.
I think than meddling with this internal window is not very useful anyway since it could at best lead to the glossary active in the Tinymce editing window, but editors are probably supposed to know the meaning of terms they use. So a Plone 5 hack could be:

--- jquery.glossarize.min.js.ori	2018-01-27 15:32:19.204510000 +0000
+++ jquery.glossarize.min.js	2018-02-07 10:34:25.946612435 +0000
@@ -155,6 +155,8 @@
         /*
           Text Node
         */
+        /* Plone 5 hack: do not mess with Tinymce internal buffer */
+        if (node.parentNode.name == 'form.widgets.IRichText.text') return;
         var temp = document.createElement('div'),
         data = node.data;
         var re = new RegExp('(?:^|\\b)('+this.cleanedTerms+ ')(?!\\w)', base.regexOption),

tested with Plone 5.1 rc1, the edit window can be used normally and the tooltips show fine when the save is done.

@hvelarde
Copy link
Member

hvelarde commented Feb 7, 2018

it would be nice to add a test to demonstrate the issue.

@gp54321
Copy link
Author

gp54321 commented Feb 7, 2018

I feel dumb, I can't quite understand what you are asking for, is it some screenshots ?

@hvelarde
Copy link
Member

hvelarde commented Feb 7, 2018

let me explain: issues are not fixed until a commit with the fix is merged and a new release is created; if you only document how to fix a problem and no commit is created and merged, then the issue is not solved.

most of the time, it is good to add a new test that demonstrates the issue using Python's unittest or Robot Framework-based Selenium tests.

IIRC, we're not running the later tests under Plone 5 because there were some compatibility issues in the past and I don't know if they are already fixed.

to run the tests you need to use an older version of Firefox and comment this lines locally; then you can use bin/test

@gp54321
Copy link
Author

gp54321 commented Feb 7, 2018

I was afraid that was you were asking. Let me explain more precisely what is happening.
When I set a breakpoint in Firefox dev at this line 228 of :
node.parentNode.removeChild(node)
when it stops, if click the run icon, everything works fine.
If I add a dummy line doing exactly nothing just after and I set up a breakpoint on this dummy line, the problem happens. So it depends on parallel execution, something of a race condition, the problem comes from Tinymce Javascript running at the same time and not playing nice with collective.glossary Javascript code.
Do you have many unittests in your code base that are designed to test for cases like that ? Because I have just no idea about how to do it. If possible it would not be an easy task to do. Definitely not worth the bother for me, my own goal being to evaluate the difficulty in migrating Plone 4 add-ons to Plone 5.

@hvelarde
Copy link
Member

hvelarde commented Feb 7, 2018

I think we should have a test for something simpler: just follow your own rationale and add a part to edit a document a second time in the existing test: if the issue is there, we'll see it immediately on the tests.

@gp54321
Copy link
Author

gp54321 commented Feb 7, 2018

Hmm.. I think I see your point. For a 5.2 Plone version, the TinyMce version could change and the window name could change too, and the hack would no longer work. This is a totally valid concern. However, it is still some immediate work for a relatively long term goal (can you guess that finding this work around has not been really easy and fast ?) and I need to get a working-for-me (that is without all the annoying warts) Plone 5.1 to present to possible customers. I need it, the sooner, the better, so I'll pass for now (never say never).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants