glitch in ITemplateStreamFilter backward compatibility support

[Christian Boos]

On an instance running r15474 where the SpamFilter is active, the (render with Jinja2 / parse as HTML / apply filters / re-render with Genshi) render cycle is broken for CDATA sections, as can be seen on the /about page:

  <div id="systeminfo">
    <script type="text/javascript">//&lt;![CDATA[
      jQuery(document).ready(function ($) {
        $("#systeminfo table")
          .before("&lt;p&gt;User Agent: &lt;code&gt;" + navigator.userAgent + "&lt;/code&gt;&lt;/p&gt;")
          .append("&lt;tr&gt;&lt;th&gt;jQuery&lt;/th&gt;&lt;td&gt;" + $().jquery + "&lt;/td&gt;&lt;/tr&gt;" +
                  "&lt;tr&gt;&lt;th&gt;jQuery UI&lt;/th&gt;&lt;td&gt;" + $.ui.version + "&lt;/td&gt;&lt;/tr&gt;" +
                  "&lt;tr&gt;&lt;th&gt;jQuery Timepicker&lt;/th&gt;&lt;td&gt;" + $.timepicker.version +
                  "&lt;/td&gt;&lt;/tr&gt;");
      });
    //]]&gt;</script>

It should be:

  <div id="systeminfo">
    <script type="text/javascript">//<![CDATA[
      jQuery(document).ready(function ($) {
        $("#systeminfo table")
          .before("<p>User Agent: <code>" + navigator.userAgent + "</code></p>")
          .append("<tr><th>jQuery</th><td>" + $().jquery + "</td></tr>" +
                  "<tr><th>jQuery UI</th><td>" + $.ui.version + "</td></tr>" +
                  "<tr><th>jQuery Timepicker</th><td>" + $.timepicker.version +
                  "</td></tr>");
      });
    //]]></script>

(rendered with Trac trunk at r15474 as well, but with normal Jinja2 rendering)

[Christian Boos]

Confusion on my part, CDATA sections are an XML thing only, that's why the genshi.input.HTML parser (rightfully) ignored the CDATA.

Using genshi.input.XML instead should fix this (r15487). The page content we generate with Jinja2, should be also valid XML, still.

However, this raises the question of whether it's valid for us to continue to use CDATA sections in our Genshi templates, if we're not formally using the XHTML syntax for HTML5.

• in "plain" HTML5 (text/html), CDATA sections are reserved for "foreign elements" (​https://www.w3.org/TR/html5/syntax.html#cdata-sections)
• in "XHTML" HTML5 (application/xml or application/xhtml+xml), CDATA sections are recognized

So with the current practice of using HTML5 (text/html), we should perhaps rather avoid having CDATA sections at all.

[Christian Boos]

Using the Genshi XML parser to re-parse the content is more fragile than using the HTML one, it seems. It failed here on t.e.o for the /about page.

OTOH, this did catch a genuine error:

      </tbody>
    </table>
  </div>
</div>
    </div>
  </body>
      <!-- # block content (content inherited from layout.html) -->
    <!-- # endblock content (content inherited from layout.html) -->

      <!-- # endblock content (placeholder in theme.html) -->
    </div>

    <div id="footer"><hr/>
      <a id="tracpowered" href="http://trac.edgewall.org/"
         ><img src="/chrome/common/trac_logo_mini.png" height="30"
               width="107" alt="Trac Powered"/></a>
      <p class="left">
Powered by         <a href="/about"><strong>Trac 1.3.2.dev0</strong></a>
        <br />
 By         <a href="http://www.edgewall.org/">Edgewall Software</a>
.      </p>
      <p class="right">En savoir plus sur le projet Trac :<br /> <a href="http://trac.edgewall.org/">http://trac.edgewall.org/</a></p>
    </div>
    <!--   # include 'site_footer.html' (theme.html) -->
    <!--   end of site_footer.html -->
    <!-- # endblock body (content inherited from theme.html) -->
  </body>
</html>

The about.html template:

      # include 'environment_info.html'

    </div>
  </body>
  ${ super() }
  # endblock content
</html>

(I should really try to enhance the checker, see TracDev/JinjaChecker#KnownIssues)

[Christian Boos]

Replying to Christian Boos:

OTOH, this did catch a genuine error: […] The about.html template:

was fixed in r15508.

[Christian Boos]

I think that before release we need to go back to HTML parsing and therefore avoid using CDATA.

[Christian Boos]

Replying to Christian Boos:

I think that before release we need to go back to HTML parsing and therefore avoid using CDATA.

This is done at the beginning of this series: log:cboos.git@t12677-html5

While I was looking at the generated HTML through the ​nu validator, I noticed a couple of other issues. The rest of the series addresses them.

I've by far not run an exhaustive check on all the pages we generate, but the most common issues should be dealt with. We can always continue to improve upon this incrementally while working towards the 1.4 release.

[Christian Boos]

Well, now I realize that even without the CDATA, the generate/re-parse cycle used in presence of ITemplateStreamFilter still breaks the content of <script> tags, i.e. < and > are still escaped as < and >.

[Christian Boos]

Replying to Christian Boos:

Well, now I realize that even without the CDATA, the generate/re-parse cycle used in presence of ITemplateStreamFilter still breaks the content of <script> tags, i.e. < and > are still escaped as < and >.

[6b2442e9/cboos.git] is a possible solution.

[Christian Boos]

Another glitch with the generate/re-parse cycle: in this situation, we don't generate the "late" <script> elements (e.g. see source HTML for TracWorkflow for an example, search for <script> elements containing jQuery.loadStyleSheet and jQuery.loadScript near the end of the page).

[Christian Boos]

… we don't generate the "late" <script> elements

Fixed in [00383b17/cboos.git].

[Christian Boos]

For [f2d323202/cboos.git], some help in testing this would be appreciated (i.e. we need to test support for a plugin which would provide its own Genshi template and make use of the late links/scripts).

[Christian Boos]

The whole series committed in [16851:16865], I preferred to keep the individual changes for more focused "blame" info and easier bisecting if needed later on.

Note that the problem described initially in this ticket is only fixed late in the series, in r16863.

[Ryan J Ollos]

Are prefix and suffix needed after ie_if is removed in 1.5.1?: trunk/trac/web/chrome.py@16864:192-193#L172​.

[Christian Boos]

Replying to Ryan J Ollos:

Are prefix and suffix​ needed after ie_if is removed in 1.5.1?

I don't see a reason for keeping them after that.

[Ryan J Ollos]

Replying to Christian Boos:

I don't see a reason for keeping them after that.

Thanks, I've posted proposed changes in comment:18:ticket:12787.

[Ryan J Ollos]

After r16852, translatable strings are not extracted inside script elements. See r17004.

Proposed fix:

trac/dist.py

@@ -66 +66 @@
 
     def handle_starttag(self, tag, attrs):
         if tag == 'script':
-            for kv in attrs:
-                if kv == ('type', 'text/javascript'):
-                    self.in_javascript = True
-                    break
+            self.in_javascript = True
 
     def handle_startendtag(self, tag, attrs):
         self.in_javascript = False

[Ryan J Ollos]

comment:20 change committed in r17011. New extraction in r17012.