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

Move the Preferences initialization/fetching code to the top of PDFViewerApplication.initialize, and add a enhanceTextSelection preference to the viewer #7586

Merged
merged 2 commits into from
Sep 3, 2016

Conversation

Snuffleupagus
Copy link
Collaborator

@Snuffleupagus Snuffleupagus commented Sep 2, 2016

Please refer to the individual commit messages.

This PR should make it easier to switch the text-selection modes, e.g. for someone working on the tasks in issue #7584. It should also simplify things for third-party implementers of the default viewer, since this allows you to choose text-selection mode without having to edit the code.

/cc @yurydelendik You suggested on IRC that we move the Preferences initialization, so I'm submitting this for feedback.

Edit: Probably easier reviewing with https://github.com/mozilla/pdf.js/pull/7586/files?w=1.

…FViewerApplication.initialize`, to enable using them when initializing the viewer components
@Snuffleupagus
Copy link
Collaborator Author

/botio-linux preview

@pdfjsbot
Copy link

pdfjsbot commented Sep 2, 2016

From: Bot.io (Linux)


Received

Command cmd_preview from @Snuffleupagus received. Current queue size: 0

Live output at: http://107.21.233.14:8877/38926ca0047cabb/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Sep 2, 2016

From: Bot.io (Linux)


Success

Full output at http://107.21.233.14:8877/38926ca0047cabb/output.txt

Total script time: 1.02 mins

Published

@timvandermeij timvandermeij self-assigned this Sep 2, 2016
@timvandermeij timvandermeij merged commit 461a18a into mozilla:master Sep 3, 2016
@timvandermeij
Copy link
Contributor

Thanks!

@Snuffleupagus
Copy link
Collaborator Author

Unfortunately it seems that this actually caused issues, e.g. with the localized event in the Firefox addon firing before the handler is ready, since it seems that we have a bunch of code that relies on certain PDFViewerApplication properties being synchronously available.
I cannot see an immediate fix for these issues, so I'm afraid that we have to back this out for now since finding a proper solution might be trickier than I first imaged.

My apologies for not testing this thoroughly before submitting the PR!

@timvandermeij
Copy link
Contributor

I'll check to see how we can revert the commit.

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

Successfully merging this pull request may close these issues.

3 participants