WIP: reimplement PDF viewer #17163
Closed
WIP: reimplement PDF viewer #17163
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
jkleinsc
force-pushed
the
add-pdfium-viewer
branch
from
7372a26
to
b243627
Compare
jkleinsc
force-pushed
the
add-pdfium-viewer
branch
from
3581f45
to
1a6d911
Compare
jkleinsc
force-pushed
the
add-pdfium-viewer
branch
2 times, most recently
from
1fcb5e1
to
5c6d014
Compare
This comment has been minimized.
This comment has been minimized.
Include extensions dep Add AtomExtensionsRenderClient Add AtomExtensionsClient Dispatch scripts on document events Add ExtensionsAPIProvider Fix compilation errors Add AtomExtensionsBrowserClient Add remaining browser extensions boilerplate Add 'load-extension' command line switch to test loading extensions Fix ExtensionPrefs runtime error Fix content::BrowserContext::SetCorsOriginAccessListForOrigin not implemented error Register 'content_scripts' manifest feature Register extension dispatcher callbacks Move extensions files into /extensions subdirectory Initialize SharedUserScriptMaster Fix content script injection Add pref_registry dep for PrefRegistrySyncable Add disabled-by-default ENABLE_ELECTRON_EXTENSIONS buildflag Fix preprocessor lint errors with extension additions Fix build error with including extensions generated code Conditionally compile extension code files Move prefs deps behind buildflag Ignore gn check on extensions deps Rename all 'shell_' files Rename Shell to Atom Fix lint issues with renamed Shell code Fix compile errors
jkleinsc
force-pushed
the
add-pdfium-viewer
branch
from
30f29df
to
6afacac
Compare
|
What happened with this pull request? I really hoped this will be reimplemented. |
|
We're still intending to one day restore the PDF viewer, but it relies on us first migrating to use Chrome's extensions library instead of our own shim, as the PDF viewer in Chromium is implemented as an extension. |
|
+1 This is killing me, the flicker issues on electron 2.x make it almost unusable but our need to iframe pdf in app is critical to the UX so we are stuck on electron 2.x. And not able to be on 4.x+ Not getting pdf iframe support continues to keep getting the kick the can down the road... Is the PR submission here not usable? I don't know the Electron internals (yet). Pardon my late comment, but I have been following this for 16 months. Without knowing yet the code required to do this, I get it's easy to sit on the sideline and wish for a solution.
Thx, in advance. |
|
Is this something the community could find sponsors for? I understand if the change required a more in-depth architectural change that does not lend itself to something like this, but I think many are stuck with an incomplete solution at present.
…Sent from my iPhone
On Dec 2, 2019, at 3:01 AM, maestrocoder ***@***.***> wrote:
+1 This is killing me, the flicker issues on electron 2.x make it almost unusable usages but iframe for pdf is critical to my app UX so we are stuck on electron 2.x.
Not getting pdf iframe support continues to keep getting the kick the can down the road...
Is the PR submission here not usable? I don't know the Electron internals (yet).
Pardon my late comment, but I have been following this for 16 months. Without knowing yet the code required to do this, I get it's easy to sit on the sideline and wish for a solution.
What is the estimated level of effort to first migrate to Chrome extension library?
Is that migration required for other requirements? If so, what branch(es)?
Is the migration to Chrome extension library already schedule? Estimated completion date?
Can we get a downstream branch off any partial work on the Chrome extension library? Is there already a Chrome extension library branch in progress?
Thx, in advance.
-m3
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
@namotco There is already a BountySource for this feature: https://www.bountysource.com/issues/56240517-enable-pdf-viewer (source: #12337 (comment)) |
|
why was the branch deleted? |
|
@aimergenge The PR was closed as it's blocked by #19447 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Change
The PDF viewer stopped working in 3-0-x due to changes in Chromium. This PR will reimplement the PDF viewer functionality trying to stay as close as possible to the Chromium implementation.
Resolves #12337
Checklist
npm testpassesRelease Notes
Notes: