Edgewall Software

Opened 5 years ago

Closed 5 years ago

#12916 closed defect (fixed)

TracError message cannot contain a div

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.2.3
Component: general Version:
Severity: normal Keywords:
Cc: Branch:
Release Notes:

Fixed incorrect rendering of TracError message containing a div.

API Changes:
Internal Changes:

Description (last modified by Ryan J Ollos)

SpamFilter raises an error with a div wrapping a list.

                req, tag.div(
                    tag_('Submission rejected as potential spam %(message)s',

This doesn't render correctly.

<h1>Trac Error</h1>
<p class="message"></p>
<div class="message">Submission rejected as potential spam 
    <li>BlogSpam says content is spam (IP previously blocked: Anchor text matches pattern: payday loan [20-ip.js])</li>

It seems reasonable that the user raising the TracError would want to render content like a list in a div, so we probably shouldn't restrict to p elements.

One way to deal with this would be to look for the message class rather than a p element when deciding whether to wrap the error message with p.message:

  • trac/templates/error.html

    diff --git a/trac/templates/error.html b/trac/templates/error.html
    index a17078d17..1fb241c65 100644
    a b ${description_en if url else description}</textarea>  
    8181        <py:when test="'TracError'">
    8282          <h1>$title</h1>
    8383          <py:choose test="">
    84             <p py:when="not find_element(message, tag='p')" class="message">$message</p>
     84            <p py:when="not find_element(message, cls='message')" class="message">$message</p>
    8585            <py:otherwise>$message</py:otherwise>
    8686          </py:choose>
    8787        </py:when>

Attachments (2)

Screen Shot 2017-09-13 at 17.17.12.png (92.2 KB ) - added by Ryan J Ollos 5 years ago.
Screen Shot 2017-09-30 at 17.31.52.png (30.0 KB ) - added by Ryan J Ollos 5 years ago.

Download all attachments as: .zip

Change History (8)

by Ryan J Ollos, 5 years ago

comment:1 by Ryan J Ollos, 5 years ago

Description: modified (diff)

comment:2 by Ryan J Ollos, 5 years ago


comment:3 by Ryan J Ollos, 5 years ago

Related changes in r16321 and r16322.

I considered a workaround for SpamFilter,

  • 1.2/tracspamfilter/filtersystem.py

    296296            rejects = []
    297297            outreasons.sort()
    298298            for r in outreasons:
    299                 rejects.append(tag.li(r))
     299                rejects.append(tag.li(tag.p(r)))
    300300            msg = tag.div(tag.ul(rejects), class_='message')
    302302            self.reject_handler.reject_content(req, msg)

however, I think it's probably better to just fix on Trac 1.2-stable.

comment:4 by Ryan J Ollos, 5 years ago

The comment from r5299 needs to be revised. As of r5299, the entire exception message was wrapped in a p.message, not everything up to the first <p>.

The behavior was changed in r5388 ([5388#file4]), and modified again in #11368.

DONE update the docstring of TracError class.

I suppose we should mainly avoid wrapping with a p any fragment or element containing a block-level element, so it may be enough to add not find_element(message, tag='div') to the conditional.

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

by Ryan J Ollos, 5 years ago

comment:5 by Ryan J Ollos, 5 years ago

Release Notes: modified (diff)

Proposed changes in [5244275bc/rjollos.git].

comment:6 by Ryan J Ollos, 5 years ago

Resolution: fixed
Status: assignedclosed

Committed to 1.2-stable in r16346, merged to trunk in r16347.

Modify Ticket

Change Properties
Set your email in Preferences
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment

E-mail address and name can be saved in the Preferences .
Note: See TracTickets for help on using tickets.