TracError message cannot contain a div

[Ryan J Ollos]

SpamFilter raises an error with a div wrapping a list.

            self.reject_handler.reject_content(
                req, tag.div(
                    tag_('Submission rejected as potential spam %(message)s',
                         message=msg),
                    class_='message'))

This doesn't render correctly.

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

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

@@ -81 +81 @@
         <py:when test="'TracError'">
           <h1>$title</h1>
           <py:choose test="">
-            <p py:when="not find_element(message, tag='p')" class="message">$message</p>
+            <p py:when="not find_element(message, cls='message')" class="message">$message</p>
             <py:otherwise>$message</py:otherwise>
           </py:choose>
         </py:when>

[Ryan J Ollos]

Related changes in r16321 and r16322.

I considered a workaround for SpamFilter,

1.2/tracspamfilter/filtersystem.py

@@ -296 +296 @@
             rejects = []
             outreasons.sort()
             for r in outreasons:
-                rejects.append(tag.li(r))
+                rejects.append(tag.li(tag.p(r)))
             msg = tag.div(tag.ul(rejects), class_='message')
 
             self.reject_handler.reject_content(req, msg)

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

[Ryan J Ollos]

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.

[Ryan J Ollos]

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

[Ryan J Ollos]

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