Invalid HTML code makes Trac 0.10.1 fail

Would you have a patch to fix the problem?

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')

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.

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))

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.

Fixed w/ changeset:1731.