Viewer origins #18
Viewer origins #18
Conversation
| function validateFileURL(file) { | ||
| try { | ||
| var viewerOrigin = new URL(window.location.href).origin || 'null'; | ||
| var viewerOrigin = window.location.origin; |
There was a problem hiding this comment.
I thought that this does the same thing (it's more readable). But according to https://developer.mozilla.org/en-US/docs/Web/API/Window/location you can't use window.location.origin is only available since IE 11, I think this is an IE workaround.
See this: http://tosbourn.com/a-fix-for-window-location-origin-in-internet-explorer/
I'll undo that change.
There was a problem hiding this comment.
@Elektron1c97 also we don't want to change that code really. It's property of the PDF.js project. Every update we make needs to reapply these changes.
|
@MattFenelon can you get your use-case to work with this addition? |
| function validateFileURL(file) { | ||
| try { | ||
| // IE workaround because window.location.origin | ||
| // is not available in < IE 11 |
There was a problem hiding this comment.
No need for a comment. We don't want to own this code. We just use what we get from PDF.js
|
@siegy22 A test-case needs to make sure the env variable has an effect. Otherwise we are going to loose this on an update. |
| @@ -1,6 +1,8 @@ | |||
| # Configure Rails Environment | |||
| ENV["RAILS_ENV"] = "test" | |||
|
|
|||
| ENV["PDFJS_VIEWER_ORIGINS"] = "http://example.com,http://random.example.com" | |||
There was a problem hiding this comment.
This should be set in the respective test-case and be reset after it (in an ensure)
|
Let's wait for feedback if this addresses the need voiced by others. |
|
That's okay for me 👌 |
|
This looks great, thanks for looking at it. I won't be able to test it until Monday now. As you own _viewer.html, rather than change viewer.js, could you modify HOSTED_VIEWER_ORIGINS in a js file loaded after viewer.js? |
|
Hey @MattFenelon I opened another pr ( #19 ) I explored a little "bug" in the code. Gonna fix this tommorow! |
|
What's the latest on this? |
|
Any updates on this? When are you planning to merge this? I would really like to have configurable origins :'( |
|
it's useful when viewing PDF saved in S3 or cloud service |
closes #12, closes #16
@andyweiss1982 and @senny have a 👀