As outlined in https://bugzilla.mozilla.org/show_bug.cgi?id=1282759 the internal Firefox name for the feature is `entireWord`, hence that name is used here as well for consistency (with "Whole words" being limited to the UI).
Given existing limitations of the PDF.js search functionality, e.g. the existing problems of searching across "new lines", there's some edge-cases where "Whole words" searching will ignore (valid) results.
However, considering that this is a pre-existing issue related to the way that the find controller joins text-content together, that shouldn't have to block this new feature in my opionion.
*Please note:* In order to enable this feature in the `MOZCENTRAL` version, a small follow-up patch for [PdfjsChromeUtils.jsm](https://hg.mozilla.org/mozilla-central/file/tip/browser/extensions/pdfjs/content/PdfjsChromeUtils.jsm) will be required once this has landed in `mozilla-central`.
For the `PDFFindBar` implementation, similar to the native Firefox findbar, the matches count displayed is now limited to a (hopefully) reasonable value.
*Please note:* In order to enable this feature in the `MOZCENTRAL` version, a follow-up patch will be required once this has landed in `mozilla-central`.
This patch is the first step to be able to eventually get rid of the `attachDOMEventsToEventBus` function, by allowing `EventBus` instances to simply re-dispatch most[1] events to the DOM.
Note that the re-dispatching is purposely implemented to occur *after* all registered `EventBus` listeners have been serviced, to prevent the ordering issues that necessitated the duplicated page/scale-change events.
The DOM events are currently necessary for the `mozilla-central` tests, see https://hg.mozilla.org/mozilla-central/file/tip/browser/extensions/pdfjs/test, and perhaps also for custom deployments of the PDF.js default viewer.
Once this have landed, and been successfully uplifted to `mozilla-central`, I intent to submit a patch to update the test-code to utilize the new preference. This will thus, eventually, make it possible to remove the `attachDOMEventsToEventBus` functionality.
*Please note:* I've successfully ran all `mozilla-central` tests locally, with these patches applied.
---
[1] The exception being events that originated on the `window` or `document`, since those are already globally available anyway.
The new events follow the same naming pattern as the 'pagesinit'/'pagesloaded' events dispatched on `BaseViewer` instances, and the intention is to allow the eventual removal of 'documentload'.
Given that the various Preferences are currently, and have been for quite some time, only used when initializing `PDFViewerApplication` re-loading them when a new PDF file is opened in the viewer is essentially a no-op.
Furthermore, with the only usage of `BasePreferences.reload` now gone, the value of that method seems questionable at best. In the event that the functionality is actually needed again, similar to the `ViewHistory`, it'd probably make more sense to simply replace `PDFViewerApplication.preferences` with a new `BasePreferences` instance instead (using e.g. `DefaultExternalServices.createPreferences`).
Given that *all* Preferences are already fetched in `PDFViewerApplication._readPreferences`, the amount of boilerplate/duplication can be considerably reduced with the addition of a `BasePreferences.getAll` method.
The only reason that this check ever existed in the first place, is that originally there was a global `PDFJS.openExternalLinkInNewWindow` option which was then subsumed by the (more generic) `PDFJS.externalLinkTarget` option. (The `externalLinkTarget` has since been moved into a `PDFLinkService` option, as part of PDF.js version `2.0`.)
Hence, during the period where both `PDFJS.openExternalLinkInNewWindow` and `PDFJS.externalLinkTarget` existed side-by-side, there was a need to allow the former one to override the latter one (for backward compatibility purposes). However, that's no longer the case, and this extra `externalLinkTarget` check can now be removed.
*This was a stupid error on my part; sorry about breaking this!*
With the current code, the value of the `externalLinkTarget` option is now (potentially) updated *after* the viewer components have been initialized. For the "viewer in iframe/object tag" case, the result is that the value of the `externalLinkTarget` option isn't adjusted as intended any more.
This moves/exposes the `URL` polyfill similarily to the existing `ReadableStream` polyfill, rather than exposing it globally, to avoid interfering with any "outside" code.
Both the `URL` and `ReadableStream` polyfills are now exposed on the `pdfjsLib` object, such that they are accessible to the viewer components.
Furthermore, the `no-restricted-globals` ESLint rule is also enabled to prevent accidental usage of the native `URL`/`ReadableStream` implementations directly in the `src/` and `web/` folders; see also https://eslint.org/docs/rules/no-restricted-globals
Addresses the remaining TODO in https://github.com/mozilla/pdf.js/projects/6
Rather than having to manually call a method on `PDFFindController` instances from `BaseViewer.setDocument`, thus essentially having to resolve the private `_firstPagePromise` from the "outside", this can be done easily with the 'pagesinit' event dispatched on the `eventBus` instead.
Please note this particular `PDFFindController` code pre-dates the `eventBus` by almost three years, which should explain why the code looks the way it does.
Since all the other viewer methods use the getter/setter pattern, e.g. for setting page/scale/rotation, the way that the Scroll/Spread modes are set thus stands out. For consistency, this really ought to use the same pattern as the rest of the `BaseViewer`. (To avoid breaking third-party implementations, the old methods are kept around as aliases.)
Note how the other "...OnLoad" preferences will allow a *non-default* value to always override a history entry. To improve overall consistency for the viewer options, and to reduce possibly confusing behaviour, this patch changes the `scrollModeOnLoad`/`spreadModeOnLoad` preferences to behave as all the other ones.
This property isn't accessed anywhere in the `web/app.js` file, and is also not being reset in `PDFViewerApplication.close`. Hence it seems that it can simply be removed, especially since the fingerprint is already synchronously available through `PDFViewerApplication.pdfDocument.fingerprint` (provided that a document is loaded).
With the current code, the location in the viewer could change *well* after the user has started to interact with the viewer (for very large and/or slow loading documents). Attempt to reduce the likelyhood of that happening, by adding an upper bound to the time spent waiting before attempting to re-apply the initial position.
Rather than using a "special" property to check if a `ViewHistory` database entry existed on load, it seems that you could just as well check for the existence of one of the actually needed properties instead (here 'page' is used).
This way we can avoid storing what, more of less, amounts to useless state, which will help reduce the size of the `ViewHistory` database. Given that we don't directly, nor need to, validate the `ViewHistory` data but rely on other parts of the code-base to do so, the existance of an 'exists' property doesn't in and of itself really add much utility.
Finally, to simplify the implementation the 'exists' property won't be actively removed from the `ViewHistory` data. Instead we'll simply stop adding 'exists' when writing `ViewHistory` data, and rely on the existing pruning of old entries to eventually remove any remnants of 'exists' from storage.
The `onOpenWithURL` method, in `PDFViewerApplication.initPassiveLoading`, accepts a `originalURL` parameter which is then passed on to `PDFViewerApplication.open` as is. However, the latter method expects the name of the parameter to be `originalUrl` (note the casing), meaning that `getDocument` will fail in this case.
For consistency, and to avoid confusion, the renaming is done in `web/chromecom.js` as well.
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 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.
*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.
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.
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.
