Feature #22483
open
Show PDF attachments and repo entries instead of downloading them
0%
Description
Following up on #22058, I would like to propose, that also PDF files are shown directly instead of forcing downloads.
This applies to two use cases:
1. Following a download link¶
When following a download link, image are sent with content-disposition: inline, while all other files are sent with attachment. Since the some browsers is able to render PDF files directly (Firefox, Chrome, Safari, IE with Adobe Reader installed) and people are probably used to it, I think that extending this exception to PDF files is a reasonable change. If native PDF rendering within the browser is not supported, then a download will be initiated by the browser anyway.
2. Displaying a PDF file in repository browser¶
When accessing "View" within the file view in the repository browsers, Redmine currently forces a download of the PDF. Instead an iframe could be rendered which displays the PDF within the Redmine UI. Browser support for this use case is similar to No. 1.
The patch series is based on current trunk (r15332) plus the changes of #22482. If desired, I could isolate the changes further, to make them work without #22482.
0004-Don-t-force-download-of-PDFs-but-show-in-browser-pre.patch¶
The change updates AttachmentsController and RepositoryController to send PDF files with content-disposition: inline. (See first use case.)
0005-Show-pdf-preview-for-repository-files-and-attachment.patch¶
The change adds attachments/pdf and common/_pdf views to render PDF files in an iframe/object construct. (See second use case.)
Since we cannot easily know the PDF’s dimensions, the frame size follows the following rules:
- it is as wide as possible (100%)
- in general it is as high as it is wide
- it is never higher than 80 % of the viewport (80vh), since we want to make sure, that there is always a bit of Redmine UI visible to avoid getting “trapped” within the frame
Using a nested object/iframe structure was taken from http://pdfobject.com/static.html
Files
Related issues
Updated by Go MAEDA over 8 years ago
- Blocked by Feature #22482: Respond with "No preview available" instead of sending the file when no preview is available added
Updated by Go MAEDA over 8 years ago
@Gregor, could you please provide rebased version of these patches?
I am very interested in this feature but patches cannot be applied to the current trunk.
Updated by Gregor Schmidt over 8 years ago
- File 0001-Don-t-force-download-of-PDFs-but-show-in-browser-pre.patch 0001-Don-t-force-download-of-PDFs-but-show-in-browser-pre.patch added
- File 0002-Show-pdf-preview-for-repository-files-and-attachment.patch 0002-Show-pdf-preview-for-repository-files-and-attachment.patch added
Updated patches to r15404.
Updated by Go MAEDA over 8 years ago
- File safari-error.png safari-error.png added
Thanks for updating patches.
But in my environemnt, I cannot view PDF in browsers. I tried two web servers, Pow 0.5.0 and Webrick, both failed.
Error:
HTTP Response header:
Content-Type is binary/octet-stream. Perhaps it causes this problem.
HTTP/1.1 200 OK X-Frame-Options: SAMEORIGIN X-XSS-Protection: 1; mode=block X-Content-Type-Options: nosniff ETag: "13891eae2aa4115e138138ea507d1fdf" Content-Disposition: inline; filename="aaaaaaa-40.pdf" Content-Transfer-Encoding: binary Content-Type: binary/octet-stream Cache-Control: private Content-Length: 8331 X-Request-Id: ef5b1ea8-cb19-4cfa-8db2-1aac427b0b1e X-Runtime: 0.025105 Date: Mon, 09 May 2016 09:17:53 GMT Connection: keep-alive
Updated by Gregor Schmidt over 8 years ago
I'm sorry, but I cannot reproduce your problems. I've tried Firefox (Mac), Safari (Mac and iOS 9 Simulator) on Webrick, Unicorn and Puma. In any case, the Content-Type was application/pdf, which is determined by the detect_content_type method within the AttachmentsController. Maybe you could try debugging there to find out, what happened.
I have also tested the changes on IE w/o Adobe Reader installed, i.e. it cannot render PDFs within the browser. It didn't give me an error but downloaded the file instead. So I was rather confident, that there should be no negative side effects when sending PDFs with Content-Disposition: inline.
Updated by Go MAEDA over 8 years ago
Thanks for investigating the problem I wrote in #22483#note-5.
I found that no problem in your patches, it is caused by files which are uploaded by Firefox. Attachment#content_type of these files are 'binary/octet-stream', so 'Content-Type' field in response header will be 'binary/octet-stream'. Maybe related to https://bugzilla.mozilla.org/show_bug.cgi?id=373621 .
2.2.4 :004 > Attachment.find(43).content_type Attachment Load (0.2ms) SELECT "attachments".* FROM "attachments" WHERE "attachments"."id" = ? LIMIT 1 [["id", 43]] => "binary/octet-stream"
The patch works fine with files uploaded by Chrome and Safari. This feature is extremely beneficial for users who handle PDF files frequently.
Updated by Go MAEDA over 8 years ago
- File 060719210727_example.pdf 060719210727_example.pdf added
Although the patches works fine, but test fails because PDF file "test/fixtures/files/2006/07/060719210727_example.pdf" does not exist.
$ ruby test/functional/attachments_controller_test.rb (snip) 1) Failure: AttachmentsControllerTest#test_show_pdf [test/functional/attachments_controller_test.rb:204]: Expected response to be a <success>, but was <404>
log/test.log:
----------------------------------------
AttachmentsControllerTest: test_show_pdf
----------------------------------------
Processing by AttachmentsController#show as HTML
Parameters: {"id"=>"21"}
(snip)
Cannot send attachment, /*****/redmine-trunk/test/fixtures/files/2006/07/060719210727_example.pdf does not exist or is unreadable.
(snip)
Completed 404 Not Found in 26ms (Views: 20.2ms | ActiveRecord: 1.0ms)
To pass tests, copy 060719210727_example.pdf to test/fixtures/files/2006/07/ directory.
Updated by Go MAEDA over 8 years ago
- Assignee set to Jean-Philippe Lang
- Target version set to 3.3.0
@Jean-Philippe, could this feature be included in 3.3.0?
I know 3.3.0 is entering feature freeze but I think it would be nice if this feature is delivered along with #22058.
Updated by Gregor Schmidt over 8 years ago
- File 060719210727_example.pdf 060719210727_example.pdf added
Go MAEDA wrote:
Although the patches works fine, but test fails because PDF file "test/fixtures/files/2006/07/060719210727_example.pdf" does not exist.
Sorry, I forgot to check how git's format-patch handles binary files. It turns out, they are omitted in the output by default.
Here is the example pdf, that I used locally. It's just a "Print as PDF" of http://example.org. As you mentioned, it needs to be placed at test/fixtures/files/2006/07/060719210727_example.pdf
Updated by Gregor Schmidt over 8 years ago
Concerning your problem with the wrong mime type in Firefox:
I am using Firefox as my main browser and also uploaded the files, that I used for testing, using Firefox. Not sure, why you would observe a different behavior.
Anyway, the underlying problem is, that different methods are used to determine the mime type and therefore different results may occur. The disposition method just checks the file ending, the detect_content_type tries to reuse the information passed by the browser. I assume, this should be fixed, so that only a single source is used. Maybe the introduction of the mimemagic gem could provide better facilities for mime handling, than those currently in place in Redmine. But this might be a topic for a separate issue.
Updated by Go MAEDA over 8 years ago
- File deleted (
0004-Don-t-force-download-of-PDFs-but-show-in-browser-pre.patch)
Updated by Go MAEDA over 8 years ago
- File deleted (
0005-Show-pdf-preview-for-repository-files-and-attachment.patch)
Updated by Jean-Philippe Lang over 8 years ago
- Status changed from New to Needs feedback
Sending PDF inline instead of forcing the download is committed, but I'm not really sure about the preview in an iframe. I find it much less confortable than viewing it using the whole viewport of the browser. I get double scrollbars, one for the body and one for the iframe (I had to lower the height to 70vp but I guess it depens on the header and window height). Also, they're no way to zoom the content of the iframe under iOS (tested with 8) which make the preview useless.
Updated by Go MAEDA over 8 years ago
- Has duplicate Feature #22098: Open attached pdf files in the browser added
Updated by Jan from Planio www.plan.io about 7 years ago
- Related to Patch #27336: Render previews for audio and video files added
Updated by donny osmon over 6 years ago
Hi, are these 2 patches for Linux (CentOS) ?
If so, do they work on Redmine 3.4.3.stable ?
Are there patch install instructions aside from "patch -p0 < the_patch.patch" ?
Do the patches require a restart of Redmine?
Thanks
0004-Don-t-force-download-of-PDFs-but-show-in-browser-pre.patch
0005-Show-pdf-preview-for-repository-files-and-attachment.patch
Updated by donny osmon over 6 years ago
donny osmon wrote:
Hi, are these 2 patches for Linux (CentOS) ?
If so, do they work on Redmine 3.4.3.stable ?
Are there patch install instructions aside from "patch -p0 < the_patch.patch" ?
Do the patches require a restart of Redmine?
Does patch 0005 include patch 0004 ?
Thanks0004-Don-t-force-download-of-PDFs-but-show-in-browser-pre.patch
0005-Show-pdf-preview-for-repository-files-and-attachment.patch
Updated by Gần Sôi Nước almost 5 years ago
Updated by Bernhard Rohloff almost 3 years ago
- Has duplicate Feature #36504: PDF preview in attachment Page added
Updated by Steven Uggowitzer over 2 years ago
Has anyone updated these patches for Version 5.x ?
Updated by Steven Uggowitzer over 2 years ago
Steven Uggowitzer wrote:
Has anyone updated these patches for Version 5.x ?
So it turns out that this actually already works in Redmine 5.x. However, for anyone trying to make it work for PDF files > 5mb, you have to actually raise the file size settings in /settings?tab=attachments to exceed the size of your PDFs. Otherwise this will not work, and you don't get a preview. Hopefully this note will help someone. All my PDFs in a repository were well over 5mb that I was testing it on, and it just didn't work for me till I realised this.