Edgewall Software
Modify

Ticket #7228 (closed enhancement: fixed)

Opened 4 years ago

Last modified 7 weeks ago

Download file as default action of browser

Reported by: Isaac.PEG Owned by: rblank
Priority: normal Milestone: 0.13
Component: version control/browser Version:
Severity: normal Keywords: consider
Cc: bread@…
Release Notes:

Repository browser now includes a download link for files and folders.

API Changes:

Description

I am attempting to use Trac to manage undergraduate curriculum, and would like to be able to download a file from the repository by clicking on the file name in the browser. This could be a server setting, or perhaps a checkbox on the browser page the user must choose.

I believe this feature could extend the usability of Trac to many non-coding projects, where the primary file type in the repository is binary. (e.g. ~95% of our files are either MS Word documents or Adobe Illustrator images.)

It would also alleviate some of the adoption issues involved: it's easier for many people to think of the Trac browser as simply browsing a file server, rather than be worrying about all the nuances of a versioned repository.

Attachments

7228-download-source-r9966.patch (30.7 KB) - added by rblank 19 months ago.
Add raw download links for source: links and in the repository browser.
download-icon-in-size-column.patch (1.9 KB) - added by cboos 19 months ago.
Move the download icon in the Size column, after the size information (applies on attachment:7228-download-source-r9966.patch)
7228-download-source-r9975.patch (32.8 KB) - added by rblank 19 months ago.
Works with IE7, download link next to size.

Download all attachments as: .zip

Change History

comment:1 in reply to: ↑ description ; follow-up: Changed 4 years ago by eblot

Replying to Isaac.PEG:

It would also alleviate some of the adoption issues involved: it's easier for many people to think of the Trac browser as simply browsing a file server, rather than be worrying about all the nuances of a versioned repository.

IMHO, Trac is first aimed at developers, maybe a less specialized tool would better fit your need.

comment:2 in reply to: ↑ 1 Changed 4 years ago by markus.einsle@…

Replying to eblot:

Replying to Isaac.PEG:

It would also alleviate some of the adoption issues involved: it's easier for many people to think of the Trac browser as simply browsing a file server, rather than be worrying about all the nuances of a versioned repository.

IMHO, Trac is first aimed at developers, maybe a less specialized tool would better fit your need.

What about providing an additional download link for each file?
This could easily be done by modifying the file icon besides each file displayed: clicking the file name enters viewing mode (as before), clicking the file icon operates as a download button.

comment:3 follow-up: Changed 3 years ago by mynameisgoor

You are not right.

comment:4 in reply to: ↑ 3 Changed 3 years ago by Isaac.PEG

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

Replying to mynameisgoor:

You are not right.

I'm not sure why you decided to make this unsupported claim after months of inactivity, but I have since given up on this idea because it seems no one else in this community believes this functionality would be useful.

comment:5 Changed 3 years ago by ebray

  • Resolution wontfix deleted
  • Status changed from closed to reopened
  • Version 0.11rc1 deleted

Actually, I've had users ask for functionality like this, and have sometimes found myself wanting it. I don't think making the file icons links to a download would be the best idea--it would be too difficult for users to find and too confusing. But I think a separate download link somewhere in the browser could work.

I'll try to come up with a solution that doesn't make things too cluttered, and then post a patch.

comment:6 Changed 3 years ago by cboos

  • Keywords consider added
  • Milestone set to 0.13

Waiting for the patch proposal...

comment:7 Changed 23 months ago by anonymous

I was searching for this feature too, and I've found this ticket: I'm sure it will be useful.

comment:8 Changed 20 months ago by rblank

  • Milestone changed from next-major-0.1X to 0.13
  • Owner changed from cboos to rblank
  • Status changed from reopened to new

We could use the same icon as for attachments (#5718) in the browser, and for source: links. I'll try that.

Changed 19 months ago by rblank

Add raw download links for source: links and in the repository browser.

comment:9 follow-up: Changed 19 months ago by rblank

The patch above adds raw download links at the following locations:

  • Next to source: links to files, with the same style as attachments (#5718).
  • Next to source: links to directories matching [browser] downloadable_paths, and the link points to the ZIP download.
  • Next to files in the repository browser.
  • Next to directories in the repository browser matching [browser] downloadable_paths.

The patch also changes the following:

  • source: links now appear grey if the destination doesn't exist, similar to wiki pages and attachments.
  • The HTML generated for the raw download links is made much simpler and smaller by using a CSS background image instead of an <img> tag, while still ensuring that the icons disappear when printing.
  • The export: links now point to /browser with format=raw instead of /export. This allowed better code reuse, and feels more consistent. This also avoid the weird structure of /export/{rev}/path.
  • Directory names in the repository browser are not wrapped in a <div> anymore in expand_dir.js (see #5277 and [5362]). This works fine on Firefox, Opera, Chrome, IE8 and Safari, so that workaround doesn't seem to be necessary anymore.

The only remaining question is: do the icons in the repository browser make the view too crowded? I find them quite discrete, so I would keep them unconditionally. But I'm open to other opinions, in which case we could make them configurable (or document how they can be removed with CSS).

Slightly OT: the filename generated for ZIP downloads of directories are sometimes nice (e.g. tags_trac-0.5-8274.zip), and sometimes extremely ugly (e.g. diff-from-dir1-1d2e5a45169b3d825ce5be5a9d969cf896aa9c7f-to-dir1-1d2e5a45169b3d825ce5be5a9d969cf896aa9c7f). I wonder where this comes from.

Last edited 19 months ago by rblank (previous) (diff)

comment:10 in reply to: ↑ 9 ; follow-up: Changed 19 months ago by cboos

Replying to rblank:

  • The HTML generated for the raw download links is made much simpler and smaller by using a CSS background image instead of an <img> tag, while still ensuring that the icons disappear when printing.

... except this doesn't seem to work with IExplorer 7. I think that complexity precisely came in the first place for the need to support IE7. So the question is, should we still support this browser?

  • The export: links now point to /browser with format=raw instead of /export. This allowed better code reuse, and feels more consistent. This also avoid the weird structure of /export/{rev}/path.

Well, the /export/{rev}/{path} URLs were added precisely to avoid having to use /browser/{path}?rev={rev}&format=raw. See r4242 and #3579.

  • Directory names in the repository browser are not wrapped in a <div> anymore in expand_dir.js (see #5277 and [5362]). This works fine on Firefox, Opera, Chrome and Safari, so that workaround doesn't seem to be necessary anymore.

But this still breaks in IExplorer 7, unfortunately.

The only remaining question is: do the icons in the repository browser make the view too crowded? I find them quite discrete, so I would keep them unconditionally. But I'm open to other opinions, in which case we could make them configurable (or document how they can be removed with CSS).

Yes, I find the repository too crowded. I tried several alternatives, and moving the download links in the Size columns looks nice and seems appropriate as we're talking in this column about getting to the "bytes". It also bears some similarity with the Rev column where we have a main link and a secondary icon link.

Changed 19 months ago by cboos

Move the download icon in the Size column, after the size information (applies on attachment:7228-download-source-r9966.patch)

comment:11 in reply to: ↑ 10 ; follow-up: Changed 19 months ago by rblank

Replying to cboos:

... except this doesn't seem to work with IExplorer 7. I think that complexity precisely came in the first place for the need to support IE7. So the question is, should we still support this browser?

No, that complexity came because I didn't know how to do it better at the time. Does it also not display the icon for external links and e-mail addresses? Those are generated using the same mechanism, except for the fact that the content of the link is non-empty. Note that I did have a few strange issues even with IE8 that went away with a clean reload in the browser.

I'll try to get access to IE7 and reproduce the issue.

Well, the /export/{rev}/{path} URLs were added precisely to avoid having to use /browser/{path}?rev={rev}&format=raw. See r4242 and #3579.

Ok, I didn't know that. I'll restore the export: links, then.

But this still breaks in IExplorer 7, unfortunately.

I'll try to test IE7. I forgot to mention in comment:9 that I had tested with IE8.

Yes, I find the repository too crowded. I tried several alternatives, and moving the download links in the Size columns looks nice and seems appropriate as we're talking in this column about getting to the "bytes". It also bears some similarity with the Rev column where we have a main link and a secondary icon link.

I'll test the patch. Thanks for the feedback.

comment:12 in reply to: ↑ 11 Changed 19 months ago by cboos

Replying to rblank:

... Note that I did have a few strange issues even with IE8 that went away with a clean reload in the browser.

I'll try to get access to IE7 and reproduce the issue.

If you have IE8 that's easy ;-) F12 and "Browser Mode: IE7". Of course, testing with a "real" IE7 wouldn't harm, but I think the compatibility mode is reliable.

Well, the /export/{rev}/{path} URLs were added precisely to avoid having to use /browser/{path}?rev={rev}&format=raw. See r4242 and #3579.

Ok, I didn't know that. I'll restore the export: links, then.

Thanks!

comment:13 Changed 19 months ago by rblank

The issue with IE7 not displaying the icons was easy, and is due to IE7 not liking links with empty content. Adding a zero-width space fixes the issue.

The issue in IE7 where the directory expanders are not displayed is separate from this ticket, as it also occurs with a pristine 0.12, and now I remember seeing this in the past. I'll try to find a fix.

The download icon next to the size looks good.

comment:14 Changed 19 months ago by rblank

That's really weird. When coming to the point in browser.html where enableExpandDir() is used, it is undefined in IE7 mode. In IE8 mode, the function is defined. What gives?

comment:15 Changed 19 months ago by rblank

Ok, that's really stupid. IE7 throws a parse error when an object literal has a trailing comma, and that prevents expand_dir.js from executing. Fixed in [9972].

Changed 19 months ago by rblank

Works with IE7, download link next to size.

comment:16 follow-up: Changed 19 months ago by rblank

This patch updates the following:

  • export: links (and all download links) use /export.
  • The links appear correctly on IE7.
  • The download links are placed next to the size in the repository browser.

With the fix in [9972], the directory expansion works even in IE7, so the wrapping in <div></div> is really not necessary anymore.

I like the result, except maybe for the download links for directories in the browser, which tend to sit all alone in their cell, as directories don't have a size. But that's ok for me.

comment:17 in reply to: ↑ 16 ; follow-up: Changed 19 months ago by cboos

Replying to rblank:

I like the result,

Yep, patch looks really good now, feel free to commit or to wait a bit more for feedback from others.

except maybe for the download links for directories in the browser, which tend to sit all alone in their cell, as directories don't have a size. But that's ok for me.

I can imagine, but I didn't try yet. Maybe a different icon (indicating an archive) would look better in this particular case?

A few more comments on the code touched by the patch, just for discussion or follow-up changesets:

  • node.can_view(perm) doesn't "read" well, as it's reversed (the perm can allow you to view the node); we should find something better (e.g. node.is_viewable(perm)) or leave that alone until we start using the shorter permission names (those without the realms, i.e. 'view' in perm(node))
  • I think we can start using the Python 2.5 conditional expression syntax, if only to check if we would really get complaints about the switch to 2.5 :-)

comment:18 in reply to: ↑ 17 Changed 19 months ago by rblank

Replying to cboos:

I can imagine, but I didn't try yet. Maybe a different icon (indicating an archive) would look better in this particular case?

That's a good idea. Unfortunately, my artistic abilities are rather lacking, so I'll need help for the other icon. I'll commit the patch as-is for now, and we can change the icon whenever we get a good one.

  • node.can_view(perm) doesn't "read" well, as it's reversed (the perm can allow you to view the node); we should find something better (e.g. node.is_viewable(perm)) or leave that alone until we start using the shorter permission names (those without the realms, i.e. 'view' in perm(node))

Yeah, is_viewable() would have been better. I guess I made up the name from the point of view of the user, but that wasn't ideal. I'm ok with the change, provided we change it on 0.12-stable as well, so that the old name doesn't get used too much (and we can keep the old name as an alias as well). I'll put that on my todo.

  • I think we can start using the Python 2.5 conditional expression syntax, if only to check if we would really get complaints about the switch to 2.5 :-)

True! But does Genshi support them?

That brings up another question: when do we switch to using with and a context manager for the transactions? I'd be in favor of doing it ASAP (and I volunteer to implement the change), but this might make merges from 0.12-stable slightly more painful. The alternative would be to wait until we get somewhat close to a 0.13 release, but I don't think it's worth waiting. I have opened #9536 for tracking that.

comment:19 Changed 19 months ago by rblank

Patch committed in [9976] with a few minor changes:

  • Made directories use a different class trac-ziplink, so that a different icon can be used. Currently, the icon is the same as for trac-rawlink.
  • Added a few conditional expressions.

Now, if someone can provide a nice icon with the same format as trunk/trac/htdocs/download.png that symbolizes downloading as a ZIP file, I would be glad to commit it.

About can_view(), should we do the switch, and to what name?

  • node.is_viewable()
  • node.can_be_viewed()
  • node.is_allowed() -- not really descriptive
  • any other ideas?

comment:20 Changed 15 months ago by rblank

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

Renamed can_view() to is_viewable() in [10320]. I left an alias for the former for backward compatibility.

comment:21 Changed 7 weeks ago by Alex Willmer <al.willmer@…>

  • Release Notes modified (diff)
View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
to The owner will be changed from rblank. Next status will be 'closed'
Author


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

 
Note: See TracTickets for help on using tickets.