Feature #27822

Remove filename from attachment preview links

Added by Felix Schäfer 7 months ago. Updated 10 days ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Go MAEDA% Done:

0%

Category:Attachments
Target version:4.0.0
Resolution:Fixed

Description

Some of our users reported being confused by the attachment previews. The most common use-case was a variation of:
  1. right-click on the link of the attachment on the issue page,
  2. "save link as" to save the attachment to disk,
  3. send the saved file to a user not on the Redmine.

With the current URL scheme for attachment preview pages (/attachments/:id/:filename), when the users click the preview attachment link instead of the download attachment link, the resulting html file still has the filename of the attachment. If the user does not have a look at the attachment before using it for something else, the user might have downloaded the html preview page instead of the attachment itself.

27822.patch Magnifier (1.51 KB) Felix Schäfer, 2017-12-19 14:47

27822-tests.patch Magnifier (3.82 KB) Gregor Schmidt, 2018-07-09 16:02

Associated revisions

Revision 17440
Added by Go MAEDA 10 days ago

Remove filename from attachment preview links (#27822).

Patch by Felix Schäfer and Gregor Schmidt.

History

#1 Updated by Felix Schäfer 7 months ago

The attached patch changes the URL rendered to preview pages from /attachments/:id/:filename to /attachments/:id. Both URLs work before and after the patch, so this will for example not break plugins or links coming from external sites. The links to the preview attachment pages in Redmine will not have the filename in the URL, thus if a user erroneously downloads that instead of the attachment the downloaded file will be called 123.html instead of image.png for example.

#2 Updated by Go MAEDA 7 months ago

  • Target version set to Candidate for next major release

#3 Updated by Go MAEDA 7 months ago

  • Target version changed from Candidate for next major release to 4.1.0

#4 Updated by Go MAEDA 12 days ago

We have to update existing tests to commit this patch.

This is an example:

Failure:
ApplicationHelperTest#test_attachment_link_should_link_to_latest_attachment [/Users/maeda/redmines/redmine-trunk/test/helpers/application_helper_test.rb:694]:
--- expected
+++ actual
@@ -1 +1 @@
-"<p><a class=\"attachment\" href=\"/attachments/1100/test.txt\">test.txt</a></p>" 
+"<p><a class=\"attachment\" href=\"/attachments/1100\">test.txt</a></p>" 

#5 Updated by Gregor Schmidt 10 days ago

Thanks for taking a look at this patch. Attached you may find a diff, which adds the necessary changes to the test suite.

#6 Updated by Go MAEDA 10 days ago

  • Status changed from New to Closed
  • Assignee set to Go MAEDA
  • Target version changed from 4.1.0 to 4.0.0
  • Resolution set to Fixed

Committed. Thank you for sharing the patches.

Also available in: Atom PDF