Modify

Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#881 closed defect (fixed)

Invalid HTML code makes Trac 0.10.1 fail

Reported by: Emmanuel Blot Owned by: Peter Kropf
Priority: high Component: GraphvizPlugin
Severity: blocker Keywords:
Cc: Trac Release: 0.10

Description

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.

Attachments (0)

Change History (6)

comment:1 Changed 11 years ago by Peter Kropf

Would you have a patch to fix the problem?

comment:2 in reply to:  1 Changed 11 years ago by 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')

comment:3 Changed 10 years ago by 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.

comment:4 Changed 10 years ago by 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))

comment:5 Changed 10 years ago by 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.

comment:6 Changed 10 years ago by Peter Kropf

Resolution: fixed
Status: newclosed

Fixed w/ changeset:1731.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Peter Kropf.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.