This is a regression from PR 8993; it causes the pages to be offset horizontally in Presentation Mode, if and only if the sidebar is currently open when the user triggers Presentation Mode.
Please note that while this doesn't seem to affect Firefox, both Chrome and IE are however affected.
Interestingly enough, despite the Chrome extension being affected as well, I cannot find any issue filed about this. (Either Presentation Mode isn't used much at all, or users don't open the sidebar before entering Presentation Mode.)
It appears that Microsoft silently fixed the problem that required disabling of fullscreen mode, in e.g. `iframe`s, in IE 11; please see issue 4711 and PR 5525 for historical context.
Unfortunately my Google-fu isn't strong enough to find any *official* information regarding the fixing of the browser bug in IE. However testing of the default viewer in IE 11, with this patch applied, it now appears that Presentation Mode is working correctly even in an `iframe` in IE 11.
Further anecdotal evidence that the bug is in fact fixed, is for example that jQuery previously contained a work-around for the IE bug. However, that's removed over two years ago now; see ff1a0822f7 and the issues referenced there.
Given that the default viewer isn't intended to be used as-is anyway (in custom deployments), it didn't seem necessary to keep the `disableFullscreen` option around since it was *only* ever added for compatibility purposes.
Fixes 9585.
Not only is the `Util.loadScript` helper function unused on the Worker side, even trying to use it there would throw an Error (since `document` isn't defined/available in Workers).
Hence this helper function is moved, and its code modernized slightly by having it return a Promise rather than needing a callback function.
Finally, to reduced code duplication, the "new" loadScript function is exported and used in the viewer.
This should provide a better out-of-the-box experience when using PDF.js in a Node.js environment, since it's missing native support for both `@font-face` and `Image`.
Please note that this change *only* affects the default values, hence it's still possible for an API consumer to override those values when calling `getDocument`.
Also, prevents "ReferenceError: document is not defined" errors, when running the unit-tests in Node.js/Travis.
This commit adds `scrollModeOnLoad` and `spreadModeOnLoad` preferences
that control the default viewer state when opening a new document for
the first time.
This commit also contains a minor refactoring of some of the option UI
rendering code in extensions/chromium/options/options.js, as I couldn't
bear creating two more functions nearly identical to the four that
already existed.
This builds on the scrolling mode work to add three buttons for joining
page spreads together: one for the default view, with no page spreads,
and two for spreads starting on odd-numbered or even-numbered pages.
Prior to this commit, if the vertical scroll bar is absent and the horizontal
scroll bar is present, a link to a particular point on the page which should
induce a horizontal scroll did not do so, because the absence of a vertical
scroll bar meant that the viewer was not recognized as the nearest scrolling
ancestor. This commit adds a check for horizontal scroll bars when searching
for the scrolling ancestor.
*This is a final piece of clean-up of code that I recently wrote, after which I'm done :-)*
When the `getMainThreadWorkerMessageHandler` function was added, in PR 9385, it did so by basically introducing a `web/app.js` dependency in `src/display/api.js` through the `window.pdfjsNonProductionPdfWorker` property[1]. Even though this is limited to non-`PRODUCTION` mode, i.e. `gulp server`, it still seems unfortunate to have that sort of viewer dependency in the API code itself.
With the new, much nicer and shorter, names introduced in PR 9565 we can remove this non-`PRODUCTION` hack and just use `window.pdfjsWorker` in both the viewer and the API regardless of the build mode.
---
[1] It didn't seem correct to piggy-back on the `window.pdfjsDistBuildPdfWorker` property in non-`PRODUCTION` mode.
Please note that this patch *purposely* doesn't add every standard (or semi-standard) page name in existence, but rather only a few common ones. This is done to lessen the burden on localizers, since it's quite possible that all of the page names could need translation (depending on locale).
It's easy to add more standard page sizes in the future, but we should take care to *only* add those that are very commonly used in actual PDF files.
This uses a whitelist, based on the locale, to determine where non-metric units should be used.
Note that the behaviour implemented here seem consistent with desktop PDF viewers (e.g. Adobe Reader), where the pageSizes are *always* displayed with locale dependent units rather than pageSize dependent ones (since the latter would probably be quite confusing).
A couple of basic unit-tests are added, and a manual `isLandscape` check (in `web/base_viewer.js`) is also converted to use the helper function instead.
Test case to exercise the different encodings:
1. Create a file "some file#@%M<br>%25 .pdf"
2. Build the extension with `gulp chromium` and load it in Chrome.
3. Go to `chrome://extensions/` and ensure that the
"Allow access to file URLs" is disabled.
4. Try to open the file from step 1 in Chrome (maybe reload once).
5. PDF.js should be showing a file chooser button.
6. Click on that button and select a different file.
Test: Check that a confirmation dialog pops up that warns about
a different file name. Cancel the dialog.
7. Click on the button again and select the original file.
Test: Check that the file opens as expected.
The units are currently repeated after each dimension, which seems unnecessary and is also not done in other PDF viewers (such as e.g. Adobe Reader).
Furthermore, the name of the l10n arguments can be simplified slightly, since the name of the strings themselves should be enough information.
Finally, the `width`/`height` should be formatted according to the current locale, as is already done for other strings in the document properties dialog.
The `getPageSizeInches` method was implemented on `PDFDocumentProxy`, which seems conceptually wrong since the size property isn't global to the document but rather specific to each page. Hence the method is moved into `PDFPageProxy`, as `get pageSizeInches` instead to address this.
Despite the fact that new API functionality was implemented, no unit-tests were added. To prevent issues later on, we should *always* ensure that new functionality has at least some test-coverage; something that this patch also takes care of.
The new `PDFDocumentProperties._parsePageSize` method seemed unnecessary convoluted. Furthermore, in the "no data provided"-case it even returned incorrect data (an array, rather than the expected object).
Finally, the fallback strings didn't actually agree with the `en-US` locale. This inconsistency doesn't look too great, and it's thus addressed here as well.
PR #9493 moved from `appConfig.defaultUrl` to `AppOptions.get('defaultUrl')`.
However, it forgot to replace `appConfig.defaultUrl` in chromecom.js,
and as a result the extension is not able to open any PDF file.
This patch fixes that issue.
One additional complication with removing this option from the global `PDFJS` object, is that the viewer currently needs to check `disableAutoFetch` in a couple of places. To address this I'm thus proposing adding a getter in `PDFDocumentProxy`, to allow checking the *actually* used values for a particular `getDocument` invocation.
The `appConfig` contains (mostly) references to various DOM elements, used when initializing the viewer components.
Hence `defaultUrl` seem like a slightly better fit for the new `AppOptions` abstraction, not to mention that it should thus be easier to set/modify it for custom deployments of the default viewer.
The way that various options are handled in the default viewer is currently a bit of a mess (to say the least). Some viewer options reside in the global `PDFJS` object, while others reside in `Preferences`. To make matters worse, some options even exist in both of the two.
Since the goal, with PDF.js version `2.0`, is to reduce our usage of the global `PDFJS` object, we'll instead want pass in the options when initializing the viewer components and when calling API methods (such as `getDocument`).
However given the current state of things in the default viewer, this wouldn't be exactly easy to implement. Hence this patch, which attempts to consolidate the way that viewer (and later API) options are handled by introducing a `AppOptions` singleton that provides *one* centralized way of interacting with the various options in the default viewer.
In a1cfa5f4d7, the textLayerMode
preference was introduced, to replace the disableTextLayer and
enhanceTextSelection preferences.
As a result, the text selection preference was no longer visible
in Chrome (because preferences are only rendered by default for
boolean preferences, not for enumerations).
This commit adds the necessary bits to
extensions/chromium/options/options.{html,js}
so that the textLayerMode preference can be changed again.
Also, migration logic has been added to move over preferences
from the old to the new names:
- In web/chromecom.js, the logic is added to translate
preferences that were set by an administrator (it is read-only,
so this layer is unavoidable).
- In extensions/chromium/options/migration.js, similar logic is
added, except in this case the preference storage is writable,
so this migration logic happens only once.
The "enhanced text selection" mode is still experimental, so it
has been marked as experimental to signal that there may be bugs.
The list of tasks that block promotion to stable is at #7584.
This partially reverts df0836b9b8.
The entry in preferences_schema.json is restored because that is
required to make managed preferences visible to the extension code.
The default key is still removed from default_preferences.json,
because this change only concerns the Chrome extension, not the
other parts of PDF.js. To account for the missing key, the
deprecated key was added back in chromecom.js
The key needs to be restored in preferences_schema.json too,
because that's the only way to make managed preferences visible.
I'm using `Object.assign`, which was introduced in Chrome 45,
so the preference module will break in Chrome 45 and earlier.
This is fine, because we do not support Chrome before 49.
In order to simplify things, the undocumented `enableStats` option was removed and `pdfBug` is now instead used to enabled general debugging *and* page request/rendering stats.
Considering that in the default viewer the `stats` was only used when debugging was also enabled, this simplification (code wise) definitely seem worthwhile to me.
Rather than having two different (but connected) options for the textLayer, I think that it makes sense to try and unify this. For example: currently if `disableTextLayer === true`, then the value of `enhanceTextSelection` is simply ignored.
Since PDF.js version `2.0` already won't be backwards compatible in lots of ways, I don't think that we need to worry about migrating existing preferences here.
This removes the `PDFJS.externalLinkTarget`/`PDFJS.externalLinkRel` dependency from the viewer components, but please note that as a *temporary* solution the default viewer still uses it.
This removes the `PDFJS.imageResourcesPath` dependency from the viewer components and the test-suite, but please note that as a *temporary* solution the default viewer still uses it.
This removes the `PDFJS.maxCanvasPixels` dependency from the viewer components, but please note that as a *temporary* solution the default viewer still uses it.
This removes the `PDFJS.useOnlyCssZoom` dependency from the viewer components, but please note that as a *temporary* solution the default viewer still uses it.
Unfortunately, as far as I can tell, we still need the ability to adjust certain viewer options depending on the browser environment in PDF.js version `2.0`. However, we should be able to separate this from the general compatibility code in the `src/shared/compatibility.js` file.
This rule is available from https://www.npmjs.com/package/eslint-plugin-mozilla, and is enforced in mozilla-central. Note that we have the necessary `Array`/`String` polyfills and that most cases have already been fixed, see PRs 9032 and 9434.
This rule is available from https://www.npmjs.com/package/eslint-plugin-mozilla, and is enforced in mozilla-central. Note that we have a polyfill for `ChildNode.remove()` and that most cases have already been fixed, see PRs 8056 and 8138.
The code responsible for highlighting/scrolling thumbnails is quite old, and parts of it hasn't really changed much over time.
In particular, the `scrollThumbnailIntoView` method is currently using `document.querySelector` in order to find both the new/current thumbnail element. This seems totally unnessary, since we can easily keep track of the current thumbnail (similar to the "regular" viewer) and thus avoid unnecessary DOM look-ups.
Furthermore, note how the `PDFThumbnailView` constructor is currently highlighting the *first* thumbnail. This is yet another leftover from older viewer code, which we can now remove and instead take care of in `PDFThumbnailViewer.setDocument`.
Given that `PDFThumbnailView` does not, nor should it, have any concept of which thumbnail is currently selected, this change also improves the general structure of this code a tiny bit.
Despite this patch removing the `disableWorker` option itself, please note that we'll still fallback to loading the worker file(s) on the main-thread when running in environments without proper Web Worker support.
Furthermore it's still possible, even with this patch, to force the use of fake workers by manually loading the necessary file using a `<script>` tag on the main-thread.[1]
That way, the functionality of the now removed `SINGLE_FILE` build target and the resulting `build/pdf.combined.js` file can still be achieved simply by adding e.g. `<script src="build/pdf.worker.js"></script>` to the HTML (obviously with the path adjusted as needed).
Finally note that the `disableWorker` option is a performance footgun, and unfortunately many existing third-party examples actually use it without providing any sort of warning/justification.
---
[1] This approach is used in the default viewer, since certain kind of debugging may be easier if the code is running directly on the main-thread.
https://crbug.com/362061 was fixed in Chrome 36, and the lowest
supported Chrome version in the extension is Chrome 49, so the
work-around for a filesystem:-bug in chromecom can be removed.
This was introduced in #3582 to work around https://crbug.com/274024 .
The bug in Chrome has been fixed a long time ago (at least 33), so let's
simplify the code.
This patch updates the `IPDFStreamReader` interface and ensures that the interface/implementation of `network.js`, `fetch_stream.js`, `node_stream.js`, and `transport_stream.js` all match properly.
The unit-tests are also adjusted, to more closely replicate the actual behaviour of the various actual `IPDFStreamReader` implementations.
Finally, this patch adjusts the use of the Content-Disposition filename when setting the title in the viewer, and adds `PDFDocumentProperties` support as well.
When navigating away from the viewer, there's no good reason for disallowing replacement of a *temporary* history entry (in addition to empty ones).
Given that the current position is temporarily added to the browser history using a (short) timeout, the history entry will most likely already be correct when the 'pagehide' event fires. However, if the user is quick enough that might not always be the case, in which case the adjusted logic may help.
Since multiple empty lines is virtually unused in the code-base, and the few cases that do exist look like "typos", let's enforce greater consistency here; please see https://eslint.org/docs/rules/no-multiple-empty-lines.
It is only used in a few places to handle prefixing style properties if
necessary. However, we used it only for `transform`, `transformOrigin`
and `borderRadius`, which according to Can I Use are supported natively
(unprefixed) in the browsers that PDF.js 2.0 supports. Therefore, we can
remove this class, which should help performance too since this avoids
extra function calls in parts of the code that are called often.
Unless the debugging tools (i.e. `PDFBug`) are enabled, or the `browsertest` is running, the `PDFPageProxy.stats` aren't actually used for anything.
Rather than initializing unnecessary `StatTimer` instances, we can simply re-use *one* dummy class (with static methods) for every page. Note that by using a dummy `StatTimer` in this way, rather than letting `PDFPageProxy.stats` be undefined, we don't need to guard *every* single stats collection callsite.
Since it wouldn't make much sense to attempt to use `PDFPageProxy.stats` when stat collection is disabled, it was instead changed to a "private" property (i.e. `PDFPageProxy._stats`) and a getter was added for accessing `PDFPageProxy.stats`. This getter will now return `null` when stat collection is disabled, making that case easy to handle.
For benchmarking purposes, the test-suite used to re-create the `StatTimer` after loading/rendering each page. However, modifying properties on various API code from the outside in this way seems very error-prone, and is an anti-pattern that we really should avoid at all cost. Hence the `PDFPageProxy.cleanup` method was modified to accept an optional parameter, which will take care of resetting `this.stats` when necessary, and `test/driver.js` was updated accordingly.
Finally, a tiny bit more validation was added on the viewer side, to ensure that all the code we're attempting to access is defined when handling `PDFPageProxy` stats.
For sufficiently large page sizes, always limiting the minimum zoom level to 25% seem a bit too high. One example is pages, e.g. the first one, in:
Hence I think that it makes sense to lower `MIN_SCALE` slightly, since other PDF viewers (e.g. Adobe Reader) isn't limiting the minimum zoom level as aggressively. Obviously this will allow a greater number of pages to be visible at the same time in the viewer, but given that they will be small that shouldn't be an issue.
Note also that e.g. the `page-fit`/`page-width` zoom levels already allow `< MIN_SCALE` values, so I don't see why we shouldn't allow users the same functionality directly.
This was added, during the refactoring in PR 8556, to avoid outright breaking third-party users of the default viewer.
With PDF.js version `2.0`, where we're making API changes that aren't backwards compatible, we ought to be able to remove this piece of viewer code as well.
We've never attempted to limit the errors displayed in the default viewer to the current PDF file, but that's not really been a problem before. However after PR 7926, it's now possible to get password related error messages for *previously* opened PDF files in the default viewer.
**STR:**
1. Open a password protected PDF file, e.g. `issue6010_1.pdf` from the test-suite.
2. Cancel the password prompt.
3. Open any new PDF file in the viewer.
**AR:**
The error UI is displayed, with a `No password given` message.
**ER:**
No error displayed, since it's only relevent for a now closed PDF file.
This is obviously a minor issue, caused by us now rejecting the still pending `pdfLoadingTask` during the `PDFViewerApplication.close` call, but I don't think that it (generally) makes sense to show errors if they're not relevant to the *currently* displayed PDF file.
In order to move viewer related options from the global `PDFJS` object and into the initialization of the relevant components, we'll need to parse the hash parameters *before* calling `PDFViewerApplication._initializeViewerComponents`.
