Issue with SVG file and tracd standalone daemon

[eblot]

I'm playing with the ​RevTree plugin which relies itself on the ​GraphvizPlugin.

When Trac is run with the tracd standalone daemon and the ​GraphvizPlugin outputs SVG data my browser only offers to save the file, rather than displaying the SVG graphics as an embedded image.

Using the LiveHttpHeaders extension for Firefox, it appears that tracd sends the SVG graphic as application/octet-stream content type, which is not a valid MIME type for SVG data.

Trac only relies on the Python mimetypes library to detect the MIME type of a file, whereas Trac implements a more powerful MIME type recognition scheme in trac.mimeview.api

I'm not sure whether the attached patch is valid, but it makes tracd to serve and identify properly the SVG files and eventually allows the ​GraphvizPlugin to work with the standalone tracd daemon.

Please comment.

[eblot]

Use Trac get_mimetypes() to detect MIME type of static files

[cboos]

The method that you patched is def send_file(self, path, mimetype=None):, so I guess a better fix would be that the caller sets the appropriate mimetype.

[eblot]

Sure, that would be the process_request from chrome.py. I was wondering whether this special mime handling has not been done on purpose... (?)

[cboos]

You're right, send_file was purposedly left like that, as to not introduce a dependency from the web layer to the mimeview one.

The Chrome module is a different story, as it's a Component, I guess there's no reason it shouldn't benefit from the rest of the infrastructure, trac.mimeview.api included.

[eblot]

should be fixed in [3567]

[mgood]

Replying to eblot:

should be fixed in [3567]

As cboos said, this introduces a dependency from the web.api to the mimeview package. AFACT there was no agreement on whether this change should be made in web.api or in the Chrome module. What's the verdict here?

[cboos]

Replying to mgood:

Replying to eblot:

should be fixed in [3567]

As cboos said, this introduces a dependency from the web.api to the mimeview package.

Yep, sorry if I was not explicit enough, Manu, but I had something exactly like [3567] rejected by cmlenz a while ago, for these reasons.

AFACT there was no agreement on whether this change should be made in web.api or in the Chrome module. What's the verdict here?

I'd be +1 to do that in the Chrome module, because I think the Chrome module is somewhat higher level than the web.api.

[eblot]

Before committing the patch I asked cmlenz on !#IRC and he told me the patch was ok (but the order of module inclusion, which I've changed / the original patch).

I can move the change to the Chrome module if you prefer. Please let me know.

[cmlenz]

Oops, I thought the patch modified web.chrome and not web.api.

Indeed, web.api should not depend on Mimeview or similar subsystems. Adding the dependency on the web.chrome layer would be okay though.

[eblot]

Ok, I'll do it.

[eblot]

Should have been fixed in [3571].