Opened 17 years ago

Closed 17 years ago

Last modified 17 years ago

#4292 closed (fixed)

[unicode] django.utils.functional.lazy can't decorate functions with unicode results

Reported by: Ivan Sagalaev <Maniac@…> Owned by: Malcolm Tredinnick
Component: Translations Version: other branch
Severity: Keywords: unicode
Cc: Maniac@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Currently the admin page showing, say, a user object is broken for Russian translation. I've traced the reason -- a leaky abstraction of a lazy proxy object.

The proxy object created by 'lazy' should pretend to be an object of type returned by a decorated function. And this should work:

ugettext = lazy(ugettext, unicode)
s = ugettext(...) # a proxy object pretending to be a unicode instance
s = unicode(s)

But it doesn't work because unicode(s) checks:

  1. if s is an instance of unicode (it isn't)
  2. if s has __unicode__ (it doesn't)
  3. if s has __str___, which it does (! unicode object itself has it)

Then it calls __str__ on unicode object and it breaks for non-ascii characters.

I've created a fix but it seems a bit fragile because it creates a special case for unicode objects in lazy proxies.

P.S. The patch also contains two str -> smart_unicode conversions that don't strictly belong here but they are required for the testcase to work.

Attachments (1)

4292.diff (2.5 KB ) - added by Ivan Sagalaev <Maniac@…> 17 years ago.
Patch

Download all attachments as: .zip

Change History (7)

by Ivan Sagalaev <Maniac@…>, 17 years ago

Attachment: 4292.diff added

Patch

comment:1 by Malcolm Tredinnick, 17 years ago

Cc: Malcolm Tredinnick removed
Triage Stage: UnreviewedAccepted

On one hand, I don't really like that change because it's special-casing one methods out of the infinity of possibilities lazy() pretends to handle. On the other hand, I don't really like the way lazy() works at the moment, so this simple fix is redeemed because it fixes a problem in a reliable fashion.

I was thinking of writing a proper lazy()-like function specifically for things that returned bytestrings and unicodes (i.e. the i18n functions) and was understandable, but this fix will work for now (and maybe forever).

Thanks, Ivan.

comment:2 by Malcolm Tredinnick, 17 years ago

Looking at this change a bit closer, particularly the part in contrib.contenttypes, I'm wondering if that isn't actually a bug: if opts.verbose_name is a ugettext_lazy() result, then shouldn't we be explicitly de-activating any thread-local translation effects so that we always use the same (untranslated) name. Otherwise, there are going to be different results depending on your current locale setting, right?

More importantly, contrib.contenttypes.models needs changing to do the same thing on retrieval (which is when varying locales could really be in effect).

Am I missing anything? Fixing it isn't hard, but I want to check I'm not being stupid.

I only stumbled on this because your patch means we can try to fix smart_unicode() to do the right thing (be lazy!) and it was causing bugs in the Content Type stuff.

comment:3 by Ivan Sagalaev <Maniac@…>, 17 years ago

As I understand verbose_name should be locale dependent since it's primarily used as human-readable title in admin. In that line it is used as a default parameter for get_or_create and hence doesn't take part in querying that goes strictly by app_label and model.

comment:4 by Malcolm Tredinnick, 17 years ago

I've no problem with verbose name being a lazy translation. That's correct.

The problem is that if the get_or_create() method has to create the object (and this happens), the value of the name inserted into the database will depend upon the current locale, which means it's almost randomly chosen from amongst all the possible translations of that name. It should always give the same result, ideally.

comment:5 by Malcolm Tredinnick, 17 years ago

Resolution: fixed
Status: newclosed

(In [5239]) unicode: Fixed #4292 -- Added support for unicode to lazy() objects.
Thanks, Ivan Sagalaev.

comment:6 by Malcolm Tredinnick, 17 years ago

[5240] also contains some of the changes from this patch. I wanted to do all the contentype changes in one hit.

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