Edgewall Software
Modify

Opened 18 years ago

Closed 18 years ago

#3456 closed defect (fixed)

Issue with SVG file and tracd standalone daemon

Reported by: Emmanuel Blot Owned by: Emmanuel Blot
Priority: normal Milestone: 0.10
Component: web frontend/tracd Version: devel
Severity: normal Keywords: review
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description

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.

Attachments (1)

mimetype-r3556.patch (833 bytes ) - added by Emmanuel Blot 18 years ago.
Use Trac get_mimetypes() to detect MIME type of static files

Download all attachments as: .zip

Change History (11)

by Emmanuel Blot, 18 years ago

Attachment: mimetype-r3556.patch added

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

comment:1 by Christian Boos, 18 years ago

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.

comment:2 by Emmanuel Blot, 18 years ago

Owner: changed from Jonas Borgström to Emmanuel Blot
Status: newassigned

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

comment:3 by Christian Boos, 18 years ago

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.

comment:4 by Emmanuel Blot, 18 years ago

Milestone: 0.10
Resolution: fixed
Status: assignedclosed

should be fixed in [3567]

in reply to:  4 ; comment:5 by Matthew Good, 18 years ago

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?

in reply to:  5 comment:6 by Christian Boos, 18 years ago

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.

comment:7 by Emmanuel Blot, 18 years ago

Resolution: fixed
Status: closedreopened

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.

comment:8 by Christopher Lenz, 18 years ago

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.

comment:9 by Emmanuel Blot, 18 years ago

Ok, I'll do it.

comment:10 by Emmanuel Blot, 18 years ago

Resolution: fixed
Status: reopenedclosed

Should have been fixed in [3571].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Emmanuel Blot.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Emmanuel Blot to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.