To avoid having to request and await various browser data early during the PDF Viewer initialization, we can include that data in the existing preference fetching instead. With other planned changes to the PDF Viewer, the current situation would only become worse over time.
*Note:* Technically this data aren't preference-values, however we're already including other non-prefs in this list (e.g. `isInAutomation`) and doing it this way simplifies the overall implementation.
This method has only a single call-site in the viewer, since it's used as a fallback, and the functionality can be moved into the `DownloadManager.download` method instead.
Part of this code is really old and pre-dates general availability of things such as `Blob` and `URL.createObjectURL`. To avoid having to duplicate the Blob-creation in the viewer, we can move this into `DownloadManager.download` instead.
Also, remove a couple of unnecessary `await` statements since the methods in question are synchronous.
This old method is essentially just adding, a small amount of, unnecessary indirection and we can easily invoke `PDFViewerApplication.open` directly from the extension-specific code instead.
The following are some highlights of this patch:
- In the Worker we only extract a *subset* of the potential contents of the `Usage` dictionary, to avoid having to implement/test a bunch of code that'd be completely unused in the viewer.
- In order to still allow the user to *manually* override the default visible layers in the viewer, the viewable/printable state is purposely *not* enforced during initialization in the `OptionalContentConfig` constructor.
- Printing will now always use the *default* visible layers, rather than using the same state as the viewer (as was the case previously).
This ensures that the printing-output will correctly take the `Usage` dictionary into account, and in practice toggling of visible layers rarely seem to be necessary except in the viewer itself (if at all).[1]
---
[1] In the unlikely case that it'd ever be deemed necessary to support fine-grained control of optional content visibility during printing, some new (additional) UI would likely be needed to support that case.
Currently the `web/app.js` file pulls in various build-specific dependencies, via the use of import maps, and those files in turn import from `web/app.js` thus creating undesirable import cycles.
To avoid this we instead pass in a `PDFViewerApplication`-reference, immediately after it's been created, to the relevant code.
Note that we use an ESLint plugin rule, see `import/no-cycle`, that is normally able to catch import cycles. However, in this case import maps are involved which is why this wasn't caught.
All of our static evaluation & dead-code elimination transforms need to
happen in post-order, transforming inner nodes first. This is so that
in complex nested cases all transforms see the simplified version of
their inner nodes.
For example:
async getNimbusExperimentData() {
if (!PDFJSDev.test("GECKOVIEW")) { return null; }
// other code
}
-> [evaluation of PDFJSDev.*]
async getNimbusExperimentData() {
if (!false) { return null; }
// other code
}
-> [!false -> true]
async getNimbusExperimentData() {
if (true) { return null; }
// other code
}
-> [if (true) -> replace with the if branch]
async getNimbusExperimentData() {
return null;
// other code
}
-> [early return -> remove dead code]
async getNimbusExperimentData() {
return null;
// other code
}
This was done correctly in all cases except for our `UnaryExpression`
transform, which was happening in pre-order.
The `DefaultExternalServices` code, which is used to provide build-specific functionality, is very old. This results in a pattern where we first initialize `PDFViewerApplication.externalServices` and then *override* it for the different builds.
By converting `DefaultExternalServices` into a "regular" class, and leveraging import maps, we can directly initialize the correct instance depending on the build.
Given the simplicity of the `createPreferences` method, we can leverage import maps to directly initialize the correct `Preferences`-instance depending on the build.
Given the simplicity of the `createDownloadManager` method, we can leverage import maps to directly initialize the correct `DownloadManager`-instance depending on the build.
This unfortunately broke in PR 17060, since I had completely forgotten about https://bugzilla.mozilla.org/show_bug.cgi?id=1632644#c5 when writing that patch.
The easiest solution, while slightly unfortunate, seems to be to add a couple of non-standard hash parameters specifically for the PDF attachment use-case in the Firefox PDF Viewer. (Note that we cannot use "nameddest" here, since we also need to support the stringified destination-Array case.)
After the two previous commits, which removed the remaining call-sites, this method is no longer used and can thus be removed.
As mentioned in the JSDocs for the now removed method, synchronous communication between the viewer and the platform code isn't really a good idea.
Once this patch has landed in mozilla-central some additional clean-up of the platform code will also be possible.
The return value is not, nor has it ever been, used for anything and we should thus be able to just send the message.
Note that the responses are already handled by the "message" event listener registered above.
Currently we *synchronously* fetch a number of browser preferences/options, from the platform code, during the viewer respectively PDF document initialization paths.
This seems unnecessary, and we can re-factor the code to instead include the relevant data when fetching the regular viewer preferences.
Given that there's now a bit more asynchronicity in the l10n-initialization in the Firefox PDF Viewer, after PR 17115, try to limit the impact of that by moving it to occur a tiny bit earlier in the default viewer initialization.
- For the generic viewer we use @fluent/dom and @fluent/bundle
- For the builtin pdf viewer in Firefox, we set a localization url
and then we rely on document.l10n which is a DOMLocalization object.
Note how the [`ChromeActions.getPreferences` method](https://searchfox.org/mozilla-central/rev/4e8f62a231e71dc53eb50b6d74afca21d6b254e9/toolkit/components/pdfjs/content/PdfStreamConverter.sys.mjs#497-530) returns the preferences as a string, which we then have to convert back into an Object in the viewer.
Back when that code was originally written it wasn't possible to send Objects from the platform-code, however that's no longer the case and we should be able to (eventually) remove this unnecessary string-parsing now.
*Please note that in order to prevent breakage we'll need to land these changes in stages:*
- Land this patch in mozilla-central, as part of regular the PDF.js updates.
- Change the return type in the `ChromeActions.getPreferences` method, in a mozilla-central patch.
- Remove the string-handling from the `FirefoxPreferences._readFromStorage` method.
Please note that we've never had any functionality in the viewer itself that *set* preferences, and we've thus only ever read them.
For the GENERIC viewer it obviously makes sense for the user to be able to modify preferences, e.g. via the console, but that doesn't really apply to the *built-in* Firefox PDF Viewer since preferences are already accessible via `about:config` there. Hence it does seems somewhat strange to expose, a limited part of, the Firefox preference system in this way when we're not even using it.
Note that the unused preference setting-code also include a fair amount of *additional* validation on the platform-side, such as limiting any possible preference changes to the `pdfjs.`-branch and also an explicit white-list of preference names[1], to make sure that this is safe; please see:
- https://searchfox.org/mozilla-central/rev/4e8f62a231e71dc53eb50b6d74afca21d6b254e9/toolkit/components/pdfjs/content/PdfStreamConverter.sys.mjs#458-495
- https://searchfox.org/mozilla-central/rev/4e8f62a231e71dc53eb50b6d74afca21d6b254e9/toolkit/modules/AsyncPrefs.sys.mjs#21-48
Assuming that this patch lands, I'll follow-up with a mozilla-central patch to remove the code mentioned above.
---
[1] This hard-coded list contains preferences that no longer exist, and also at least one (fairly obvious) typo.
In PR #16295 one occurrence of this was changed, but a few more remained
in the codebase. This commit fixes the other occurrences so that we
don't use the deprecated way of creating custom events anywhere anymore.
According to MDN, see https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent/initCustomEvent,
using the `CustomEvent.initCustomEvent` method is deprecated and the
`CustomEvent` constructor should be used instead.
Extends d9bf571f5c.
*Please note:* This patch only extends the `PDFFindController` implementation itself to support this functionality, however it's *purposely* not exposed in the default viewer.
This replaces the previous `phraseSearch`-parameter, and a `query`-string will now always be interpreted as a phrase-search.
To enable searching for individual words, the `query`-parameter must instead consist of an Array of strings. This way it's now also possible to combine phrase/word searches, with a `query`-parameter looking something like `["Lorem ipsum", "foo", "bar"]` which will search for the phrase "Lorem ipsum" *and* the words "foo" respectively "bar".
The only parameter that we actually need here is the `PDFDataRangeTransport`-instance, since the others are not necessary.
- The `url` parameter, as passed to the `getDocument` function in the API, is simply being ignored; see 2d87a2eb1c/src/display/api.js (L447-L458)
- The `length` parameter, as passed to the `getDocument` function in the API, is always being overwritten; see 2d87a2eb1c/src/display/api.js (L519-L525)
- this way the context menu in Firefox can take into account what we
have in the clipboard, if an editor is selected, ...
- when the user will click on a context menu item, an action will be
triggered, hence this patch adds what is required to handle it;
- some tests will be added in the Firefox' patch.
After the changes in https://bugzilla.mozilla.org/show_bug.cgi?id=1757771, that simplified the MOZCENTRAL downloading code, the `ChromeActions.download`-method will no longer invoke the `sendResponse`-callback.
Hence it should no longer be necessary for the `DownloadManager`, in the MOZCENTRAL viewer, to use `FirefoxCom.requestAsync` since no response is ever provided.[1] For the allocated BlobURLs, they should (hopefully) be released when navigating away from the viewer.
---
[1] Note that that was *already* the case, for one of the previous code-paths in the `ChromeActions.download`-method.
