Opened 9 years ago

Closed 9 years ago

#4539 closed (fixed)

translation.ugettext loses variable content from blocktrans

Reported by: permonik@… Owned by: Tomáš Kopeček
Component: Internationalization Version: master
Severity: Keywords: blocktrans sprintsept14
Cc: permonik@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Look at the following template:

{% load i18n %}
{% for spell in spells %}
    {% blocktrans with spell.spell as spell and spell.wave as wave and spell.target as target and spell.caster as caster and spell.count as count%}
        '{{spell}}', '{{wave}}', '{{target}}', '{{caster}}', '{{count}}'
    {% endblocktrans %}
{% endfor %}

Imagine corresponding correct input list of dictionaries (spells). Output looks like this:

'spell', '', '', 'caster', 'count'

So, variables target and wave gets lost. This behaviour heavily depends on other variables, e.g. blocktrans with only wave and target works fine.

Behaviour could be seen in actual SVN version (rev. 5465). Simplest view and template are attached. Feel free to ask any questions, I'll be monitoring this ticket.

Attachments (4)

testing.html (300 bytes) - added by permonik@… 9 years ago.
Template used in example
views.py (257 bytes) - added by permonik@… 9 years ago.
Sample view
patch.diff (639 bytes) - added by Tomáš Kopeček 9 years ago.
patch
patch.2.diff (798 bytes) - added by Tomáš Kopeček 9 years ago.
patch - updated

Download all attachments as: .zip

Change History (14)

Changed 9 years ago by permonik@…

Attachment: testing.html added

Template used in example

Changed 9 years ago by permonik@…

Attachment: views.py added

Sample view

comment:1 Changed 9 years ago by Simon G. <dev@…>

Triage Stage: UnreviewedAccepted

comment:2 Changed 9 years ago by Simon G. <dev@…>

Summary: blocktrans 'losing' variable contenttranslation.ugettext loses variable content from blocktrans

I can confirm this behavior on r5724. It appears that the values are getting lost when going through translation.ugettext in BlockTranslateNode.render

Changed 9 years ago by Tomáš Kopeček

Attachment: patch.diff added

patch

Changed 9 years ago by Tomáš Kopeček

Attachment: patch.2.diff added

patch - updated

comment:3 in reply to:  2 Changed 9 years ago by permonik@…

Has patch: set

Replying to Simon G. <dev@simon.net.nz>:

I can confirm this behavior on r5724. It appears that the values are getting lost when going through translation.ugettext in BlockTranslateNode.render

Problem solved. Context was updated during variable resolving. So, in rare cases, could occur resolving from bad scope. Small patch adds temporary dictionary, which is after successful resolving put to context as inner scope. Please, check, that I understand well to how Context.update() works. It looks, that it doesn't update current scope but adds new one. So I've worked with it in this way.
Patch is against r6098

comment:4 Changed 9 years ago by Malcolm Tredinnick

This doesn't look like the right solution. context.push() doesn't clean out the context; it only adds a marker, in effect, so that we can remove (pop) any new content added since that point. The existing content is still accessible.

So this patch doesn't look like it's getting to the heart of the problem, which is why the variables are resolving to blank in the first place if they exist in the context.

comment:5 in reply to:  4 ; Changed 9 years ago by permonik@…

Replying to mtredinnick:

This doesn't look like the right solution. context.push() doesn't clean out the context; it only adds a marker, in effect, so that we can remove (pop) any new content added since that point. The existing content is still accessible.

So this patch doesn't look like it's getting to the heart of the problem, which is why the variables are resolving to blank in the first place if they exist in the context.

Look at the example:
first is created context for loop:
[{spell: {spell: 'spell', wave: 'wave', target: 'target', caster: 'caster' count: 'count'}}
then we move into blocktrans and try to resolve variable:

  • spell.count is resolved as count and pushed to the context
  • spell.caster is resolved as caster and pushed to the context
  • spell.spell is resolved as 'spell' and pushed to the context
  • this is crucial point - from now spell.* takes resolution from wrong (new) context level, so:
  • spell.target is wrong (spell.spell.target doesn't exist) and results to
  • spell.wave is wrong (spell.spell.wave doesn't exist) and results to

context corresponding to these step is as follows:

  • [{u'count': u'count'}, {'forloop': {'parentloop': {}, 'last': True, 'counter': 1, 'revcounter0': 0, 'revcounter': 1, 'counter0': 0, 'first': True}, u'spell': {'count': 'count', 'caster': 'caster', 'spell': 'spell', 'target': 'target', 'wave': 'wave'}}, {'spells': [{'count': 'count', 'caster': 'caster', 'spell': 'spell', 'target': 'target', 'wave': 'wave'}]}]
  • [{u'count': u'count', u'caster': u'caster'}, {'forloop': {'parentloop': {}, 'last': True, 'counter': 1, 'revcounter0': 0, 'revcounter': 1, 'counter0': 0, 'first': True}, u'spell': {'count': 'count', 'caster': 'caster', 'spell': 'spell', 'target': 'target', 'wave': 'wave'}}, {'spells': [{'count': 'count', 'caster': 'caster', 'spell': 'spell', 'target': 'target', 'wave': 'wave'}]}]
  • [{u'count': u'count', u'caster': u'caster', u'spell': u'spell'}, {'forloop': {'parentloop': {}, 'last': True, 'counter': 1, 'revcounter0': 0, 'revcounter': 1, 'counter0': 0, 'first': True}, u'spell': {'count': 'count', 'caster': 'caster', 'spell': 'spell', 'target': 'target', 'wave': 'wave'}}, {'spells': [{'count': 'count', 'caster': 'caster', 'spell': 'spell', 'target': 'target', 'wave': 'wave'}]}]
  • [{u'count': u'count', u'caster': u'caster', u'spell': u'spell', u'target': }, {'forloop': {'parentloop': {}, 'last': True, 'counter': 1, 'revcounter0': 0, 'revcounter': 1, 'counter0': 0, 'first': True}, u'spell': {'count': 'count', 'caster': 'caster', 'spell': 'spell', 'target': 'target', 'wave': 'wave'}}, {'spells': [{'count': 'count', 'caster': 'caster', 'spell': 'spell', 'target': 'target', 'wave': 'wave'}]}]
  • [{u'count': u'count', u'caster': u'caster', u'spell': u'spell', u'target': , u'wave': }, {'forloop': {'parentloop': {}, 'last': True, 'counter': 1, 'revcounter0': 0, 'revcounter': 1, 'counter0': 0, 'first': True}, u'spell': {'count': 'count', 'caster': 'caster', 'spell': 'spell', 'target': 'target', 'wave': 'wave'}}, {'spells': [{'count': 'count', 'caster': 'caster', 'spell': 'spell', 'target': 'target', 'wave': 'wave'}]}]

If context[0] doesn't exist during computation of new context level, everything goes ok. So I still think, that my patch is correct solution. We cannot resolve variable from context which is unde generation. We have to use upper levels (so context[1] and upper levels). So I still think, that my patch is correct solution.

comment:6 in reply to:  5 Changed 9 years ago by anonymous

It would be better seen as code:

[{u'count': u'count'}, {'forloop': {'parentloop': {}, 'last': True, 'counter': 1, 'revcounter0': 0, 'revcounter': 1, 'counter0': 0, 'first': True}, u'spell': {'count': 'count', 'caster': 'caster', 'spell': 'spell', 'target': 'target', 'wave': 'wave'}}, {'spells': [{'count': 'count', 'caster': 'caster', 'spell': 'spell', 'target': 'target', 'wave': 'wave'}]}]
[{u'count': u'count', u'caster': u'caster'}, {'forloop': {'parentloop': {}, 'last': True, 'counter': 1, 'revcounter0': 0, 'revcounter': 1, 'counter0': 0, 'first': True}, u'spell': {'count': 'count', 'caster': 'caster', 'spell': 'spell', 'target': 'target', 'wave': 'wave'}}, {'spells': [{'count': 'count', 'caster': 'caster', 'spell': 'spell', 'target': 'target', 'wave': 'wave'}]}]
[{u'count': u'count', u'caster': u'caster', u'spell': u'spell'}, {'forloop': {'parentloop': {}, 'last': True, 'counter': 1, 'revcounter0': 0, 'revcounter': 1, 'counter0': 0, 'first': True}, u'spell': {'count': 'count', 'caster': 'caster', 'spell': 'spell', 'target': 'target', 'wave': 'wave'}}, {'spells': [{'count': 'count', 'caster': 'caster', 'spell': 'spell', 'target': 'target', 'wave': 'wave'}]}]
[{u'count': u'count', u'caster': u'caster', u'spell': u'spell', u'target': ''}, {'forloop': {'parentloop': {}, 'last': True, 'counter': 1, 'revcounter0': 0, 'revcounter': 1, 'counter0': 0, 'first': True}, u'spell': {'count': 'count', 'caster': 'caster', 'spell': 'spell', 'target': 'target', 'wave': 'wave'}}, {'spells': [{'count': 'count', 'caster': 'caster', 'spell': 'spell', 'target': 'target', 'wave': 'wave'}]}]
[{u'count': u'count', u'caster': u'caster', u'spell': u'spell', u'target': '', u'wave': ''}, {'forloop': {'parentloop': {}, 'last': True, 'counter': 1, 'revcounter0': 0, 'revcounter': 1, 'counter0': 0, 'first': True}, u'spell': {'count': 'count', 'caster': 'caster', 'spell': 'spell', 'target': 'target', 'wave': 'wave'}}, {'spells': [{'count': 'count', 'caster': 'caster', 'spell': 'spell', 'target': 'target', 'wave': 'wave'}]}]

P.S. I look at the last sentence and it looks little bit crude. I didn't think it, only cut & paste in wrong place ;-)

comment:7 Changed 9 years ago by Tomáš Kopeček

Owner: changed from nobody to Tomáš Kopeček
Status: newassigned

comment:8 Changed 9 years ago by Tomáš Kopeček

Keywords: sprintsept14 added

comment:9 Changed 9 years ago by Tomáš Kopeček

Triage Stage: AcceptedReady for checkin

comment:10 Changed 9 years ago by Malcolm Tredinnick

Resolution: fixed
Status: assignedclosed

(In [7261]) Fixed #4539 -- Fixed a subtle context resolving bug in the i18n template tag.
Excellent debugging from permonik@….

Note: See TracTickets for help on using tickets.
Back to Top