tal:content and tal:replace are sometimes treated as to be translated #876
Comments
|
Viktor Dick wrote at 2020-7-20 01:34 -0700:
...
> That said, the approach taken is quite different from how Chameleon was first imagined.
Indeed -- to make sure that the same TALES implementation
can be used all over Zope and not one for templates and a completely
different one for CMFCore, portlets, DCWorkflow, ...
The idea with expression compilers is that they generate code at compile time, not at render time (even if it is cached). I would imagine that quite a bit of performance is lost due to the extra function calls and scope changes.
There will be performance loss -- but is it really significant,
say more than about 1 % in practical cases?
We have seen some weird behavior when switching to Zope4 that seems to be related to the Chameleon cache.
I do not think that the `chameleon` cache is to blame: it caches only the code
(similar to what Python does with ".pyc" files) - it does
not cache runtime objects (such a the translate function).
We use a translation service that effectively delegates translation to a Python Script in the context of the page template in question. But for the sake of providing an MWE, I stripped it down to something that simply returns a fixed text whenever something is to be translated. You can find it at [https://github.com/perfact/Products.PerFactTranslationDomain/tree/dummy-translator.](url).
If using a page template like
<p i18n:translate="">original text</p>
This correctly replaces the content of the `p` tag by `translated`.
However, sometimes it also replaces the content of
<p tal:content="string:original text">something</p>
and the same with `tal:replace`. By trying to nail down the problem, I found that this happens if the Chameleon cache file for the given page template has been created by the currently running Zope instance. So if I create a new page template, the behavior can be observed. If I restart Zope, it is gone. If I stop Zope, clear the Chameleon cache and start it again, it can again be observed.
Interesting behavior.
It is a feature that Zope (in this case actually `chameleon`)
sometimes translates things without an explicit `i18n` directive:
e.g. for `i18n.messageid`s. This is good and must not be abandonned.
This implies that expressions cannot be coerced to text - it would
remove the "messageid" character and we would lose automatic translation.
In your example, there is no "i18n.messageid" and no "i18n" directive;
thus, nothing should get translated. This looks like a `chameleon` bug,
problably usually hidden by the fact that without a translation the original
text is used.
I have no idea how this could be related to `chameleon` caching.
Trying to debug the issue, I found myself inside the Chameleon cache file in a code block like
def __quote(target, quote, quote_entity, default, default_marker):
__tt = type(target)
if ((__tt is int) or (__tt is float) or (__tt is int)):
target = str(target)
else:
if (__tt is bytes):
target = decode(target)
else:
if (__tt is not str):
try:
target = target.__html__
except:
__converted = convert(target)
target = (str(target) if (target is __converted) else __converted)
else:
return target()
if (target is not None):
...
If we are in the restarted run (where `tal:replace` does not get translated), `target` is of type `str` (with content `original text`) and we land in the last branch of the evaluation, where the string is being escaped.
However, if the Chameleon cache was cleared before starting Zope, `target` is of type `<class 'chameleon.tokenize.Token'>`. Since such a `Token` does not have the `__html__` attribute, `convert(target)` is called, which somehow ends up calling the translation service.
`chameleon.tokenize.Token` is a class related to `chameleon` parsing
- naturally, it is absent when the template came from the cache
(as in this case, there is no `chameleon` parsing).
Your observations let me think that (for efficiency reasons)
`chameleon` does not use the byte code (it puts into the cache)
when it has freshly created it - but some different representation
which still contains parsing data (such as `Token`s).
From what you say, I get convinced that this is a `chameleon` bug
and should be fixed there.
I suggest, you try to reproduce the problem with stock `chameleon`.
Of course, you cannot use there a "Script (Python)", but `chameleon`
allows to pass in the `translate` function as parameter. Use
a function that returns a constant value.
There is one thing most probably broken here and one thing that I maybe simply am not using correctly: One is that the type of `target` changes depending on if there was already a Chameleon cache present when Zope was started, leading to different behaviors.
Definitely a bug (IMO) - probably due to aggressive optimation.
The second is that the `econtext` given to the `render` function contains the fields
{'__convert': functools.partial(<function PageTemplate.render.<locals>.translate at 0x7fb5b35e5af0>, target_language=None),
'__decode': <function PageTemplate.render.<locals>.decode at 0x7fb5b35e5ca0>,
'__on_error_handler': None,
'__translate': <function PageTemplate.render.<locals>.translate at 0x7fb5b35e5af0>,
...
Not sure if `convert` should point to a `translate` function if there is a specific `__translate` field also which seems to be the correct one for calling tags with `i18n:translate`.
I do not think that the behavior is related to a wrong `__convert`.
|
|
Dieter Maurer wrote at 2020-7-20 12:06 +0200:
Viktor Dick wrote at 2020-7-20 01:34 -0700:
> ...
>We use a translation service that effectively delegates translation to a Python Script in the context of the page template in question. But for the sake of providing an MWE, I stripped it down to something that simply returns a fixed text whenever something is to be translated. You can find it at [https://github.com/perfact/Products.PerFactTranslationDomain/tree/dummy-translator.](url).
...
>However, sometimes it also replaces the content of
>
> <p tal:content="string:original text">something</p>
I have tried to reproduce this with base `chameleon`:
```
from chameleon.zpt.template import PageTemplate
def translate(*args, **kw): return "translated"
ts = """<div tal:content="string:x" />"""
t = PageTemplate(ts)
t(translate=translate)
```
But, `chameleon` works as it should.
A `Products.PageTemplate` template works as well:
```
from Products.PageTemplates.PageTemplate import PageTemplate as ZPT
zpt = ZPT()
zpt.write(ts)
zpt(translate=translate)
```
In both cases, the result is `<div>x</div>` (as it should be).
Could you privide a minimal (Zope) template which reproduces the problem?
|
|
I also can not reproduce it with chameleon itself (I have come up with essentially the same example). I described the minimal example within Zope in the original post. Maybe we are doing something wrong with our translation product - that code has been untouched for years and I don't have contact to the original author. I somehow made it work under Zope 4, but for example I do not understand why we are providing an |
|
Viktor Dick wrote at 2020-7-20 13:01 -0700:
I also can not reproduce it with chameleon itself (I have come up with essentially the same example).
I described the minimal example within Zope in the original post. Maybe we are doing something wrong with our translation product - that code has been untouched for years and I don't have contact to the original author.
Do not worry.
`chameleon` is responsible to call or not call `translate`.
If it does not call it, translate can do whatever is wants.
In the code you have shown, `chameleon` should not call `translate`
but apparently it does.
What we need is an example which can reproduce the bad `translate/convert`
call. This will be independent of your translation service.
|
|
Viktor Dick wrote at 2020-7-20 13:01 -0700:
I also can not reproduce it with chameleon itself (I have come up with essentially the same example).
I described the minimal example within Zope in the original post.
...
I have meanwhile read @malthe 's comment on
"malthe/chameleon#326"
and I have to admit that he might be right.
In this case, however, I do not yet understand why I cannot (yet)
reproduce the problem with a `Zope` template.
I will investigate and keep you informed.
|
|
I asked @malthe for help to reproduce the problem ("malthe/chameleon#326 (comment)"). Apparently, even for the initial compilation, |
|
The problem occurs only with Python 3. A small problem reproducing example for Python 3 is This results in |
|
@d-maurer I created a Zope installation from your branch as described in my original post. The problem is the same. I put a breakpoint into my translation function and this is the call stack: I will try to further narrow down the problem, but maybe this helps. |
|
@d-maurer OK I am sorry. Somehow I failed to actually install the correct branch - partly due to the Anyhow, if I insert your changes directly, it works! |
|
I have to do some more tests. With a minimal setup it seems that the problem originally posted is now gone. However, I have another example that still seems to be broken (an HTML fragment that is inserted using |
|
The problem that I am still seeing is that if I have a file named the translation is called with the The problem is similar to the original one because here also inside the chameleon cache file the passed target has no So maybe it is not the same problem. I can work around it by using |
|
Let me add to that comment: Even though I can work around it, there is no The with |
|
Viktor Dick wrote at 2020-7-22 08:18 -0700:
...
Even though I can work around it, there is no `i18n` tag and (as far as I know, but I have to get more acquainted with that) no `i18n.messageid`, so the original assessment holds here also: no translation should be called. But the file created by `chameleon` seems to follow the logic: If the message is not a string and has no `__html__` attribute, it should be passed through `convert`,
Many "tal commands" add something to the output; this something
must be some kind of text (corresponding to the output type).
If the initial value is not itself text, it must be converted.
Thus, it is correct to call `convert`.
which happens to point to the `translate` service.
That is likely a bug: `chameleon` should not suppose
that anything which is not already text should be translated.
The `target` is constructed by
__cache_139679322444512 = _static_139679340286928('path', 'here/file_containing_content', econtext=econtext)(_static_139679340286736(econtext, __zt_tmp))
with
_static_139679340286928 == <function _compile_zt_expr at 0x...>
_static_139679340286736 == <function _C2ZContextWrapper at 0x...>
_static_139679340286736(econtext, __zt_tmp) == <Products.PageTemplates.engine._with_vars_from_chameleon.<locals>.ContextWrapper object at 0x...>
__cache_139679322444512 == <File at /file_containing_content>
This will evaluate "here/file_containing_content" and return
the file object.
It is not responsible for the decision to "convert" nor to translate.
As explained above, it is correct to call `convert`; it is wrong
that `convert` entails "translation", because translation is
controlled via either "i18n commands" or by the type of the
converted object (e.g. when it is a `zope.i18nmessageid.Message` object).
The following transscript shows that this is a `chameleon` bug:
```
>> class ToConvertButNotTranslate:
... def __init__(self, text): self.text = text
... def __str__(self): return self.text
...
>> x = ToConvertButNotTranslate("x")
>>
>> from chameleon.zpt.template import PageTemplate
>>
>> def translate(*args, **kw): return "translated"
...
>> ts = """<div tal:content="x" />"""
>> t = PageTemplate(ts)
>> t(translate=translate, x=x)
'<div>translated</div>'
```
|
|
I think I have described what's happening rather well in my comment in the Chameleon issue and given an easy workaround. Not sure what else is there to do? This is not a Chameleon issue. |
|
Malthe Borch wrote at 2020-7-22 10:58 -0700:
I think I have described what's happening rather well in my comment in the Chameleon issue and given an easy workaround. Not sure what else is there to do? This is not a Chameleon issue.
@malthe
In my previous comment I have shown a transscript
involving stock `chameleon` only which demonstrates a
case of unsolicited translation.
`chameleon` must not associate translation with the need for
a conversion: both things are independent from one another.
|
|
Malthe Borch wrote at 2020-7-22 10:58 -0700:
I think I have described what's happening rather well in my comment in the Chameleon issue and given an easy workaround. Not sure what else is there to do? This is not a Chameleon issue.
@malthe I have created "malthe/chameleon#328".
|
|
Viktor Dick wrote at 2020-7-22 08:18 -0700:
Let me add to that comment: Even though I can work around it, there is no `i18n` tag and (as far as I know, but I have to get more acquainted with that) no `i18n.messageid`, so the original assessment holds here also: no translation should be called. But the file created by `chameleon` seems to follow the logic: If the message is not a string and has no `__html__` attribute, it should be passed through `convert`, which happens to point to the `translate` service.
@viktordick
Following @malthe 's comment I have read the `chameleon`
documentation: "https://chameleon.readthedocs.io/en/latest/reference.html?highlight=translation#translation-i18n".
According to this documentation, `chameleon` has an extremely wide
notion for "i18n message": anything which is not a string, nor
a number nor has a `__html__` method.
This means that the translation function must be ready to handle
any such object (which might be used in connection with a template).
This behavior differs considerably from that of `zope.pagetemplate`:
for this, only instance of `zope.i18nmessageid.Message` are
considered "i18n message"s and get translated automatically.
Thus there are only 2 options for you:
continue to use `zope.pagetemplate` as template engine
or ensure your translation function is sufficiently flexible
to handle everything `chameleon` treats as "i18n message".
|
|
@d-maurer Thank you very much. I guess this simply means that our translation product should return any object that it is not ready to translate (probably anything that is not a string). This seems to work! I hope this is the last surprise with this. I do not know where this difference between |
|
Viktor Dick wrote at 2020-7-22 12:07 -0700:
...
I guess this simply means that our translation product should return any object that it is not ready to translate (probably anything that is not a string).
Do not forget to handle (i.e. translate) `zope.i18nmessageid.Message`
appropriately.
|
I think we do not use that, but I will keep it in mind. Thanks. |
|
Fixed in 4.x via #878 |
|
Note that The fix in #878 cannot change this; it only prevents that an internal |
|
I found one more problem, although it is a corner case that we probably do not actually use. I mentioned above how using However, now the case also does not get translated. In fact, the chameleon cache file contains the lines According to the observations above, this will pass the object twice through the translation, but it will in both cases be rejected and the Returning This only occurs with I also reported this in chameleon#329. So in summary, Chameleon has a different notion about what needs translating and while an explicit By the way, in a comment above I said that the PRs related to this might not be needed any more because the translation engine anyhow has to reject messages it does not expect. However, upon closer inspection this is not correct. In particular it solves the deviating behavior between a run that creates the cache file and one that uses it. Without these changes, a similar problem like the one I still observe with @d-maurer Unless you think this case should also be fixed somehow, I guess we could close this issue. |
|
Viktor Dick wrote at 2020-7-25 05:07 -0700:
I found one more problem, although it is a corner case that we probably do not actually use. I mentioned above how using `tal:content="here/somefile"` with a `File` object will also trigger the translation, so we changed our translation so it rejects any messages that fall under Chameleon's definition of translatable message but not `zope.pagetemplate`'s.
However, now the case
<div i18n:translate="" tal:content="here/somefile"></div>
also does not get translated. In fact, the chameleon cache file contains the lines
__content = translate(__content, default=None, domain=__i18n_domain, context=__i18n_context, target_language=getitem('target_language'))
__content = __quote(__content, None, '\xad', None, None)
According to the observations above, this will pass the object twice through the translation, but it will in both cases be rejected and the `__quote` call will finally transform the message into a string using `str` because the translation returned the object itself.
Unfortunately, inside the `translate` function I have no way of knowing if there is an actual `i18n:translate` attribute on the tag and we are called from the explicit `translate` or if we are called from `__quote` because the object has no `__html__` method. Short of inspecting the call stack that is, but I do not think that would be a good idea.
There is nothing `Zope` could do for you regarding this case:
calling the translation function is completely under the responsibility
of the template engine, i.e. either `zope.pagetemplate` or
`chameleon`.
But I have a simple workaround: instead of
`here/somefile` use `python: str(here.somefile)` when you want
that `i18n:translate` matters.
Note: you can use `zope.pagetemplate` with `Zope 4` (even if this
is not documented explicitly)
@d-maurer Unless you think this case should also be fixed somehow, I guess we could close this issue.
It cannot be fixed by `Zope` (which sees only expressions and not
what they are used for - e.g. for a `tal:content` or in a `tal:define`).
If the template engine handles the case correctly, then it will
work with Zope as well -- unless `chameleon` solves the problem
by introducing further configuration options.
Likely, `chameleon` has this wide notion of "i18n message"
because its aim is not to support `Zope` but all
web application frameworks implemented in Python, likely
each of them with their own notion of "i18n message".
A way to solve the issue you are reporting would be to
allow the `chameleon` integration to inform `chameleon` what
"i18n message"s it uses. For Zope, this would be
instances of `zope.i18nmessageid.Message`.
|
|
The translation function should always convert the object to a string, even
a file-like object. I don’t see how this case is different than the other
one we’ve discussed.
…On Sat, 25 Jul 2020 at 14.59, Dieter Maurer ***@***.***> wrote:
Viktor Dick wrote at 2020-7-25 05:07 -0700:
>I found one more problem, although it is a corner case that we probably
do not actually use. I mentioned above how using
`tal:content="here/somefile"` with a `File` object will also trigger the
translation, so we changed our translation so it rejects any messages that
fall under Chameleon's definition of translatable message but not
`zope.pagetemplate`'s.
>
>However, now the case
>
> <div i18n:translate="" tal:content="here/somefile"></div>
>
>also does not get translated. In fact, the chameleon cache file contains
the lines
>
> __content = translate(__content, default=None, domain=__i18n_domain,
context=__i18n_context, target_language=getitem('target_language'))
> __content = __quote(__content, None, '\xad', None, None)
>
>According to the observations above, this will pass the object twice
through the translation, but it will in both cases be rejected and the
`__quote` call will finally transform the message into a string using `str`
because the translation returned the object itself.
>Unfortunately, inside the `translate` function I have no way of knowing
if there is an actual `i18n:translate` attribute on the tag and we are
called from the explicit `translate` or if we are called from `__quote`
because the object has no `__html__` method. Short of inspecting the call
stack that is, but I do not think that would be a good idea.
There is nothing `Zope` could do for you regarding this case:
calling the translation function is completely under the responsibility
of the template engine, i.e. either `zope.pagetemplate` or
`chameleon`.
But I have a simple workaround: instead of
`here/somefile` use `python: str(here.somefile)` when you want
that `i18n:translate` matters.
Note: you can use `zope.pagetemplate` with `Zope 4` (even if this
is not documented explicitly)
***@***.*** Unless you think this case should also be fixed somehow, I
guess we could close this issue.
It cannot be fixed by `Zope` (which sees only expressions and not
what they are used for - e.g. for a `tal:content` or in a `tal:define`).
If the template engine handles the case correctly, then it will
work with Zope as well -- unless `chameleon` solves the problem
by introducing further configuration options.
Likely, `chameleon` has this wide notion of "i18n message"
because its aim is not to support `Zope` but all
web application frameworks implemented in Python, likely
each of them with their own notion of "i18n message".
A way to solve the issue you are reporting would be to
allow the `chameleon` integration to inform `chameleon` what
"i18n message"s it uses. For Zope, this would be
instances of `zope.i18nmessageid.Message`.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#876 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAGOJJRRVVDLUCR2OAONVDR5LJLJANCNFSM4PCA7YVQ>
.
|
|
Malthe Borch wrote at 2020-7-25 07:09 -0700:
The translation function should always convert the object to a string, even
a file-like object. I don’t see how this case is different than the other
one we’ve discussed.
Sometimes, you want a file object to be translated and sometimes
you do not.
This is not problem with `zope.pagetemplate: in the first case,
you use `i18n:translate`, otherwise, you do not.
This is a problem with `chameleon` because it treats almost
everything as "to be translated automatically".
|
|
But turning it into a string does not “translate“ it. It will only be
translated in the second step, caused by i18n:translate at which point it
is no longer a file, but a string.
…On Sat, 25 Jul 2020 at 18.17, Dieter Maurer ***@***.***> wrote:
Malthe Borch wrote at 2020-7-25 07:09 -0700:
>The translation function should always convert the object to a string,
even
>a file-like object. I don’t see how this case is different than the other
>one we’ve discussed.
Sometimes, you want a file object to be translated and sometimes
you do not.
This is not problem with `zope.pagetemplate: in the first case,
you use `i18n:translate`, otherwise, you do not.
This is a problem with `chameleon` because it treats almost
everything as "to be translated automatically".
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#876 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAGOJJ6US6WTNBL2RKKI4DR5MAQBANCNFSM4PCA7YVQ>
.
|
So the conclusion seems to be that chameleon expects the input to be a string in these cases. This can be achieved by either using The only way to cover this case also would be to allow a configuration of Chameleon that causes it to act like I will close this issue now because the only think that could still be fixed would have to be fixed in Chameleon. |
|
In the case where you have a file-object and
Why does this not result in a double translation? Because in (1) we either convert to a string or translate (a string or a message), never both. But it's up to the translate function to implement this logic. |
If I implement the translation function as either converting to string or translating, there is no double translation. But there is no single translation either, even if there is an Maybe the order should be different, but I guess the primary function of we expect the translated content to be quoted, or the quoted content to be translated. I think I would expect the translated content to be quoted. |
|
As a side note, the original problem that caused me to investigate and post this issue was a where the |
|
Malthe Borch wrote at 2020-7-25 13:58 -0700:
But turning it into a string does not “translate“ it. It will only be
translated in the second step, caused by i18n:translate at which point it
is no longer a file, but a string.
Turning it into a string explicitly is a workaround -- but a nasty one:
instead of e.g `tal:content="here/some_file"` you must use
`tal:content="python: str(here.some_file)"` -- just to avoid
the unsolicited translation.
|
|
I think I will take back some of what I have suggested after having looked more at this issue. However, here's an idea: Typically, an That is, for a file object, you should be able to tell whether or not a translation is being requested based on whether or not a translation domain is provided. |
I originally posted this in malthe/chameleon#326 but it was closed there as
wontfixwith the hintWe have seen some weird behavior when switching to Zope4 that seems to be related to the Chameleon cache.
We use a translation service that effectively delegates translation to a Python Script in the context of the page template in question. But for the sake of providing an MWE, I stripped it down to something that simply returns a fixed text whenever something is to be translated. You can find it here.
If using a page template like
This correctly replaces the content of the
ptag bytranslated.However, sometimes it also replaces the content of
and the same with
tal:replace. By trying to nail down the problem, I found that this happens if the Chameleon cache file for the given page template has been created by the currently running Zope instance. So if I create a new page template, the behavior can be observed. If I restart Zope, it is gone. If I stop Zope, clear the Chameleon cache and start it again, it can again be observed.Trying to debug the issue, I found myself inside the Chameleon cache file in a code block like
If we are in the restarted run (where
tal:replacedoes not get translated),targetis of typestr(with contentoriginal text) and we land in the last branch of the evaluation, where the string is being escaped.However, if the Chameleon cache was cleared before starting Zope,
targetis of type<class 'chameleon.tokenize.Token'>. Since such aTokendoes not have the__html__attribute,convert(target)is called, which somehow ends up calling the translation service.There is one thing most probably broken here and one thing that I maybe simply am not using correctly: One is that the type of
targetchanges depending on if there was already a Chameleon cache present when Zope was started, leading to different behaviors. The second is that theecontextgiven to therenderfunction contains the fieldsNot sure if
convertshould point to atranslatefunction if there is a specific__translatefield also which seems to be the correct one for calling tags withi18n:translate.Steps to reproduce
Open a browser at
http://localhost:8080/manage_mainand provide the credentialsadmin:admin. Now add a page templatetestptwith the contentand let it render (
http://localhost:8080/testpt)Expected behavior
We should obtain a paragraph with the content
replaced text.Actual behavior
On the first try, we obtain a paragraph with the content
translated. If restarting Zope, we again obtain the expected behavior. If removing everything inzope/var/cache/and restarting Zope, we again obtaintranslated.Versions
As seen above, I took the newly released Zope 4.5 as starting point. It contains Chameleon 3.8.1
The text was updated successfully, but these errors were encountered: