pgettext should return SafeData if both `message` and `context` are instances of SafeData

[Artem Polunin]

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 "&#39;"
msgstr "&#39; відсотків"

msgctxt "percent"
msgid "&#39;"
msgstr "&#39; відсотків"

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('&#39;')))    # 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 `&`
'' відсотків'
>>> 
>>> 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.

[Tim Graham]

Seems okay at first glance, although I'm not a translations expert. Perhaps it would suffice to mark CONTEXT_SEPARATOR as safe?

[Artem Polunin]

Replying to 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'>

[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.

[Artem Polunin]

Replying to 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.

[Claude Paroz]

​PR

[Claude Paroz <claude@…>]

In ceca221b:

Fixed #28304 -- Kept SafeData type for pgettext-translated strings