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

Ensure that the same exact version of PDF.js is used in both the API and the Worker #8959

Merged
merged 1 commit into from
Sep 27, 2017
Merged

Ensure that the same exact version of PDF.js is used in both the API and the Worker #8959

merged 1 commit into from
Sep 27, 2017

Conversation

Snuffleupagus
Copy link
Collaborator

I don't have a good example at hand right know, but I recall seeing custom deployments of PDF.js that bundle a specific version of the build/pdf.js file and then set PDFJS.workerSrc to point to https://mozilla.github.io/pdf.js/build/pdf.worker.js.
That practice seems really bad since, besides (obviously) causing unnecessary server load, it will very quickly result in a version mismatch between the pdf.js and pdf.worker.js files in those PDF.js deployments.
Such a version mismatch could easily lead to either breaking errors, or even worse slightly inconsistent behaviour for an API call (if the API -> Worker interface changes, which does happen from time to time).

To avoid the problems described above, I'm thus proposing that we enforce that the versions of the pdf.js and pdf.worker.js files must always match.

…and the Worker

I don't have a good example at hand right know, but I recall seeing custom deployments of PDF.js that bundle a *specific* version of the `build/pdf.js` file and then set `PDFJS.workerSrc` to point to https://mozilla.github.io/pdf.js/build/pdf.worker.js.
That practice seems really bad since, besides (obviously) causing unnecessary server load, it will very quickly result in a version mismatch between the `pdf.js` and `pdf.worker.js` files in those PDF.js deployments.
Such a version mismatch could easily lead to either breaking errors, or even worse slightly inconsistent behaviour for an API call (if the API -> Worker interface changes, which does happen from time to time).

To avoid the problems described above, I'm thus proposing that we enforce that the versions of the `pdf.js` and `pdf.worker.js` files must always match.
@Snuffleupagus
Copy link
Collaborator Author

/botio test

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.67.70.0:8877/da56ca07c857446/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

Live output at: http://54.215.176.217:8877/0b165bbdfc10bec/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/da56ca07c857446/output.txt

Total script time: 16.52 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: Passed

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://54.215.176.217:8877/0b165bbdfc10bec/output.txt

Total script time: 22.81 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: Passed

@Snuffleupagus
Copy link
Collaborator Author

/botio-linux preview

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.67.70.0:8877/33e4e394e554ea2/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/33e4e394e554ea2/output.txt

Total script time: 2.46 mins

Published

@Snuffleupagus Snuffleupagus merged commit 25806d1 into mozilla:master Sep 27, 2017
@Snuffleupagus
Copy link
Collaborator Author

@yurydelendik Thank you for the review :-)

@Snuffleupagus Snuffleupagus deleted the api-worker-version-check branch September 27, 2017 16:55
movsb pushed a commit to movsb/pdf.js that referenced this pull request Jul 14, 2018
…-check

Ensure that the same exact version of PDF.js is used in both the API and the Worker
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