Invalid HTML code makes Trac 0.10.1 fail

[Emmanuel Blot]

The following line: ​http://trac-hacks.org/browser/graphvizplugin/0.9/graphviz/graphviz.py#L243 contains invalid XHTML syntax.

This invalid syntax has been ignored up to now by both the Trac engine and the web browser. With the protection against CSRF attacks introduced in Trac ​0.10.1, the HTML validation engine fails when it encounters this pseudo-tag.

This issue make the plugin always fail with the latest official Trac release.

[Peter Kropf]

Would you have a patch to fix the problem?

[Emmanuel Blot]

Replying to pkropf:

Would you have a patch to fix the problem?

Well the following code is not valid from an HTML point of view

<!--[if IE]><embed src="%s/graphviz/%s" type="image/svg+xml" %s></embed><![endif]-->
<![if !IE]><object data="%s/graphviz/%s" type="image/svg+xml" %s>SVG Object</object><![endif]>

I'm not sure the <object>/<embed> differenciation is required. embed works with Firefox for example. The second line should have a valid comment tag: <!-- [if !IE]>

Anyway if a browser differenciation is required, it would be better to use the user agent HTTP header to generate the appropriate XHTML sequence (and get rid of the comment tags):

agentstr = req.get_header('user-agent')

[Jim Cheetham]

I can confirm on trac 0.10.2-1~edgy1 that adding comments almost as described above sort-of works; the page does not crash, pre-existing graphs are rendered, but not in IE, and I also get spurious text in the IE page ...

$ diff graphviz.py graphviz.py.orig
250c250
<             buf.write('<!--[if !IE]--><object data="%s/graphviz/%s" type="image/svg+xml" %s>SVG Object</object><!--[endif]-->' % (req.base_url, img_name, dimensions))
---
>             buf.write('<![if !IE]><object data="%s/graphviz/%s" type="image/svg+xml" %s>SVG Object</object><![endif]>' % (req.base_url, img_name, dimensions))

So it looks like the existing comments crash trac itself; changing them as above gets trac to work, but then you have a lot of browser tag issues. Still, better than nothing at the moment.

[Jim Cheetham]

Here's a better patch, however the browser detection is based upon finding the string 'MSIE' in the user agent, and I'm sure that's not generally supportable ..

# diff ../graphviz.py 0.9/graphviz/graphviz.py
28d27
< import string
250,253c249,250
<           if string.find(req.get_header('user-agent'),'MSIE') <> -1:
<                 buf.write('<!--[if IE]><embed src="%s/graphviz/%s" type="image/svg+xml" %s></embed><![endif]--> ' % (req.base_url, img_name, dimensions))
<           else:
<                 buf.write('<object data="%s/graphviz/%s" type="image/svg+xml" %s>SVG Object</object>' % (req.base_url, img_name, dimensions))
---
>             buf.write('<!--[if IE]><embed src="%s/graphviz/%s" type="image/svg+xml" %s></embed><![endif]--> ' % (req.base_url, img_name, dimensions))
>             buf.write('<![if !IE]><object data="%s/graphviz/%s" type="image/svg+xml" %s>SVG Object</object><![endif]>' % (req.base_url, img_name, dimensions))

[Jim Cheetham]

Finally a better way ...

Instead of using the string library and looking at the user-agent header, just output both the standards-compliant <object> and the legacy-style <embed> within it :-

buf.write('<object data="%s/graphviz/%s" type="image/svg+xml" %s><embed src="%s/graphviz/%s" type="image/svg+xml" %s></embed></object>' % (req.base_url, img_name, dimensions, req.base_url, img_name, dimensions))

This seems to be far better than attempting to do browser detection (which is inherently not stable) and certainly works on my FFox 2.0, IE6 and Opera 9 browsers on WinXP.

[Peter Kropf]

Fixed w/ changeset:1731.