Django

Code

Ticket #4539 (closed: fixed)

Opened 1 year ago

Last modified 6 months ago

translation.ugettext loses variable content from blocktrans

Reported by: permonik@mesias.brnonet.cz Assigned to: permon
Milestone: Component: Internationalization
Version: SVN Keywords: blocktrans sprintsept14
Cc: permonik@mesias.brnonet.cz Triage Stage: Ready for checkin
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

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

testing.html (300 bytes) - added by permonik@mesias.brnonet.cz on 06/11/07 15:28:13.
Template used in example
views.py (257 bytes) - added by permonik@mesias.brnonet.cz on 06/11/07 15:28:50.
Sample view
patch.diff (0.6 kB) - added by permon on 09/11/07 17:10:36.
patch
patch.2.diff (0.8 kB) - added by permon on 09/11/07 17:23:30.
patch - updated

Change History

06/11/07 15:28:13 changed by permonik@mesias.brnonet.cz

  • attachment testing.html added.

Template used in example

06/11/07 15:28:50 changed by permonik@mesias.brnonet.cz

  • attachment views.py added.

Sample view

06/15/07 04:40:12 changed by Simon G. <dev@simon.net.nz>

  • needs_better_patch changed.
  • stage changed from Unreviewed to Accepted.
  • needs_tests changed.
  • needs_docs changed.

(follow-up: ↓ 3 ) 07/21/07 10:15:04 changed by Simon G. <dev@simon.net.nz>

  • summary changed from blocktrans 'losing' variable content to translation.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

09/11/07 17:10:36 changed by permon

  • attachment patch.diff added.

patch

09/11/07 17:23:30 changed by permon

  • attachment patch.2.diff added.

patch - updated

(in reply to: ↑ 2 ) 09/11/07 17:28:51 changed by permonik@mesias.brnonet.cz

  • has_patch set to 1.

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

(follow-up: ↓ 5 ) 09/11/07 17:52:15 changed by 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.

(in reply to: ↑ 4 ; follow-up: ↓ 6 ) 09/12/07 04:00:25 changed by permonik@mesias.brnonet.cz

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.

(in reply to: ↑ 5 ) 09/12/07 04:04:40 changed 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 ;-)

09/14/07 09:57:11 changed by permon

  • owner changed from nobody to permon.
  • status changed from new to assigned.

09/14/07 14:45:47 changed by permon

  • keywords changed from blocktrans to blocktrans sprintsept14.

01/04/08 15:40:37 changed by permon

  • stage changed from Accepted to Ready for checkin.

03/17/08 09:55:31 changed by mtredinnick

  • status changed from assigned to closed.
  • resolution set to fixed.

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

Add/Change #4539 (translation.ugettext loses variable content from blocktrans)




Change Properties
Action