Moving to BS5 with a bit of 3rd party updating

[trollfot]

This is a PR encompassing a few BS5 update for fields (classes) and a few upgrade of versions (tinyMCE, datepicker). The PR is presented early to allow comments and change in direction.

[trollfot]

Addressing : https://github.com/Pylons/deform/issues/499

[trollfot]

Addressing: https://github.com/Pylons/deform/issues/452

[trollfot]

Addressing : https://github.com/Pylons/deform/issues/431

[trollfot]

Addressing: https://github.com/Pylons/deform/issues/448

[stevepiercy]

59 test failures: https://github.com/Pylons/deform/actions/runs/6956721991/job/18928147423?pr=531#step:7:6539. See https://github.com/Pylons/deform/blob/main/contributing.md#when-to-add-or-edit-functional-tests for how to fix the tests.

[trollfot]

I'm currently trying to touch a bit of everything in a "do it first then fix it" kind of way. I'll take care of the tests once we're happy with the fixes. This PR is by no means complete, it allows us to possibly communicate of the different issues. Sorry for the noise, if it produces any.

[stevepiercy]

Ah, OK, in that case, I converted this PR to Draft status, which implies, "Please don't review yet, I'm working on it." When ready, you can either @ me or request a review from me and others, and change the status to ready for review.

[trollfot]

The upload widget is flat out not working, because of the javascript behind it. This needs some heavier work. In the meantime, checked inputs/password were altered to use an input group. this is a try out, and probably a matter of taste. The previous rendering was very unflattering, so...

[trollfot]

Starting working on the functional tests. They need a bit of love since the introduction of the html5 "required" changes a lot of the validation tests.

[trollfot]

@stevepiercy The bs5 testing is a bit of a rabbit hole. There are many things that need tuning/changing. I'm afraid it's a lot of changes for a single PR. Any thoughts ?

[stevepiercy]

One option would be to create a long-lived branch in Deform against which smaller pull requests could be made and merged. Then when the long-lived branch passes all its tests, merge to main. That would also allow me to cut incremental alpha releases from the long-lived branch. Would that work for you?

[trollfot]

It would work, but some issues are transversal, meaning, they spawn from "little" changes but affect everything. "required" is one of them. Some issues are easily bundled together. I think the current state of this PR is not too bad. I'm currently fixing tests. I won't add anything more to it and we could merge it into a new branch, maybe. I'm not quite sure

[trollfot]

@stevepiercy I'm almost done fixing tests, but most of them are not really fixes. They are from the fact we moved to a HTML5 validation, browser-side. It raises a number of questions like the i18n. The checks of submission can't be done, since the browser prevents submission until it's valid. The solution would be to "double" the tests with a <form> decorated with a "novalidate" attribute, so it would actually test deform's messages and all. It means retracing my steps and reintrocuding all these tests. The other tests not working at the file upload ones. The file upload is broken completely. I did not investigate yet. What's your opinion on this ?

[stevepiercy]

An attacker could still circumvent browser validation and submit invalid data, so the validation on the server side is still required.

I think, however, you are talking about the Selenium tests. For those, they should reflect what the user actually experiences, not what an attacker can do.

We might need to expand the Selenium tests to simulate what an attacker can do. Perhaps try that for one test on a text input to see how that might work?

[trollfot]

I've added a "disable_html5_validation" function that put a "novalidate" on the form. It helped me reinstate the previous tests and add new ones, taking care of the html5 'required' and 'validationMessage' control.

I fixed the file upload by removing the old boostrap3 "custom" upload javascript.

There is one thing still problematic : tinyMCE is no longer pre-loadable, it seems. I read wikis and forums, and i didn't find a way to make it work.

Oh, and another thing : html5 validation messages are translatable => should they be translated when we switch the language ? It should be based on the browser's language, but it's customable.

[delijati]

@trollfot maybe it helps i had the same issue in the function test and solved it my manually scrolling a bit https://github.com/Pylons/deformdemo/pull/128/files#diff-e1608f44e2798812f15d2c4a50224a8eabd5e8604a8bffbf981523345ae6e130R3369

[stevepiercy]

Oh, and another thing : html5 validation messages are translatable => should they be translated when we switch the language ? It should be based on the browser's language, but it's customable.

Indeed, they should be translatable.

BTW, I really appreciate all the work y'all are putting into this PR. I've been swamped with $WORK, and have a couple more weeks until I can adequately address progress and carve out time to make that alpha release I've been promising.

[trollfot]

@delijati What issue are you talking about ? The code you referenced is already there, since it's based on the min branch where your code was already merged. Am I missing something ?

[delijati]

@delijati What issue are you talking about ? The code you referenced is already there, since it's based on the min branch where your code was already merged. Am I missing something ?

this branch here aka this test is failing https://github.com/Pylons/deform/actions/runs/7199539548/job/19611426675?pr=531#step:7:607

the link i posted was just an example on how the test could be fixed

[trollfot]

@delijati The failing test is "FAILED deformdemo/test.py::DelayedRichTextWidgetTests::test_submit_filled"n due to the update of TinyMCE. I guess i'm totally missing the point

[delijati]

@delijati The failing test is "FAILED deformdemo/test.py::DelayedRichTextWidgetTests::test_submit_filled"n due to the update of TinyMCE. I guess i'm totally missing the point

No problem, the tinymce test is failing because tinymce is not in "view":

E       selenium.common.exceptions.ElementNotInteractableException: Message: Element <body id="tinymce" class="mce-content-body form-control"> could not be scrolled into view
E       Stacktrace:
E       RemoteError@chrome://remote/content/shared/RemoteError.sys.mjs:8:8
E       WebDriverError@chrome://remote/content/shared/webdriver/Errors.sys.mjs:189:5
E       ElementNotInteractableError@chrome://remote/content/shared/webdriver/Errors.sys.mjs:349:5
E       webdriverClickElement@chrome://remote/content/marionette/interaction.sys.mjs:166:11
E       interaction.clickElement@chrome://remote/content/marionette/interaction.sys.mjs:135:11
E       clickElement@chrome://remote/content/marionette/actors/MarionetteCommandsChild.sys.mjs:202:29
E       receiveMessage@chrome://remote/content/marionette/actors/MarionetteCommandsChild.sys.mjs:84:31

What i'm suggesting is a fix i did on some other tests to scroll manually into view

ActionChains(browser).scroll_by_amount(0, 200).perform()

[delijati]

@trollfot do you have some time to look into the remaining issues?

[trollfot]

@delijati I tried the method you gave me but I couldn't get the test to pass. I think it's the last problem

[delijati]

@delijati I tried the method you gave me but I couldn't get the test to pass. I think it's the last problem

Ok i will also try to find some time to look into it :)

[trollfot]

@delijati I have time for this, we really want it merged and moving forward

[delijati]

Hi i got the last functional test [1] to work. The problem is that the iframe inside of tinymce has a height of 0 so it is not displayed and can't be edited , is seams that the new tinymce can't calculated the height if the element is "hidden" aka display=none. My solution [2], [3] is to remove the preload span and just work with the textarea element itself.

This is also not really perfect as the text in the textarea will be shown with its html tags. Also do we really need this feature?

[1] error

=========================== short test summary info ============================
FAILED deformdemo/test.py::DelayedRichTextWidgetTests::test_submit_filled - s...
============ 1 failed, 284 passed, 7 warnings in 328.68s (0:05:28) =============

[2] deformdemo

diff --git a/deformdemo/test.py b/deformdemo/test.py
index ab4bfa2..c6eaca6 100644
--- a/deformdemo/test.py
+++ b/deformdemo/test.py
@@ -3398,7 +3398,7 @@ class DelayedRichTextWidgetTests(Base, unittest.TestCase):
     url = test_url("/delayed_richtext/")

     def test_submit_filled(self):
-        findcss(".tinymce-preload").click()
+        findcss(".tinymce").click()
         time.sleep(0.5)
         browser.switch_to.frame(browser.find_element(By.TAG_NAME, "iframe"))
         ActionChains(browser).scroll_by_amount(0, 200).perform()

[3] deform

diff --git a/deform/templates/richtext.pt b/deform/templates/richtext.pt
index 63fca94..dd17a54 100644
--- a/deform/templates/richtext.pt
+++ b/deform/templates/richtext.pt
@@ -7,13 +7,6 @@
     i18n:domain="deform"
     tal:omit-tag="">

-    <style type="text/css">
-      .deform .tinymce-preload{
-          border: 1px solid #CCC;
-          height: 240px;
-          display: block;
-      }
-    </style>
     <textarea id="${oid}" name="${name}"
               tal:content="cstruct"
               class="tinymce form-control ${field.error and error_class or ''}" />
@@ -22,9 +15,6 @@
     that might result in non-HTML-escaped interpolation of
     user input.
   -->
-  <span id="${oid}-preload" class="tinymce-preload"
-        tal:condition="delayed_load and not field.error"
-        tal:content="structure cstruct" />
   <script type="text/javascript">
     (function($){
       deform.addCallback('${oid}', function(oid) {
@@ -35,16 +25,12 @@
           selector: '#' + oid
         };
         var jqoid = $('#' + oid);
-        var jqoid_preload = $('#' + oid + '-preload');
-        if (jqoid_preload.length == 0) {
-          tinyMCE.init(options);
-        } else {
-          jqoid.hide();
-          jqoid_preload.one('click', function(){
-            jqoid.show();
-            jqoid_preload.remove();
+        if ('${delayed_load and not field.error}' == 'True') {
+          jqoid.one('click', function(){
             tinyMCE.init(options);
           });
+        } else {
+          tinyMCE.init(options);
         }
       });
       $().bind('form.pre.serialize', function(event, $form, options) {

[trollfot]

Tests are green

[trollfot]

@stevepiercy Can we review this and get it moving ?

[delijati]

I have setup a deform demo server so it is easier to cross check :) When we can merge i will stop the instance.

NEW:

http://5.75.157.57/

OLD:

https://deformdemo.pylonsproject.org/

[delijati]

@stevepiercy sorry to bother you but can we get a ETA when this can be reviewed and merged?

[trollfot]

Could we get the ball rolling ? It's been a while :)

[stevepiercy]

I no longer use deform in any of my client projects. I've pinged the other maintainers here @miohtama @mcdonc @ericof to see if they have any interest in taking it on.

We could also take on new maintainers. Would you be interested?