Opened 5 years ago

Closed 4 years ago

#12104 closed New feature (fixed)

Object->object lookup fails in blocktrans

Reported by: philipn Owned by: nobody
Component: Internationalization Version: 1.1
Severity: Normal Keywords:
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Lookups such as {{O.a}} fail in blocktrans blocks.

Here is a complete example:

{% load i18n %}

<html>
<body>
This causes a keyerror:

{% blocktrans %}
Value 'a' of object O: {{ O.a }}
{% endblocktrans %}

The following, of course, works:

Value 'a' of object O: {{ O.a }}
</body>
</html>

With a view being:

def blocktrans(request):
    O = {'a': 'Foo!'}
    d = {
        'O': O,
    }
    return render_to_response('home.html', d)

I have attached a patch.

Attachments (3)

django_svn_blocktrans_patch.diff (1.0 KB) - added by philipn 5 years ago.
blocktrans_display_error.diff (1.5 KB) - added by philipn 5 years ago.
displays templateerror instead of keyerror
blocktrans_variable_attribute_works.diff (1.1 KB) - added by philipn 5 years ago.
normal-like variable reference in blocktrans

Download all attachments as: .zip

Change History (8)

Changed 5 years ago by philipn

comment:1 Changed 5 years ago by carljm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to invalid
  • Status changed from new to closed

I believe this behavior is intentional (perhaps to keep substitution strings in translation messages simple?), as it is clearly documented. You need to use "with O.a as O_a" in your blocktrans opening tag.

comment:2 Changed 5 years ago by philipn

  • Resolution invalid deleted
  • Status changed from closed to reopened

If this is to keep translations simple then that makes sense, especially considering blocktrans doesn't allow for other blocks inside it.

Adding support for other block tags inside of blocktrans seems easy enough to add, though (using the same idea as my patch -- executing a template rendering).

But, unlike other blocks in blocktrans, this kind of variable access doesn't trigger an error message.

I've attached a patch that causes an error to be displayed when you try and use complex variable lookups inside a blocktrans block.

If we did decide to allow for variable.attr syntax then it would probably be better to use django.template's Variable class to look up the variable's values rather than the context-dictionary-key approach used in the code currently.

I guess it's also worth noting that the current blocktrans variable lookup behaviour will throw a keyerror for {{ variable_that_isnt_in_context }} when templates normally do not. With this in mind it really seems like the right thing to do is to replace the lookup behaviour with Variable(..).resolve(). I've attached a patch that does this, as well.

Changed 5 years ago by philipn

displays templateerror instead of keyerror

Changed 5 years ago by philipn

normal-like variable reference in blocktrans

comment:3 Changed 5 years ago by ramiro

  • Triage Stage changed from Unreviewed to Design decision needed

comment:4 Changed 4 years ago by julien

  • Severity set to Normal
  • Type set to New feature

comment:5 Changed 4 years ago by julien

  • Easy pickings unset
  • Resolution set to fixed
  • Status changed from reopened to closed
  • UI/UX unset

This is essentially fixed since KeyError isn't thrown any more after [16723].

Besides, I don't think that {{ variable.attribute }} lookups should be allowed, especially since blocktrans already offers ways of creating simple temporary variables and it's all well documented. One can always use {{ foo.bar }} inside blocktrans but that will count as a flat variable name called "foo.bar", which should work as far as gettext itself is concerned.

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