Edgewall Software

Opened 10 years ago

Closed 10 years ago

Last modified 3 years ago

#11369 closed defect (fixed)

Tags in error message from ticket notification is wrongly shown — at Version 9

Reported by: Jun Omae Owned by: Jun Omae
Priority: normal Milestone: 0.12.6
Component: ticket system Version: 1.0-stable
Severity: normal Keywords: notification
Cc: Branch:
Release Notes:

Fix wrongly showing warning messages from notification if both smtp_from and smtp_replyto in [notification] section are empty.

API Changes:

Added trac.util.html:to_fragment which converts an object to a Fragment instance.

Internal Changes:

Description (last modified by Jun Omae)

When [notification] smtp_from or smtp_reply_to have the problem, ticket notification would show an error message which has Genshi elements. But the tags is wrongly shown…. Trac 0.12-stable has the same issue.

Change History (11)

comment:1 by Jun Omae, 10 years ago

Description: modified (diff)

comment:2 by Ryan J Ollos, 10 years ago

Milestone: next-stable-1.0.x0.12.6
Owner: set to Ryan J Ollos
Status: newassigned

#11368 is cosmetic so I thought I'd push the change to 1.0-stable, but this one looks like it should be fixed on 0.12-stable, as you may have already been suggesting.

Last edited 10 years ago by Ryan J Ollos (previous) (diff)

comment:3 by Ryan J Ollos, 10 years ago

There are at least two issues:

  • The exception message is wrapped in a pair of sibling p tags. I think we can probably remove these, particularly if the error will always be handled and displayed as a warning, which is the only case that I see.
  • The Fragment is serialized to a unicode string when doing string interpolation to construct another fragment: tags/trac-0.12.5/trac/ticket/web_ui.py@:1250-1253#L1221. This is why we end up with markup such as:
    Warning:
    • The change has been saved, but an error occurred while sending notifications: <p>Unable to send email due to identity crisis.</p><p>Neither &lt;b&gt;notification.from&lt;/b&gt; nor &lt;b&gt;notification.reply_to&lt;/b&gt; are specified in the configuration.</p>

For the latter issue, I think we need to iterate over the Fragment, replacing the ascii strings with unicode strings: log:rjollos.git:t11369. I still plan to do some more testing since I'm not confident of the change yet.

Another issue, also related to #11368, is that it would be nice to strip out the markup when logging the error. That would make these errors more readable:

12:45:47 PM Trac[main] WARNING: HTTPInternalError: 500 Trac Error
(Authentication information not available. Please refer to the
<a href="/tracdev/wiki/TracInstall#ConfiguringAuthentication"
title="Configuring Authentication">installation documentation</a>.)
12:38:55 PM Trac[web_ui] ERROR: Failure sending notification on change to
ticket #5: TracError: Unable to send email due to identity crisis. Neither
<b>notification.from</b> nor <b>notification.reply_to</b> are specified
in the configuration.
Last edited 10 years ago by Ryan J Ollos (previous) (diff)

comment:4 by Ryan J Ollos, 10 years ago

I've thought about this more and it seems better to have to_unicode to always return a string, and instead add a new function, e.g. unicode_fragment.

in reply to:  4 comment:5 by Ryan J Ollos, 10 years ago

Replying to rjollos:

I've thought about this more and it seems better to have to_unicode to always return a string, and instead add a new function, e.g. unicode_fragment.

I made that change in [31e1f910/rjollos.git], but still considering alternatives.

by Jun Omae, 10 years ago

Attachment: 20131123T224600.png added

comment:6 by Jun Omae, 10 years ago

  1. If Babel is intalled, the warnings are broken.
  2. In 0.12-stable, I don't think we must change translation messages.
  3. I don't understand need of unicode_text_nodes. Why does not the method create new Fragment at all?

I've worked in log:jomae.git:t11369.1. But it breaks the warnings if Babel is installed, also.

by Jun Omae, 10 years ago

Attachment: 20131125T155058.png added

in reply to:  6 comment:7 by Jun Omae, 10 years ago

I updated log:jomae.git:t11369.1.

  1. If Babel is intalled, the warnings are broken.

This problem is caused by genshi.builder be not supporting Babel's LazyProxy object.

>>> from genshi.builder import tag
>>> from trac.util.translation import tag_
>>> tag_("Neither %(from_)s", from_=tag.b('from'))
<babel.support.LazyProxy object at 0xb7f458ec>
>>> unicode( tag.p(tag_("Neither %(from_)s", from_=tag.b('from'))) )
u'<p>TEXTNeither -1-1STARTb-1-1TEXTfrom-1-1ENDb-1-1TEXT-1-1</p>'

I've added to_fragment function in log:jomae.git:t11369.1 to solve it.

>>> from trac.util.html import to_fragment
>>> to_fragment( tag_("Neither %(from_)s", from_=tag.b('from')) )
<Fragment>
>>> unicode(to_fragment( tag_("Neither %(from_)s", from_=tag.b('from')) ))
u'Neither <b>from</b>'
>>> unicode( tag.p(to_fragment(tag_("Neither %(from_)s", from_=tag.b('from')))) )
u'<p>Neither <b>from</b></p>'

But another issue is showing English message even if locale isn't en_US. It is caused by temporarily disabling localization while sending ticket notification, see #8903 and r8969.

comment:8 by Ryan J Ollos, 10 years ago

Owner: changed from Ryan J Ollos to Jun Omae

comment:9 by Jun Omae, 10 years ago

API Changes: modified (diff)
Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

Applied log:jomae.git:t11369.1 and replaced notification.from and notification.reply_to in warnings with [notification] smtp_from and [notification] smtp_replyto in [12321]. Also, merged into 1.0-stable in [12322-12323] and trunk in [12324].

Note: See TracTickets for help on using tickets.