Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

PDFs containing Japanese characters #76

Closed
yohasebe opened this issue Sep 21, 2015 · 6 comments · Fixed by #105
Closed

PDFs containing Japanese characters #76

yohasebe opened this issue Sep 21, 2015 · 6 comments · Fixed by #105

Comments

@yohasebe
Copy link

The plugin (0.25.0) as it is does not show characters in Japanese (and probably many other languages) properly. They just appear as blank characters.

Solving this problem is quite easy, though. Adding

PDFJS.cMapUrl = "../cmaps/";
PDFJS.cMapPacked = true;

to ~/.atom/packages/pdf-view/node_modules/pdfjs-dist/build/pdf.js just makes my PDFs look fine. Hope the fix is applied in the next release.

Thanks!

@izuzak
Copy link
Owner

izuzak commented Sep 21, 2015

Can you provide a link to an example PDF where you're seeing this? Which version of Atom are you using and which OS? Did you see the same problem in previous versions of pdf-view?

As you can see, this project uses pdfjs-dist as a dependency, and it seems that your fix is related to that dependency. What do you think about reporting that problem to the dependency?

@yohasebe
Copy link
Author

I started using Atom and atom-pdf-view just two days ago and have been seeing this problem since then. Currently I'm on OSX 10.11 Beta with Atom 1.0.15.

Here is the link to a sample PDF that is problematic, just for your reference: https://www.dropbox.com/s/wz07fkgvo2nslsp/atom.pdf?dl=0

Actually, the PDFs that I have been trying on atom-pdf-view are all generated with LaTeX (TeX Live 2015). I just found a moment ago that PDFs generated otherwise are okay even if they contained Japanese. As you say, it looks like a problem of PDF.js rendering files generated with LaTeX.

I'll probably report it rather to the dependency. Thanks for your great work anyway.

@izuzak
Copy link
Owner

izuzak commented Sep 21, 2015

Thanks for following up -- yeah, I see the same problem in pdf-view. Let me know if you end up opening those issues and it turns out that this needs to be fixed here. So far, it sounds like it should be fixed either in pdfjs-dist or the place that's generating PDFs. Happy to reopen this then.

@izuzak izuzak closed this as completed Sep 21, 2015
@maruta
Copy link

maruta commented Jan 5, 2016

@izuzak
Thank you so much for developing this important package.
I think the parameters (cMapUrl and cMapPacked) are expected to be given by atom-pdf-view to pdfjs-dist through the global variable PDFJS.
https://github.com/mozilla/pdfjs-dist/blob/v1.3.148/build/pdf.js#L7611-7622
So, could you reopen this issue?

And,I found that if I replace the following code in atom-pdf-view/lib/pdf-editor-view.js
https://github.com/izuzak/atom-pdf-view/blob/b450c7fe8ea0237d050cc44e26fa273fcfb70bf6/lib/pdf-editor-view.js#L12-14
by the following one

global.PDFJS = {workerSrc: "temp",cMapUrl:"temp",cMapPacked:true};
require('./../node_modules/pdfjs-dist/build/pdf.js');
PDFJS.workerSrc = "file://" + path.resolve(__dirname, "../node_modules/pdfjs-dist/build/pdf.worker.js");
PDFJS.cMapUrl = "file://" + path.resolve(__dirname, "../node_modules/pdfjs-dist/cmaps")+"/";

the problem seems to be solved.

I'm not familiar with coding, so please forgive me if miss the point.

@izuzak
Copy link
Owner

izuzak commented Jan 7, 2016

Thanks for the explanation and for the code snippet to fix this, @maruta -- I just applied that fix and published a new version. Give it a try and let me know if things are okay now.

🙇

@maruta
Copy link

maruta commented Jan 7, 2016

@izuzak
I tried new version and found it works well.
Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants