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

<object> / <embed> support in Chrome / Opera #4549

Merged
merged 1 commit into from
May 14, 2014

Conversation

Rob--W
Copy link
Member

@Rob--W Rob--W commented Apr 1, 2014

This PR brings support for embedded PDFs of the following forms:

<object type="application/pdf" data="...">
<embed type="application/pdf" src="...">
<object data="....pdf"> <!-- note: no type specified -->
<embed src="....pdf"> <!-- note: no type specified -->

Fixes operasoftware#19
Fixes #3736
Supersedes #4194

Tests

To check whether the feature works as intended, visit the following pages, and check whether the PDF viewer is rendered:

@Rob--W
Copy link
Member Author

Rob--W commented Apr 18, 2014

I will revisit this PR, and try to make it even more performant.

Without the patch, the second DOM benchmark at http://dromaeo.com/?dom-mod peeks around 700-900MB memory. With the patch, it triples to ~2.2 GB, and occasionally freezes for a second due to Garbage collection.

A quick test with a mutation observer and a no-op callback shows that the use of mutation observer is not necessarily the bottleneck.

@Rob--W
Copy link
Member Author

Rob--W commented May 5, 2014

@yurydelendik
Could you review this PR? I switched to a different method that does not have any negative impact on the performance of DOM.

@Rob--W
Copy link
Member Author

Rob--W commented May 14, 2014

@timvandermeij I want to prepare an update of the Chrome extension. Could you test this PR and merge it?

timvandermeij added a commit that referenced this pull request May 14, 2014
<object> / <embed> support in Chrome / Opera
@timvandermeij timvandermeij merged commit 6d33025 into mozilla:master May 14, 2014
@timvandermeij
Copy link
Contributor

Nice work, thanks!

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 this pull request may close these issues.

Unable to show embedded PDF files PDF.js does not work in <embed> tags in Chromium extension
2 participants