Vote #77354
未完了Show PDF attachments and repo entries instead of downloading them
0%
説明
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:
h3. 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.
h3. 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.
h3. @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.)
h3. @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
journals
!pdf-preview.png!
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
@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 patches to r15404.
--------------------------------------------------------------------------------
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:**
!safari-error.png!
**HTTP Response header:**
Content-Type is @binary/octet-stream@. Perhaps it causes this problem.
<pre>
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
</pre>
--------------------------------------------------------------------------------
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@.
--------------------------------------------------------------------------------
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 .
<pre>
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"
</pre>
The patch works fine with files uploaded by Chrome and Safari. This feature is extremely beneficial for users who handle PDF files frequently.
--------------------------------------------------------------------------------
Although the patches works fine, but test fails because PDF file "test/fixtures/files/2006/07/060719210727_example.pdf" does not exist.
<pre>
$ 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>
</pre>
log/test.log:
<pre>
----------------------------------------
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)
</pre>
To pass tests, copy attachment:060719210727_example.pdf to test/fixtures/files/2006/07/ directory.
--------------------------------------------------------------------------------
@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.
--------------------------------------------------------------------------------
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@
--------------------------------------------------------------------------------
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.
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
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.
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
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
--------------------------------------------------------------------------------
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 ?
> Thanks
>
> 0004-Don-t-force-download-of-PDFs-but-show-in-browser-pre.patch
> 0005-Show-pdf-preview-for-repository-files-and-attachment.patch
--------------------------------------------------------------------------------
Gregor Schmidt wrote:
> Updated patches to r15404.
Thank you!
--------------------------------------------------------------------------------
--------------------------------------------------------------------------------
related_issues
relates,Closed,27336,Render previews for audio and video files
duplicates,Closed,22098,Open attached pdf files in the browser
duplicates,Closed,36504,PDF preview in attachment Page
blocks,Closed,22482,Respond with "No preview available" instead of sending the file when no preview is available