When I translate a confirmation message that contains HTML code, the HTML code gets converted to HTML entities.
| Comment | File | Size | Author |
|---|---|---|---|
| #54 | confirmation_message-2040511-54.patch | 4.1 KB | idflood |
| #25 | confirmation_message-2040511-25.patch | 5.3 KB | radubutco |
| #15 | confirmation_message-2040511-9.patch | 3.05 KB | fietserwin |
Comments
Comment #1
yoshz commentedSee attachment for my workaround.
Comment #2
GDrupal commentedA perfect work around indeed...but perhaps the best way to approach this is to add support for webform_tt so we can add some elements to the options array. So we don't sacrifice consistence. What do you think. It will imply to patch webform too, just a small patch, but that could be a little slow.
Comment #3
GDrupal commentedShould be fixed by this commit . Please review and mark as fixed if it works.
Comment #4
GDrupal commentedWorking ok right now.
Comment #6
fietserwinThis (still) doesn't work in the 4.2 version, nor the current dev version (17-12-2015). I will post a patch shortly.
Comment #7
fietserwinPatch attached.
I tested this patch and it works fine for me.
- via node/{nid}/webform/configure
- via admin/config/regional/translate/translate
- and it works for the visitor.
Comment #8
fietserwinPatch contains wrong folders, (starting at my site root instead of this project). Again
Comment #15
fietserwinI didn't test creating a new webform and setting the translation mode ...
(kind of) interdiff:
Comment #16
Philou88 commentedWith patch 9, with HTML choice, impossible to translate the message, I have a warning message.
I came back in Plain Text, and the translation impose
in the message : "
Thank you for your inquiry. We will contact you shortly.
"
Comment #17
fietserwinThanks for testing the patch.
- What is the warning message?
- Did you configure your site to allow to translate filtered and full html (admin/config/regional/i18n/strings)? If not:
* you will get a warning on trying to update the webform in a non default language ({langcode}/node/{nid}/webform/configure)
* you won't see the string at all in the translation interface (/admin/config/regional/translate/translate).
Comment #18
joseph.olstadWe likely need to add a sub module to automatically allow translate filtered and full html, then enable it automatically when string translation single webform option is selected.
This should help the related issues I was banging my head on
Thanks for the good idea, need to use features to create this sub module with required configuration .
This way we should be able to get this working out of the box
Comment #19
joseph.olstad**EDIT** see previous comment
Comment #20
joseph.olstad**EDIT** see above comment
Comment #21
fietserwinUnless, I understand your remark incorrectly, I don't think we should "automatically allow translate filtered and full html". That is governed by an access right and thus should be obeyed.
Comment #22
joseph.olstadAccess rights for translating strings is what will control who can translate your webform. What the problem is now is that we don't have ANY reasonable filter settings out of the box. i18n_string_xss_admin is the most permissive filter that is out of the box for the i18n_string function however it's too restrictive and removes important tags. Our use case of i18n_string is unique in that our whole webform is translated this way when selecting the string translation method. We need more robust filter options but i18n_string does not provide this out of the box we have to therefore create our own filters.
Currently a webform is created , the filter options are only respected in one language, as soon as i18n_string kicks in its filtered again using the filter as defined in our i18n_string function calls , or else converted into plain text.
The way to fix this is to add filters, capture appropriate filters in a feature, put the feature in as a sub module, then when "single webform" options are selected in the webform_localization config options, to enable the sub module with the string filter settings, we'll have to also replace the i18n_string_xss_admin and i18n_string_xss filters with the sub module filters (could be an option in the gui for which type of filter to use) , then at that point rather than losing tags or having your html converted to plain text the translation will can have the same markup as the original webform as per the string translation.
At least this is what I'm thinking in theory. Got any other suggestions?
Comment #23
fietserwinIMO, this should thus be the same for the translated text:
So, I don't see any need to write our own filter functions or am I still missing something?
Comment #24
joseph.olstadduplicate of #2633966: Translated e-mail body sent as plain text
Comment #25
radubutco commentedI'm not sure this problem is solved on last dev version.
Rerolled the patch from #15 to work with last dev version.
Additionally I've changed the format option sent to i18n_string, from $node->webform['confirmation_format'] to I18N_STRING_FILTER_XSS, because the browser got stacked on full_html format option.
Also, I've changed all the misleading text 'poperties' into 'properties'.
Comment #26
radubutco commentedComment #31
joseph.olstadComment #32
joseph.olstad@radubutco thanks for the new patch. Since this is fresh in your memory, would it be too much trouble to ask for detailed steps to reproduce from a fresh D7 vanilla install? If its easier, include a node export of your webform, or a feature to capture some settings variables. This will help us and others save time involved in reviewing and testing the patch. It might lead to an additional simpletest test for this scenario.
I spent quite a bit of time on this and a similar issue, want to make sure I have the test scenario right.
Comment #34
joseph.olstadComment #36
fgjohnson@lojoh.ca commentedJust downloaded the latest .dev version.
This is still an issue.
Strips out most of the html.
When I applied confirmation_message-2040511-25.patch it assumed Reversed (or previously applied) patch detected!
I choose Assume -R? [n] y and all components applied and the issue seems to be resolved.
Perhaps I need a refresher on patches.. :-)
...why did I need to elect to assume Y?
Comment #37
joseph.olstadok, so you're saying rolling this back fixed your problem?
It appears from what you described Assume -R
and you selected 'y'
then you rolled back patch #25
Unfortunately I don't have much time for this project lately, but I can rollback this commit and make a new release .
Please confirm that this solved the problem for you?
Seems that this might be a combination of configuration/code.
but if it works best without #24 then great.
Comment #38
joseph.olstadreview rolling back #25
Comment #40
joseph.olstadI rolled back patch 25 on the dev branch.
Going to push a new release in rolled back state.
Ideally NO html should be stripped at all. User input yes but not structure/config of Webform fields .
User input validation I Believe is handled entirely by webform not webform_localization. If I recall this is i18n_string forcing us to do this.
As a work around I suggest we roll back 25 as I have done and foradd documentation for theming webform using tpl instead of field label html and webform config.
Unless someone can finally fix this for good.
I Might think that we could permanently fix this by patching i18n_string adding a new more permissive filter.
This issue has dragged on way too long. Getting annoyed
Comment #41
joseph.olstadThen again, the confirmation message is probably not themeable in a way that I might try to in a workaround as I suggested.
Previously immediately before patch 25 was committed and now presently that it is removed the confirmation message gets some basic html , whatever xml_admin filter will allow
Comment #42
fgjohnson@lojoh.ca commentedJoseph,
Thanks for revisiting.
I'll try out 4.9
seems to be the proper thing to me.
-------------
Sadly,
I have some custom bits that might be causing some additional pain for me!
(On a side note...
Do you know if Wetkit will be updating their version of webform_localization from 4.6? )
Comment #43
idflood commentedI believe there was some confusion in #36 / #37. I tried the latest dev version and the
tag was still escaped. I then applied the patch in #25 and the issue is resolved for me. I applied the patch with the following commands and it all went perfectly:
Comment #44
joseph.olstad@idflood I suspect there may have been some custom filters in either your environment or @fgjohnsons` environment ,
It would be helpful to have a test scenario with detailed configuration /setup instructions from a fresh d7 installation, this would really be helpful to pinpoint what is going on here , consistency/inconsistency.
thanks for any assistance
Comment #45
fgjohnson@lojoh.ca commentedJoseph,
Sorry... long.
Tested in a default Wetkit from simplytest.me this am.
My issue may be related to Wetkit 4.12
Maybe I should open an issue @ the Wetkit issue queue?
webform - 7.x-4.14
wetkit webforms - 4.12-12+1dev
webform localization - 7.x-4.9
localization client - 7.x-1.3
WYSIWYG Text Filters / Order
---------------------------------------------------------
Enabled WetKit Webform and it's dependencies.
Created a Webform
Added a field to that webform
Navigated to Webform / Form settings / Confirmation message and added an English message.
Saved the Form.
Clicked Translate / Add to add a french Translation of the Webform.
Clicked Webform / Paramètres du formulaire
Types in 3 lines of text.
When we save the Translation the WYSIWYG confirmation message appears like the following in the default WYSIWYG config.
This is the French Confirmation content as it appears in the WYSIWYG.
<p>Three lines</p> <p>One Token = <a title="Insert this token into your form">[current-date:long]</a></p>Switching to Source we see that this has happened.
<p><p>This is the French Confirmation content</p> <p>Three lines</p> <p>One Token = <a title="Insert this token into your form">[current-date:long]</a></p></p>Comment #46
joseph.olstadadding an old 6.x i18n issue as 'related' just as food for thought.
It'd be nice to get to the bottom of this issue once and for all.
Now that we know that the wetkit distro is affected, we just need a d7 vanilla test scenario to confirm that patch #25 actually works for that scenario.
Once it is determined if this is wetkit config or not, we can then open an issue in the wetkit queue for that.
TODO:
I'd like to know if patch #25 can work with d7 vanilla plus wetkit_localization dependencies and a minimalist configuration. We just need a test case scenario for that.
any assistance is appreciated
Comment #47
idflood commentedI will try to come up with a proper step-by-step to reproduce the issue on vanilla drupal but first here is a manual reroll of path #25 since it doesn't apply anymore (the comments have been fixed).
Comment #48
idflood commentedSo here is a step by step way to reproduce it:
<p>This is a test<br>Will it keep my <em>tags?</em></p><p>another paragraph for test</p>Now if you submit the translated form the confirmation message should display the message exactly how we entered them (tags escaped, html code visible). In original language the message is displayed correctly.
So now we can apply the rerolled patch:
Once this is done:
<p>tag for instance). Hit "save translation" withtout changing anything.Note that this time the
<em>tag is kept but the<p>get losts (the message is wrapped by what i believe is a default<p>)So I think there is a small missing piece in patch #25. Here is a new one which add a little twist in webform_localization_translate_strings. In that function #25 force the confirmation message to be in I18N_STRING_FILTER_XSS format, but interestingly we can get the real format confirmation format from $node->webform['confirmation_format'].
So now let's try to clone again the 7.x-4.x branch and apply the new patch. Once this is done:
<p>in the list of allowed tagsComment #49
joseph.olstadwow @idflood, great job! How do you suggest we go from here? So assuming we apply your patch, is there some sort of way we can warn the user if they have not allowed "Filtered HTML" format to be translateable? So that we can kind of help people along the way get this working correctly?
Comment #50
joseph.olstad@fgjohnson , any thoughts? with instructions by @idflood, does this help you get it (his patch) working in your custom environment/distro? and more importantly, pinpoint the divergence
Comment #51
idflood commented@joseph.olstad I think we should double check that whether or not these translation get lost. If this is really the case and the new patch still looks like the way to go then:
Eventually the solution n°2 would be really sad : / I will try to replicate the issue and try to debug what happens in the database.
Comment #52
idflood commentedI found why the translation got deleted. It happen because the confirmation message format is not in the "translatable text formats". If I refresh the webform localization string while the "filtered html" format is not checked then this message is displayed:
Once it happens the entry about the confirmation message in the table
i18n_stringget deleted. If I first enable "Filtered HTML" translation and only after update the webform localization strings then the message translation is not deleted.So one potential fix to this issue would be to first check that the format is allowed before refreshing the confirmation translation string. If it is not then we could display a warning message like "The string […] could not be refreshed because the text format XYT is not allowed for translation".
Comment #53
idflood commentedWell, I tried to simply add some condition in
webform_localization_translate_stringsbut it's not really successful yet (too much warning repeated and I'm not sure it's the correct way). For reference I replaced:With something like that:
Comment #54
idflood commentedHere is a new patch which tries to prevent translation text loss. So what it does is that it keeps by default the
I18N_STRING_FILTER_XSSformat, and only if the confirmation format is supported use it for the translation. If it is not then a message is displayed informing about the impossibility to use this format.I will try to test it on some websites to see if everything goes well but it would be awesome if we could get some other confirmation.
Comment #55
joseph.olstadHi @idflood , great work again, I'm hoping that @fgjohnson can try this patch on his environment as well.
I'll run some of my own tests soon, we're working on some new webforms as we speak so we'll be needing this.
Comment #56
joseph.olstadOk, we did some more testing, according to our tests, version 4.8 with the original patch was actually working fairly well but did strip some html but overall was better than without patch
Version 4.9 (no patches) the HTML gets rendered into HTML entities, so nope, not good.
Now with patch #54 we're back in business and our translation appears to reflect what was in the source language. Great work @idflood
Comment #57
gdaw commentedNice patch folks, 4.8 stripped some HTML and 4.9 translation was full-on HTML displayed, but this latest patch keeps even the most complicated HTML intact.
RTBC + 1 , and thumbs way up!
Comment #59
joseph.olstadComment #60
fgjohnson@lojoh.ca commentedidflood Your head must be spinning... mine is just reading! :-)
I will try to confirm all this in the next couple of days...
Comment #61
fgjohnson@lojoh.ca commentedSaw that this was committed to 7.x-4.10
Tested in Wetkit 4.12.
Seems to be fixed there too.
Comment #62
joseph.olstad@fgjohnson, thanks for the followup, good to hear this issue is finally resolved.
Comment #63
joseph.olstadmight be nice to get some extra test coverage on this though ( an extra assert in a simpletest)