Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

PDF preview broken on 2.1 #1194

Closed
wants to merge 1 commit into from
Closed

PDF preview broken on 2.1 #1194

wants to merge 1 commit into from

Conversation

domoritz
Copy link
Contributor

Failing on demo:
http://demo.ckan.org/dataset/pdf-test/resource/5597e098-83bb-4ffd-835b-2e94688007b8

Working:
http://demo.offene-daten.me/dataset/pdf-test/resource/f725204e-297b-44dd-bd1f-69764e34618b

@vitorbaptista :

Quickly looking into demo, it seems that pdfjs' minified version in https://github.com/okfn/ckan/blob/master/ckanext/pdfpreview/theme/public/vendor/pdfjs/pdf.min.js is broken. Looking in pdf.js' issues, it seems they had problems with minification already (mozilla/pdf.js#2479). As our pdf.min.js version is 10 months old, while pdf.js is 2 months, I guess we should re-minify it.

@domoritz
Copy link
Contributor

Duplicate #1195 :-)

@domoritz domoritz closed this Aug 15, 2013
@domoritz
Copy link
Contributor

Actually, I think we should just remove the minified file from 2.1 as there is no official minified file available.

@domoritz domoritz reopened this Aug 15, 2013
@ghost ghost assigned amercader Aug 15, 2013
@domoritz
Copy link
Contributor

@amercader Made a pull request that removes the file in 2.1.1. This way, the non-minified version is used.

@formwandler
Copy link

Correction: The example on http://demo.offene-daten.me/dataset/pdf-test/resource/f725204e-297b-44dd-bd1f-69764e34618b was initially NOT working.

But now, after removing pdf.min.js and restarting Apache it's working. Thanks!

@vitorbaptista
Copy link
Contributor

@amercader Was this fixed in another pull request, or it's still broken?

@amercader
Copy link
Member Author

@vitorbaptista I'm not sure tbh, I'll need to investigate

domoritz added a commit that referenced this pull request Oct 15, 2013
@amercader
Copy link
Member Author

Cherry-picked into 2.1.1

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants