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

Allow experimenting with the printResolution AppOption when printing with the built-in Firefox version #10898

Merged

Conversation

Snuffleupagus
Copy link
Collaborator

As have already been stated multiple times, simply increasing the printing resolution may have undesirable effects on both memory usage and general performance. Hence why PR #10854 did not add a preference, and only exposed AppOption by default in GENERIC builds for now.

However, considering how differently printing works in the built-in Firefox version (with mozPrintCallback) compared to the general default viewer, any testing done in the latter case might not be completely relevant to the first (and most important) case of the Firefox PDF Viewer.

Note that considering the implementation of AppOptions.get, this patch will be safe and should allow experimenting with printResolution in all builds of the default viewer[1]. By not, however, having printResolution appear in AppOptions for either the MOZCENTRAL or CHROMIUM build targets, there should be no indication of official support for now.
Furthermore, it shouldn't be a preference at this point in time (or even at all), since that makes it too easy for users to change it permanently[2] and possible "break" printing.


[1] By running PDFViewerApplicationOptions.get('printResolution', /* value here */); in the console after the viewer loads.

[2] I've seen Firefox bugs, filed in Bugzilla, where users modified e.g. preferences manually in about:config and then some time later (maybe months) wondered why something was suddenly broken. In those cases, trying to work out that a preference change was the culprit can take time/effort.

…g with the built-in Firefox version

As have already been stated multiple times, simply increasing the printing resolution may have undesirable effects on both memory usage *and* general performance. Hence why PR 10854 did *not* add a preference, and only exposed AppOption by default in `GENERIC` builds for now.

However, considering how differently printing works in the built-in Firefox version (with `mozPrintCallback`) compared to the general default viewer, any testing done in the latter case might not be completely relevant to the first (and most important) case of the Firefox PDF Viewer.

Note that considering the implementation of `AppOptions.get`, this patch will be safe and should allow experimenting with `printResolution` in all builds of the default viewer[1]. By not, however, having `printResolution` appear in AppOptions for either the `MOZCENTRAL` or `CHROMIUM` build targets, there should be no indication of official support for now.
Furthermore, it shouldn't be a preference at this point in time (or even at all), since that makes it too easy for users to change it permanently[2] and possible "break" printing.

---

[1] By running `PDFViewerApplicationOptions.get('printResolution', /* value here */);` in the console after the viewer loads.

[2] I've seen Firefox bugs, filed in Bugzilla, where users modified e.g. preferences manually in `about:config` and then some time later (maybe months) wondered why something was suddenly broken. In those cases, trying to work out that a preference change was the culprit can take time/effort.
@timvandermeij
Copy link
Contributor

/botio-linux preview

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

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

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

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

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

Total script time: 1.85 mins

Published

@timvandermeij timvandermeij merged commit 7348275 into mozilla:master Jun 11, 2019
@timvandermeij
Copy link
Contributor

Looks like a good idea; thanks!

@Snuffleupagus Snuffleupagus deleted the firefox-print-resolution branch June 12, 2019 06:33
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