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

Issue with some pdfs #369

Open
Gozala opened this issue Jan 9, 2020 · 10 comments
Open

Issue with some pdfs #369

Gozala opened this issue Jan 9, 2020 · 10 comments

Comments

@Gozala
Copy link
Contributor

Gozala commented Jan 9, 2020

For whatever reason pushpin seems to have issue with a following pdf
https://tools.ietf.org/pdf/rfc2557.pdf

@pvh
Copy link
Member

pvh commented Jan 9, 2020

I've seen this before. I see a ton of errors in the console like this:

Warning: getPathGenerator - ignoring character: "Error: Requesting object that isn't resolved yet Courier_path_).".
C:\Users\pvh\Dev\pushpin\node_modules\pdfjs-dist\build\pdf.js:549 Warning: getPathGenerator - ignoring character: "Error: Requesting object that isn't resolved yet Courier_path_".".
C:\Users\pvh\Dev\pushpin\node_modules\pdfjs-dist\build\pdf.js:549 Warning: getPathGenerator - ignoring character: "Error: Requesting object that isn't resolved yet Courier_path_z.".
C:\Users\pvh\Dev\pushpin\node_modules\pdfjs-dist\build\pdf.js:549 Warning: getPathGenerator - ignoring character: "Error: Requesting object that isn't resolved yet Courier_path_[.".
C:\Users\pvh\Dev\pushpin\node_modules\pdfjs-dist\build\pdf.js:549 Warning: getPathGenerator - ignoring character: "Error: Requesting object that isn't resolved yet Courier_path_F.".
C:\Users\pvh\Dev\pushpin\node_modules\pdfjs-dist\build\pdf.js:549 Warning: getPathGenerator - ignoring character: "Error: Requesting object that isn't resolved yet Courier_path_8.".

I think this is a bug in how I've set up PDF.js and remember seeing something about it in the FAQ. Hopefully any easy fix with easy confirmation.

@Gozala
Copy link
Contributor Author

Gozala commented Jan 9, 2020

I tried hypothesis on that pdf as I recall them using pdf.js and it seems to work fine in their embedding
https://via.hypothes.is/https://tools.ietf.org/pdf/rfc2557.pdf#annotations:zUk5UDM2Eeq1FYMkzSVmuw

@Gozala
Copy link
Contributor Author

Gozala commented Jan 9, 2020

I suppose that is their embedding https://github.com/hypothesis/pdf.js-hypothes.is

@Gozala
Copy link
Contributor Author

Gozala commented Jan 10, 2020

I made some changes to try and follow instructions for cmaps here
https://www.npmjs.com/package/react-pdf

diff --git a/webpack.config.ts b/webpack.config.ts
index ab3abd69..bb1c18ff 100644
--- a/webpack.config.ts
+++ b/webpack.config.ts
@@ -239,6 +239,12 @@ export default [
         chunks: ['background'],
         filename: 'background.html',
       }),
+      new CopyPlugin([
+        {
+          from: 'node_modules/pdfjs-dist/cmaps/',
+          to: 'assets/pdfjs-cmaps/',
+        },
+      ]),
       ...(isDev
         ? [
             new HardSourcePlugin({
diff --git a/src/renderer/components/content-types/files/PdfContent.tsx b/src/renderer/components/content-types/files/PdfContent.tsx
index bcf633ce..ac0d1907 100644
--- a/src/renderer/components/content-types/files/PdfContent.tsx
+++ b/src/renderer/components/content-types/files/PdfContent.tsx
@@ -118,7 +118,14 @@ export default function PdfContent(props: ContentProps) {
     <>
       {header}
       {buffer ? (
-        <Document file={{ data: buffer }} onLoadSuccess={onDocumentLoadSuccess}>
+        <Document
+          file={{ data: buffer }}
+          onLoadSuccess={onDocumentLoadSuccess}
+          options={{
+            cMapUrl: '/assets/pdfjs-cmaps/',
+            cMapPacked: true,
+          }}
+        >
           <Page
             loading=""
             pageNumber={pageNum}

But I see no requests for cmaps and issues still seem to be present.

@Gozala
Copy link
Contributor Author

Gozala commented Jan 10, 2020

This seems relevant wojtekmaj/react-pdf#275

@Gozala
Copy link
Contributor Author

Gozala commented Jan 10, 2020

Tried following to see if the problem was with electron:

document.documentElement.innerHTML = "<iframe style='width: 100%;height:100%;top:0;left:0;position:fixed;'src='https://via.hypothes.is/https://tools.ietf.org/pdf/rfc2557.pdf#annotations:zUk5UDM2Eeq1FYMkzSVmuw'>"

Seems to work as expected and there seems to be no requests to cmap files

@Gozala
Copy link
Contributor Author

Gozala commented Jan 10, 2020

I am now suspecting that hypothesis might be doing this
mozilla/pdf.js#4244 (comment)

@Gozala
Copy link
Contributor Author

Gozala commented Jan 10, 2020

Surprisingly setting a following setting disableFontFace: false makes it work. I don't see a way to switch pages but that might be unrelated.

@Gozala
Copy link
Contributor Author

Gozala commented Jan 10, 2020

This does not seem ideal as per docs https://mozilla.github.io/pdf.js/api/draft/module-pdfjsLib.html

By default fonts are converted to OpenType fonts and loaded via font face rules. If disabled, fonts will be rendered using a built-in font renderer that constructs the glyphs with primitive path commands. The default value is false.

@Gozala
Copy link
Contributor Author

Gozala commented Jan 10, 2020

Some poking around debugger also showed that font that causes error is 'Courier' with 'monospace' fallback. However there seems to be no compiled glimpse and that is why those errors are logged.

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

No branches or pull requests

2 participants