This module currently adds custom JS code to the message structure of the webform-confirmation template and assumes that it is provided as Webform's default HTML editor markup.
But Webform can be configured to use another text format for all rich-text fields, in which case the custom code fails to get called when a webform is submitted.
Expected result: The custom code is executed.
Actual result:The code is not executed.
In case said setting is active, the confirmation message is provided in a different form which this module does not expect. Since most text formats will most likely not allow <script> tags and I don't know how to allow them just for this use case, it might easier to transform the message back to Webform's default HTML editor markup before adding the tag.
Agree on solution. Commit.
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
anybodyThanks @mrshowerman!
I think as first step, we should add a warning in this module, if the setting you're talking about is enabled. This should be detectable from the webform's config value.
Honestly, I'm not sure if this is solvable in a secure way at all, as the editor might just strip the JavaScript and that's not something we would revert.
If that's the case, we need to look for other solutions, but when writing this module we already did that and didn't find a better way without needing Webform to provide support for this case and that was denied so far...
Very happy to review patches and further ideas!
Comment #4
mrshowermanI know that we might lose some formatting in the confirmation message if text format features are used that aren't supported in Webform's editor markup, but since we can set the text format for all elements only and not on a per-element basis, the only way is to override the format.
I think this is acceptable in this case, because when you actively use this module, you would expect to do what it should.
That's what the MR is trying to do.
I agree that adding a warning to the field if a different format is selected might be helpful, so site builders aren't surprised by the format change.
Comment #5
anybodyThanks @mrshowerman looks acceptable. Still the code and comments aren't clear enough from my perspective to understand why this is needed and what it does. Could you improve that?
Someone new into the code should be able to understand what these lines do and why they are needed. Otherwise, this might be removed one day in a refactoring, if someone thinks it's unnecessary.
Comment #6
anybodyOnce finished, we also need test coverage to ensure the current functionality isn't broken (not matching the if) and the described functionality works as expected (matching the if).
Comment #7
mrshowermanI updated the code comment so it's hopefully clear what we're trying to do. Also added some explanation to the confirmation message help text.
I'm working on a test as well, but that's not finished yet.
Comment #8
mrshowermanLet's see if this test passes
Comment #9
anybody@mrshowerman thanks! Please note: #3389136: Fix failing tests and code style issues
Comment #10
mrshowermanYes, I saw those code style issues. Good to know there's already an issue.
Comment #11
mrshowermanAdded test group, but test still fails, probably due to #3390639: Tests fail because webform stable is not Drupal 10 compatible.
Comment #12
guillaumeduveauHi, it looks good however there's another case : if we use CodeMirror for the rich-text fields, there is no allowed_tags list. But maybe it's misconfiguration on my end or an issue related to webform itself.
dpm($variables['message']);