On 03/09/2016 12:38 PM, Jakub Wilk wrote:
> * Ben Finney <ben+deb...@benfinney.id.au>, 2016-03-09, 10:51:
>> On 06-Mar-2016, efkin wrote:
>>> This should fix email headers that were not properly rendered.
>>
>> Thank you, efkin, for making a patch.

Thanks both for quick answering.

>> I think that is addressing the problem in the wrong place.
>>
>> The template tag should not do any encoding. Instead, it should just
>> do template interpolation, and return the Unicode text.
>>
>> Encoding that text for transmission should be done only after the
>> whole message is composed.

I thought about trying to be delicate with the patch and try to refactor
as less code as possible given that these are my first steps towards
contributing in this project.

And at the same time i tried to refactor accordingly to the already
existing code style/logic.

So i thought that it could be a good place to do it.

The problem about moving this operation to
`backend.email.send_notification` or to
`backend.email.template_to_email` for me it relies on coding extra and
unnecesary calls to `email.utils.parseaddr` and `email.utils.formataddr`.

And i understand that a template filter may not be the better place. But
as the `email.utils.formataddr` call was there i thought about the
minimum possible change.

However i could prepare another patch that implements your suggestions
if necessary. Just let me know.

>> So I think the correct solution entails:
>>
>> * In ‘backend.templatetags.nm.formataddr’, remove the ‘encode’ call.
>>
>> * In ‘backend.email.send_notification’, encode all fields of the
>> header (using ‘email.header.Header’ as you suggest).
> 
> No, that wouldn't work:
> 
>>>> email.utils.formataddr((u'Cédric Boutillier', 'bou...@debian.org'))
> u'C\xe9dric Boutillier <bou...@debian.org>'
>>>> str(email.header.Header(_))
> '=?utf-8?q?C=C3=A9dric_Boutillier_=3Cboutil=40debian=2Eorg=3E?='
> 
> The result is not a valid address.
> 
> RFC 2047 encoding must be applied only to the display-name part (as it
> was done in the original patch), not the whole address.

Exactly.

bests,

efkin.

Reply via email to