Skip to content

Improve how the PDF.js version/commit information is exposed in the *built* files #19956

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

Merged
merged 1 commit into from
May 22, 2025

Conversation

Snuffleupagus
Copy link
Collaborator

@Snuffleupagus Snuffleupagus commented May 19, 2025

To make it easier to tell which PDF.js version/commit that the built files correspond to, they have (since many years) included pdfjsVersion and pdfjsBuild constants with that information.

As currently implemented this has a few shortcomings:

  • It requires manually adding the code, with its preprocessor statements, in all relevant files.

  • It requires ESLint disable statements, since it's obviously unused code.

  • Being unused, this code is removed in the minified builds.

  • This information would be more appropriate as comments, however Babel discards all comments during building.

  • It would be helpful to have this information at the top of the built files, however it's being moved during building.

To address all of these issues, we'll instead utilize Webpack to insert the version/commit information as a comment placed just after the license header.

@Snuffleupagus Snuffleupagus changed the title Improve the PDF.js version/commit information is exposed in the *built* files Improve how the PDF.js version/commit information is exposed in the *built* files May 19, 2025
…built* files

To make it easier to tell which PDF.js version/commit that the *built* files correspond to, they have (since many years) included `pdfjsVersion` and `pdfjsBuild` constants with that information.

As currently implemented this has a few shortcomings:
 - It requires manually adding the code, with its preprocessor statements, in all relevant files.

 - It requires ESLint disable statements, since it's obviously unused code.

 - Being unused, this code is removed in the minified builds.

 - This information would be more appropriate as comments, however Babel discards all comments during building.

 - It would be helpful to have this information at the top of the *built* files, however it's being moved during building.

To address all of these issues, we'll instead utilize Webpack to insert the version/commit information as a comment placed just after the license header.
@Snuffleupagus
Copy link
Collaborator Author

/botio-linux test

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.241.84.105:8877/236d97279486376/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/236d97279486376/output.txt

Total script time: 30.09 mins

  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: Passed

@Snuffleupagus Snuffleupagus added the release-blocker Blocker for the upcoming release label May 20, 2025
Copy link
Contributor

@calixteman calixteman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you.

@Snuffleupagus Snuffleupagus removed the request for review from timvandermeij May 22, 2025 13:23
@Snuffleupagus Snuffleupagus merged commit d81174d into mozilla:master May 22, 2025
9 checks passed
@Snuffleupagus Snuffleupagus deleted the versionInfoHeader branch May 22, 2025 13:24
toofar added a commit to qutebrowser/qutebrowser that referenced this pull request Jun 2, 2025
It's changed in this PR: mozilla/pdf.js#19956
to have the version string as a comment in the file, instead of the
variable.

Making the regex more forgiving increases the chance of matching on the
wrong string on a past or future version. I've only tested with the
previous version (v5.2.133) and it seemed to still work there.

Changes I made to the regex:

* add the literal * to the match group of possible prefixes of pdfjsVersion
* make the quotes around the version optional
* make the semicolon at the end of the line optional
* add newline to the list of characters that can terminate the match
  group (otherwise it was matching across the end of the line up to the
  first string, kinda odd when there was a $ after the match group)
toofar added a commit to qutebrowser/qutebrowser that referenced this pull request Jun 2, 2025
It's changed in this PR: mozilla/pdf.js#19956
to have the version string as a comment in the file, instead of the
variable.

Making the regex more forgiving increases the chance of matching on the
wrong string on a past or future version. I've only tested with the
previous version (v5.2.133) and it seemed to still work there.

Changes I made to the regex:

* add the literal * to the match group of possible prefixes of pdfjsVersion
* make the quotes around the version optional
* make the semicolon at the end of the line optional
* add newline to the list of characters that can terminate the match
  group (otherwise it was matching across the end of the line up to the
  first string, kinda odd when there was a $ after the match group)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
other release-blocker Blocker for the upcoming release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants