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

[GENERIC viewer] Export some viewer constants in the default viewer (issue 15294) #15300

Merged
merged 2 commits into from
Aug 13, 2022

Conversation

Snuffleupagus
Copy link
Collaborator

This exports the same constants as the viewer components, but in the default viewer. To avoid bloating the global-scope the constants are added to a new PDFViewerApplicationConstants object[1], which also allows us to skip this in builds where it's not actually needed (e.g. the Firefox built-in PDF Viewer).

Please note: I'm not completely sold on this idea, and thus wouldn't mind the patch being rejected, since we probably don't want to export every single viewer constant this way. (And it may seem a bit arbitrary, to users, why some constants are exported and others are not.)


[1] Somewhat similar to the existing PDFViewerApplicationOptions structure.

In addition to the existing `LinkTarget` constant, used by the `PDFLinkService`-constructor, this patch exports the following constants in the viewer components:
 - `ScrollMode` and `SpreadMode`, since the `BaseViewer` has getters/setters which work with those constants.
 - `RenderingStates`, since that one may be helpful when using `PDFPageView` directly.
…issue 15294)

This exports the same constants as the viewer components, but in the default viewer. To avoid bloating the global-scope the constants are added to a new `PDFViewerApplicationConstants` object[1], which also allows us to skip this in builds where it's not actually needed (e.g. the Firefox *built-in* PDF Viewer).

*Please note:* I'm not completely sold on this idea, and thus wouldn't mind the patch being rejected, since we probably don't want to export every single viewer constant this way. (And it may seem a bit arbitrary, to users, why some constants are exported and others are not.)

---
[1] Somewhat similar to the existing `PDFViewerApplicationOptions` structure.
@Snuffleupagus
Copy link
Collaborator Author

/botio integrationtest

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

Live output at: http://54.193.163.58:8877/0aeaca5d7c58f6d/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.241.84.105:8877/0be0d5a278a8526/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/0be0d5a278a8526/output.txt

Total script time: 4.86 mins

  • Integration Tests: Passed

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/0aeaca5d7c58f6d/output.txt

Total script time: 8.81 mins

  • Integration Tests: FAILED

@timvandermeij timvandermeij merged commit b040d64 into mozilla:master Aug 13, 2022
@timvandermeij
Copy link
Contributor

I can see a use case for this, so let's do it. Thank you!

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.

Expose named constants as globals or properties on PDFViewerApplication
3 participants