[Django] #28304: pgettext should return SafeData if both `message` and `context` are instances of SafeData
Django
instances of SafeData
------------------------------------------------+------------------------
Reporter: Artem Polunin | Owner: nobody
Type: Bug | Status: new
Component: Internationalization | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
`pgettext` always returns `str` even if both `message` and `context` are
instances of `SafeData` (assuming translations exist).
If we have following translations into Ukrainian
{{{
msgid ""
msgstr ""
"Project-Id-Version: django\n"
"Report-Msgid-Bugs-To: \n"
"POT-Creation-Date: 2017-06-09 14:23+0000\n"
"PO-Revision-Date: 2017-06-09 10:22-0400\n"
"Last-Translator: None\n"
"Language-Team: Ukrainian\n"
"Language: uk_UA\n"
"MIME-Version: 1.0\n"
"Content-Type: text/plain; charset=UTF-8\n"
"Content-Transfer-Encoding: 8bit\n"
"Plural-Forms: nplurals=3; plural=(n%10==1 && n%100!=11 ? 0 : n%10>=2 &&
n"
"%10<=4 && (n%100<10 || n%100>=20) ? 1 : 2);\n"
msgid "'"
msgstr "' відсотків"
msgctxt "percent"
msgid "'"
msgstr "' відсотків"
}}}
then
{{{
>>> from django.utils.translation import activate, ugettext, pgettext
>>> from django.utils.safestring import SafeText
>>>
>>> activate('ua')
>>>
>>> type(pgettext(SafeText('percent'), SafeText('''))) # Should
return `SafeText` instance
<class 'str'>
>>>
>>> type(ugettext(SafeText('''))) # This works correctly
<class 'django.utils.safestring.SafeText'>
}}}
This causes additional escape when using `trans` with `context` in
templates:
{{{
>>> from django.utils.translation import activate
>>> from django.template import Context, Template
>>>
>>> activate('ua')
>>>
>>> Template("{% load i18n %}{% trans ''' context 'percent'
%}").render(Context()) # `&` unnecessary escaped into `&`
'&#39; відсотків'
>>>
>>> Template("{% load i18n %}{% trans ''' %}").render(Context()) #
This works correctly
'' відсотків'
}}}
-----
The fix can be as follows:
This line
https://github.com/django/django/blob/master/django/utils/translation/trans_real.py#L325
in `pgettext` can be changed into:
{{{
msg_with_ctxt = "%s%s%s" % (context, CONTEXT_SEPARATOR, message)
if isinstance(context, SafeData) and isinstance(message, SafeData):
msg_with_ctxt = mark_safe(msg_with_ctxt)
}}}
-----
All versions from 1.8 to master are affected.
--
Ticket URL: <https://code.djangoproject.com/ticket/28304>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
instances of SafeData
Reporter: Artem Polunin | Owner: nobody
Type: Bug | Status: new
Component: Internationalization | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* stage: Unreviewed => Accepted
Comment:
Seems okay at first glance, although I'm not a translations expert.
Perhaps it would suffice to mark `CONTEXT_SEPARATOR` as safe?
--
Ticket URL: <https://code.djangoproject.com/ticket/28304#comment:1>
Django
instances of SafeData
Reporter: Artem Polunin | Owner: nobody
Type: Bug | Status: new
Component: Internationalization | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Artem Polunin):
Replying to [comment:1 Tim Graham]:
> Seems okay at first glance, although I'm not a translations expert.
Perhaps it would suffice to mark `CONTEXT_SEPARATOR` as safe?
It won't help, because `"%s" % ...` always evaluates to `str`:
{{{
>>> from django.utils.safestring import SafeText
>>> type("%s%s%s" % (SafeText('context'), SafeText('separator'),
SafeText('message')))
<class 'str'>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28304#comment:2>
Django
instances of SafeData
Reporter: Artem Polunin | Owner: nobody
Type: Bug | Status: new
Component: Internationalization | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Changes (by Claude Paroz):
* easy: 0 => 1
Comment:
I think that simply adding the `if isinstance(message, SafeData): return
mark_safe(result)` like in the main `gettext` call will do it.
--
Ticket URL: <https://code.djangoproject.com/ticket/28304#comment:3>
Django
instances of SafeData
Reporter: Artem Polunin | Owner: nobody
Type: Bug | Status: new
Component: Internationalization | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Comment (by Artem Polunin):
Replying to [comment:3 Claude Paroz]:
> I think that simply adding the `if isinstance(message, SafeData): return
mark_safe(result)` like in the main `gettext` call will do it.
There is already such thing in `gettext`
https://github.com/django/django/blob/master/django/utils/translation/trans_real.py#L318-L319
and it doesn't help, because `gettext` receives `message` as `str` from
`pgettext`.
--
Ticket URL: <https://code.djangoproject.com/ticket/28304#comment:4>
Django
instances of SafeData
Reporter: Artem Polunin | Owner: nobody
Type: Bug | Status: new
Component: Internationalization | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Changes (by Claude Paroz):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/8645 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/28304#comment:5>
Django
instances of SafeData
Reporter: Artem Polunin | Owner: nobody
Type: Bug | Status: new
Internationalization |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/28304#comment:6>
Django
instances of SafeData
Reporter: Artem Polunin | Owner: nobody
Component: | Version: master
Internationalization |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"ceca221b31a38b01ee951fab82d5dc1c200c27b4" ceca221b]:
{{{
#!CommitTicketReference repository=""
revision="ceca221b31a38b01ee951fab82d5dc1c200c27b4"
Fixed #28304 -- Kept SafeData type for pgettext-translated strings
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28304#comment:7>