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

Remove the ability to pass a scale parameter in the (optional) args object parameter of PDFViewerApplication.open(file, args) #8820

Merged
merged 1 commit into from
Aug 24, 2017

Conversation

Snuffleupagus
Copy link
Collaborator

Since the very early days of the viewer, it's been possible to pass in a scale when opening a PDF file. However, most of the time it was/is actually being ignored, which limits its usefulness considerably.

In older versions of the viewer, if a document hash was present (i.e. PDFViewerApplication.initialBookmark being set) or if the document existed in the ViewHistory, the scale passed to PDFViewerApplication.open would thus always be ignored.
In addition to the above, in the current viewer there's even more cases where the scale parameter will be ignored: if a (valid) browser history entry exists on document load, or if the defaultZoomValue preference is set to a non-default value.
Hence the result is that in most situation, a scale passed to PDFViewerApplication.open will be completely ignored.

A much better, not to mention supported, way of setting the initial scale is by using the defaultZoomLevel preference. In comparision, this also has the advantage of being used in situations where the scale would be ignored.

All in all this leads to the current situation where we have code which is essentially dead, since no part of the viewer (by default) relies on it.
To clean up this code, and to avoid having to pass (basically) unused parameters around, I'd thus like to remove the ability to pass a scale to PDFViewerApplication.open.

…s` object parameter of `PDFViewerApplication.open(file, args)`

Since the very early days of the viewer, it's been possible to pass in a `scale` when opening a PDF file. However, most of the time it was/is actually being ignored, which limits its usefulness considerably.

In older versions of the viewer, if a document hash was present (i.e. `PDFViewerApplication.initialBookmark` being set) or if the document existed in the `ViewHistory`, the `scale` passed to `PDFViewerApplication.open` would thus always be ignored.
In addition to the above, in the current viewer there's even more cases where the `scale` parameter will be ignored: if a (valid) browser history entry exists on document load, or if the `defaultZoomValue` preference is set to a non-default value.
Hence the result is that in most situation, a `scale` passed to `PDFViewerApplication.open` will be completely ignored.

A much better, not to mention supported, way of setting the initial scale is by using the `defaultZoomLevel` preference. In comparision, this also has the advantage of being used in situations where the `scale` would be ignored.

All in all this leads to the current situation where we have code which is essentially dead, since no part of the viewer (by default) relies on it.
To clean up this code, and to avoid having to pass (basically) unused parameters around, I'd thus like to remove the ability to pass a `scale` to `PDFViewerApplication.open`.
@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/b9ff794bc36b351/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

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

Total script time: 2.34 mins

Published

@timvandermeij timvandermeij merged commit 26c4536 into mozilla:master Aug 24, 2017
@timvandermeij
Copy link
Contributor

I agree completely with the reasoning here. Thank you for cleaning this up!

@Snuffleupagus Snuffleupagus deleted the viewer-open-rm-scale branch August 25, 2017 08:00
movsb pushed a commit to movsb/pdf.js that referenced this pull request Jul 14, 2018
Remove the ability to pass a `scale` parameter in the (optional) `args` object parameter of `PDFViewerApplication.open(file, args)`
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