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

Add pdf viewer #2867

Merged
merged 10 commits into from Aug 21, 2017
Merged

Add pdf viewer #2867

merged 10 commits into from Aug 21, 2017

Conversation

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented Aug 17, 2017

Adds a mimerenderer extension for embedded PDFs, addressing #670

I have tested it in Firefox and Chrome for Linux. Windows tests forthcoming.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 17, 2017

I'm getting a blank div in Chrome 60.0.3112.90 on OSX 10.12.6. Firefox is working though.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 17, 2017

Weird, I tried a different PDF and it worked. I'll try a few more.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 17, 2017

I'm not sure I can see a pattern. I loaded a 158 page dissertation with equations and figures but other things do not work like http://do1.dr-chuck.com/pythonlearn/EN_us/pythonlearn.pdf.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 17, 2017

It must be a limitation of the inline src handling, this works for instance:

<iframe class="jp-PDFViewer" type="application/pdf" src="http://localhost:8890/foo/files/Python.pdf">placeholder</iframe>

But that doesn't help us when we have just the content.

@ian-r-rose
Copy link
Member Author

@ian-r-rose ian-r-rose commented Aug 17, 2017

Hmm, I am not sure how to move forward with that. I am also seeing flakiness with Chrome on Windows, especially for large pdfs (like the one you linked). Perhaps there is some limit to the buffer size of data urls?

@ellisonbg
Copy link
Contributor

@ellisonbg ellisonbg commented Aug 17, 2017

@ian-r-rose
Copy link
Member Author

@ian-r-rose ian-r-rose commented Aug 17, 2017

Perhaps we can add an URL field to the IMimeModel interface, something like

interface IMimeModel {
  readonly trusted: boolean;
  readonly data: ReadonlyJSONObject;
  readonly urls: ReadonlyJSONObject;
  readonly metadata: ReadonlyJSONObject;
  setData(options: IMimeModel.ISetDataOptions): void;
}

And we could then access the URL with something like

if (mimemodel.urls[MIME_TYPE]) {
  url = mimemodel.urls[MIME_TYPE]
} else {
  url = `data:${MIME_TYPE};base64,${data}`;
}
<embed src=url ... />

@ellisonbg
Copy link
Contributor

@ellisonbg ellisonbg commented Aug 17, 2017

@ian-r-rose
Copy link
Member Author

@ian-r-rose ian-r-rose commented Aug 18, 2017

After some snooping around, it seems that there are typically size limits for data URIs, but they are not well documented or stable between browser versions (but a limit of around a few megabytes seems common). A more robust way of referring to larger data is to use an objectUrl, created using URL.createObjectURL()

I have switched to doing that, and in my testing this seems more robust. It works with larger PDFs on Chrome and Firefox, both with Linux and Windows. I am not able to test Mac OS at the moment. Could somebody test that for me?

If this is sufficient, the changes to the IMimeModel interface seem less pressing, though we may still want to do that for other reasons.

@ian-r-rose ian-r-rose added this to the 0.27 milestone Aug 18, 2017
Copy link
Contributor

@ellisonbg ellisonbg left a comment

Tested this locally with about a dozen different PDF files and it seems to work great. Also tested with Matplotlib. Honestly, it is amazing! The code looks solid. I am +1 with us merging and iterating, but will let others review/comment first.

@ellisonbg
Copy link
Contributor

@ellisonbg ellisonbg commented Aug 20, 2017

I should note that I did my tests on a Mac, latest Chrome and Firefox worked fine.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 21, 2017

Thanks!

@blink1073 blink1073 merged commit e7c2a82 into jupyterlab:master Aug 21, 2017
2 checks passed
@ian-r-rose ian-r-rose mentioned this pull request Aug 21, 2017
@oschulz
Copy link

@oschulz oschulz commented Aug 21, 2017

Thanks!

@davidchall
Copy link

@davidchall davidchall commented Aug 23, 2017

This doesn't seem to work in Safari on my Mac (works just fine in Chrome though!)

@joshjob42
Copy link

@joshjob42 joshjob42 commented Sep 11, 2017

Also doesn't work in Safari Technology Preview or the Safari for High Sierra beta, alas. Sucks for mac owners as other browsers drain the battery a lot by comparison.

@idaho-hewitts
Copy link

@idaho-hewitts idaho-hewitts commented Jul 22, 2018

For PDFs opened from the Jupiter file browser, no renderer should be needed for most browsers, since they can natively render PDF files. However, the MIME type must be set properly when the file is served up as per this stack overflow article. I recently fixed a similar problem in a Python web server.

I am teaching an introductory high-school programming class using Jupyter. I have placed the textbook as a PDF alongside the Jupyter notebooks, but when I attempt to open it from the Jupyter file browser, it shows a black window. I can open the file just fine in the browser if I open it manually with File->Open. It sure would be nice for my students if they could just click on the textbook.

@sushmit86
Copy link

@sushmit86 sushmit86 commented Apr 25, 2019

I am having an issue while opening pdf files, Every time I go back to the pdf tab it reverts to the first page. Seems like the pdf gets reloaded once again. Is this an expected behavior?

@ian-r-rose
Copy link
Member Author

@ian-r-rose ian-r-rose commented Apr 25, 2019

Hi @sushmit86, this is a known issue and is being tracked in #5714

@lock lock bot locked as resolved and limited conversation to collaborators Aug 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

8 participants