Modify

Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#881 closed defect (fixed)

Invalid HTML code makes Trac 0.10.1 fail

Reported by: eblot Owned by: pkropf
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 follow-up: Changed 8 years ago by pkropf

Would you have a patch to fix the problem?

comment:2 in reply to: ↑ 1 Changed 8 years ago by eblot

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 8 years ago by jim@…

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 8 years ago by jim@…

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 8 years ago by jim@…

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 8 years ago by pkropf

  • Resolution set to fixed
  • Status changed from new to closed

Fixed w/ changeset:1731.

Add Comment

Modify Ticket

Action
as closed The owner will remain pkropf.
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.