Until now we've not actually had *any* tests that ensure that the *official* PDF.js-viewer API exposes the intended functionality, which means that things can easily break accidentally.
*Please note:* This unit-test cannot (easily) be run in Node.js-environments, since the `external/webL10n/l10n.js` file contains various browser-specific functionality.
- Change (most) fields/methods into private ones, since that's now supported.
- Tweak the constructor-parameters, and simplify the sandbox initialization w.r.t. the viewer components.
- Remove some unused function/method parameters.
- Slightly simplify the "updatefromsandbox"-handler by using local variables and inverting some conditions.
Rather than sprinkling pre-processor statements throughout the viewer-code, simply "disable" the relevant `PDFViewer` setters instead.
Also, given that the GeckoView-specific viewer doesn't have a sidebar we don't actually need to explicitly ignore a `pageMode` during loading.
This helper function was added almost two years ago, in PR 13696, and it still has only a single call-site. Furthermore, with the changes made in PR 16572 it also cannot hurt to reduce the size of the `web/l10n_utils.js` file slightly.
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.
This method was added only for consistency with the `register`-method, however it's never actually been used. To avoid including dead code in the builds, let's just remove the `unregister`-method for now.
*Please note:* If this method ever becomes useful, it'll be trivial to revert this commit.
With the changes in PR 16552 we can now move general translation into the `AnnotationLayer` itself, which should improve things ever so slightly in third-party implementations where the default viewer isn't used.
*This is something that I completely overlooked during review of PR 16552, despite leaving a l10n-related comment.*
The new l10n-handling of PopupAnnotations assume that the `AnnotationLayer` is always initialized with a l10n-instance, which might not actually be the case in third-party implementations where the default viewer isn't used.
To work-around that we'll now bundle, and fallback on, the existing `NullL10n`-implementation in GENERIC builds of the PDF.js library. This will only result in a slight file-size increase for the *built* `pdf.js` file, again limited to GENERIC builds, since the `web/l10n_utils.js` file has no dependencies.
Also, tweaks a couple of TESTING pre-processor checks to *only* include that code when running the reference tests.
- it'll help to be able to move popups on screen to let the user read the text
- popups won't inherit some properties from their parent:
- the popup can be misrendered if for example the parent has a clip-path property.
- add an outline to the popup when the parent is focused.
- hide a popup when it's clicked.
While it's slightly difficult to trigger in practice, unless the `defaultZoomDelay`-value is increased, it's currently possible to generate thumbnails from *partially* rendered pages when doing *temporary* CSS-only zooming.
We shouldn't dispatch a "pagerendered"-event when doing *temporary* CSS-only zooming, but simply wait until the actual rendering is done.
While I don't believe that this regression has caused any actual bugs, dispatching *duplicate* events is nonetheless inconsistent and should be fixed.
Given that this functionality is only relevant in third-party use-cases, for example the viewer-components, we can avoid needlessly including it in e.g. the MOZCENTRAL build.
This patch does two things:
- Moves the updating of thumbnails into `web/app.js`, via a new `PDFSidebar` callback-function, to avoid having to include otherwise unnecessary parameters when initializing a `PDFSidebar`-instance.
- Only attempt to generate thumbnail-images from pages that are *cached* in the viewer. Note that only pages that exist in the `PDFPageViewBuffer`-instance can be rendered, hence it's not actually meaningful to check every single page when updating the thumbnails.
For large documents, with thousands of pages, this should be a tiny bit more efficient when e.g. opening the sidebar since we no longer need to check pages that we know have not been rendered.
The way that the cleanup was implemented in PR 12613 has always bothered me slightly, since the `isPageCached`-method that I introduced there always felt quite out-of-place in the `IPDFLinkService`-implementations.
By introducing a new "thumbnailrendered" event, similar to the existing "pagerendered" one, we're able to move the cleanup handling into the `PDFViewer`-class instead.
The way that this was implemented in PR 10217 has always bothered me slightly, since the `isPageVisible`-method that I introduced there always felt quite out-of-place in the `IPDFLinkService`-implementations.
Hence this is instead replaced by a callback-function in `PDFFindController`, to handle the page-visibility checks. Note that since the `PDFViewer`-constructor always sets this callback-function, e.g. the viewer-component examples still work as-is.
- Remove the dependency on fit-curve;
- Improve the way to draw the current line in using a Path2D and
in clearing only the last part of the curve instead of clearing
all the canvas;
- Smooth the curve when drawing to avoid to have some changes after
the drawing ends;
- Make the smoothing a bit less agressive.
Looking at the behaviour in Adobe Reader it doesn't appear that attachments are sorted alphabetically, hence it doesn't seem necessary for us to do so either in the viewer.
An additional benefit of *not* sorting the attachments is that any "actual" attachments are now always placed at the top of the list in the sidebar, and if any `FileAttachment`-annotations exist in the document they will now be appended at the end.
After PR 12563 we're now free to use optional chaining in the worker-thread as well. (This patch also fixes one previously "missed" case in the `web/` folder.)
For the MOZCENTRAL build-target this patch reduces the total bundle-size by `1.6` kilobytes.
Originally the `PDFSidebarResizer` class was slightly larger, since the code used to contain e.g. feature testing for older (and no longer supported) browsers.
Given that there's some amount of overlap, when it comes to what DOM-elements and state that these classes need, it now seems reasonable to simply move the sidebar-resizing into the `PDFSidebar` class.
For the MOZCENTRAL build-target this patch reduces the size of the *built* `web/viewer.js` file by just over `1.1` kilobytes.
Similar to other toolbar/secondaryToolbar buttons that open toolbars or dialogs, it seems reasonable to use "aria-controls" for the editor-toolbar buttons as well.
- Replace FoxitSans with LiberationSans: LiberationSans is already there (for XFA) and we can use
it as a good replacement of FoxitSans.
- For now we just try to substitue standard fonts, the strategy is the following:
* we try to find a font locally from a hardcoded list;
* if it fails then we use Liberation as fallback (only for Helvetica for the moment);
* else we just fallback on the system serif/sansserif/monospace font.
The fallback code-path has never really been used, since the `PDFSidebar` is only used in the default viewer (and has never been exposed in e.g. the COMPONENTS-build).
This patch tries to simplify, and improve, the thumbnail styling:
- For rendered thumbnails there's one less DOM-element per thumbnail, which can't hurt in longer documents.
- Use CSS-variables to set the dimensions of all relevant DOM-elements at once.
- Simplify the visual styling of the thumbnails, e.g. remove the border since the viewer no longer has visible borders around pages, since the relevant CSS-rules are quite old code.
These changes also, at least in my opinion, makes the relevant CSS-rules much easier to understand and work with.
- Make it easier to work on e.g. [bug 1690428](https://bugzilla.mozilla.org/show_bug.cgi?id=1690428) without affecting the other sidebarViews.
Now that https://bugzilla.mozilla.org/show_bug.cgi?id=1247687 has landed in Firefox, we're able to use worker-modules during development :-)
This removes the final piece of SystemJS usage from the PDF.js library, thus allowing a fair bit of clean-up, and we now use *only* native `import`/`export` statements everywhere in development mode.
This patch tries to mimic the look of the message-element in the Firefox browser-findbar, and thus makes the following changes:
- Remove the red colour, since it didn't take the light/dark themes into account.
- Display the "notFound" message in bold.
Currently we only prevent triggering the actual text-extraction multiple times in "parallel", when using the "copy all text" feature, however the "copy"-event itself is not prevented.
The result is that if the user selects all text in a long PDF document and then uses the copy-shortcut multiple times in quick succession, we'll actually populate the clipboard with "incomplete" contents (via a `TextLayerBuilder` copy-listener) until all text-extraction finishes.
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.
When permissions are enabled and the PDF document doesn't have the COPY-flag set, it shouldn't be possible for the user to trigger the "copy all text" feature.
While this slightly reduces duplication in the CSS rules, some of the auto-formatting done by Prettier is perhaps not great. (Given the overall advantage of using Prettier, we'll probably have to simply accept this.)
Hopefully these changes make sense (since this functionality is new to me), however the existing `xfa`-tests should help avoid any outright regressions.
Some arabic chars like \ufe94 could be searched in a pdf, hence it must be normalized
when creating the search query. So to avoid to duplicate the normalization code,
everything is moved in the find controller.
The previous code to normalize text was using NFKC but with a hardcoded map, hence it
has been replaced by the use of normalize("NFKC") (it helps to reduce the bundle size
by 30kb).
In playing with this \ufe94 char, I noticed that the bidi algorithm wasn't taking into
account some RTL unicode ranges, the generated font wasn't embedding the mapping this
char and the unicode ranges in the OS/2 table weren't up-to-date.
When normalized some chars can be replaced by several ones and it induced to have
some extra chars in the text layer. To avoid any regression, when copying some text
from the text layer, a copied string is normalized (NFKC) before being put in the
clipboard (it works like this in either Acrobat or Chrome).
After the previous patch we now have only *a single* `PRODUCTION` occurrence in the entire code-base, more specifically in the `web/viewer.html` file.
This special build-target can be replaced with any condition that always evaluate to `false`, such as e.g. a comment.
*Please note:* This patch might be considered too hacky, hence I completely understand if it's rejected.
This *special* build-target is very old, and was introduced with the first pre-processor that only uses comments to enable/disable code.
When the new pre-processor was added `PRODUCTION` effectively became redundant, at least in JavaScript code, since `typeof PDFJSDev === "undefined"` checks now do the same thing.
This patch proposes that we remove `PRODUCTION` from the JavaScript code, since that simplifies the conditions and thus improves readability in many cases.
*Please note:* There's not, nor has there ever been, any gulp-task that set `PRODUCTION = false` during building.
To make this functionality work out-of-the-box in custom implementations, see e.g. the "viewer components" examples, it'd be slightly easier if we dynamically create/insert the "hiddenCopyElement" in the `PDFViewer` constructor.
Given that the "copy all text" feature still appears to work just as before with this patch, hopefully I'm not overlooking any reason why doing this would be a bad idea.
I was playing with the new "copy all text" feature, and stumbled upon one document where the copied text was truncated; see http://mirrors.ctan.org/info/lshort/english/lshort.pdf
The problem turns out to be that on [page 83](https://ftp.acc.umu.se/mirror/CTAN/info/lshort/english/lshort.pdf#page=83) the textLayer contains `\u0000` and apparently copying just stops when a null char is encountered.
To fix this we can simply use an existing helper function, and with this patch we're able to successfully copy all the text in that document.
*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".
By getting the width/height of the first page initially, we can slightly reduce the amount of code needed both in the `hasEqualPageSizes`-check and when building the print-styles.
For the moment there is no real consensus on how we should download a pdf on Android.
Hence we keep this solution for the moment but behind a pref (which will be true on
nightly only).
Currently we repeat the same code in lots of places, to update the "toggled" class and "aria-checked" attribute, when various toolbar buttons are clicked.
For the MOZCENTRAL build-target this patch reduces the size of the *built* `web/viewer.js` file by just over `1.2` kilo-bytes.
Currently `float: inline-start/inline-end` is only supported in Firefox, see https://developer.mozilla.org/en-US/docs/Web/CSS/float#browser_compatibility, and in order to support other browsers we're thus forced to jump through some hoops.
This leads to slightly less nice code in the *built-in* Firefox PDF Viewer, and this patch attempts to improve the current situation:
- Use Stylelint to forbid direct use of `float: inline-start/inline-end` in the CSS files, to prevent future bugs in the general PDF.js viewer.
- Do a build-time replacement, only in MOZCENTRAL builds, to replace the CSS-variables with raw `float: inline-start/inline-end` instances.
This effectively implements some of the changes from https://phabricator.services.mozilla.com/D170496, but using our existing "direction aware" CSS-variable to limit the amount of code changes needed.
With the changes in PR 16153 we're no longer setting a `<base href>` in the Firefox PDF Viewer, hence it shouldn't be necessary to keep setting a `baseUrl` in the `PDFLinkService`-class.
Given that the original document URL is now kept, the browser itself will handle relative URLs and we can thus slightly reduce the amount of string parsing required when handling various links in the viewer.
Currently if you e.g. enable the `useOnlyCssZoom` option rendering may no longer finish as intended. To reproduce:
- Enable the `useOnlyCssZoom` option.
- Load https://github.com/mozilla/pdf.js/files/1522715/wuppertal_2012.pdf (in the development viewer).
- When rendering starts, *immediately* change the zoom-level.
In this case the document will never finish rendering, since the `postponeDrawing`-functionality will (here incorrectly) abort rendering and with CSS-only zooming rendering is only expected to happen once per page.
To fix this we'll simply ignore any `drawingDelay` when CSS-only zooming is used (regardless if it's triggered via the option or the zoom-level being very large).
Currently the `zoomLayer` isn't rotated correctly in all cases. To reproduce:
- Load https://github.com/mozilla/pdf.js/files/1522715/wuppertal_2012.pdf
- Let the document render.
- Rotate the document *four* times, such that the original rotation is restored.
The easiest solution, as far as I can tell, is that we always set the `transform` just as we did (for years) prior to the changes in PR 15812.
- Reduce a little bit of duplication by enforcing the max/min scale-values once, at the end, in the `increaseScale`/`decreaseScale` methods.
- Convert the "private" `PDFViewer` scale-related methods into actually private ones, now that JavaScript supports that.
Given that the viewer always set the `dir`-attribute, to either LTR or RTL, we should be able to use this logical CSS property to (very slightly) reduce the size of the CSS; please see https://developer.mozilla.org/en-US/docs/Web/CSS/inset-block
The signatures of these methods were changed in PR 15886, which has now been included in a couple of releases, hence it should hopefully be OK to remove the fallback code-paths now.
Also, the methods are updated slightly to be explicit about what options are supported and we'll no longer pass along any arbitrary options to the "private" methods.
Some of these pre-processor statements are *many* years old, and could thus do with some clean-up. Note that the pre-processor originally didn't support else-if statements, and by using those the code becomes a bit less verbose.
The idea is to apply an overall filter on each page: the main advantage
is to have some filtered images which could help to make them visible for
some users.
The tag <base> is used to resolve relative URIs within the document.
Newly added SVG filters use a relative URI which then use the URI in base
but this one mismatches with the document URI and consequently filters are
not found in the Firefox viewer.
So this patch just removes <base> and replace few relative URLs by absolute
ones.
In order to help to identify a link, we add a border around it with the LinkText color.
And backdrop colors are inverted when the mouse pointer hovers them, this way it should
help to identify the link where the pointer is.
Given that the debugging hash-parameters will only be used when the `pdfBugEnabled` option is manually set[1], we can skip a *tiny* bit of asynchronicity for "regular" users.
---
[1] Note that it's enabled by default in the development viewer, i.e. in `gulp server` mode.
In the mac case we don't want to care about the scaleFactor threshold
because else if too big another move could start and then subsequent
events aren't considered as wheel events.
It isn't really ideal and at some point we'll need to find a way at
least for the Firefox case to get the real events instead of the fake
wheel ones.
In looking at a profile, I noticed in Marker chart that there's an animation
for loading-icon.gif even if this icon isn't visible.
This patch doesn't completely remove it but just slightly postpones it.
This further extends the web-specific import maps introduced in PR 16009, to allow removing *most* of the build-time `require` statements from the viewer. The few remaining ones are fallbacks used for the COMPONENTS respectively the `legacy` GENERIC builds.
After the compatibility updates in PR 15968 it's no longer strictly necessary to build the `viewer.css` file in order for the *development viewer* to work in Chromium-based browsers.
*Please note:* Given that Chromium-based browsers still don't support the *unprefixed* `mask-image` property the icons won't look right, however the development viewer itself works.
Given that Firefox is the *primary* development target, and that running `gulp generic` locally will generate polyfilled CSS, it seems reasonable to make this simplification here.
Currently there's no toolbar in the GV-viewer, hence invoking the pageLabels functionality isn't meaningful and just leads to unnecessary parsing on both the main- and worker-threads. (And if a toolbar is added at some point, it's not clear to me if we'd want to support pageLabels in the GV-viewer anyway.)
Currently we have a couple of pre-processor checks, specifically for the GV-viewer, spread throughout the code. This works fine when *building* the viewer, however they're obviously ignored in development mode (i.e. `gulp server`).
This leads to a situation where the GV development viewer, i.e. http://localhost:8888/web/viewer-geckoview.html, behaves subtly different from its built version. This could easily lead to bugs, hence this patch introduces a development mode constant to hopefully improve things here.
Finally, in a follow-up to PR 15842, also ignores the `pageMode`-state since there's no sidebar available.
Currently there's no UI for this functionality in the GV-viewer, however we still call the API methods. This potentially leads to a bunch of worker-thread parsing, for PDF documents with these features, despite the result being completely unused.
Given that mobile devices are usually more resource constrained than desktop/laptop computers, not to mentioned battery life, we can avoid doing work that'll just be ignored anyway.
The `DownloadManager.openOrDownloadData` method is written for the default-viewer specifically, assuming a viewer able to handle e.g. URL search/hash parameters. In the viewer components there's obviously no such functionality, and we should thus trigger downloading of PDF attachments directly instead.
Given that the GV-viewer isn't using most of the UI-related components of the default-viewer, we can avoid including them in the *built* viewer to save space.[1]
The least "invasive" way of implementing this, at least that I could come up with, is to leverage import maps with suitable stubs for the GV-viewer.
The one slightly annoying thing is that we now have larger import maps across multiple html-files, and you'll need to remember to update all of them when making future changes.
---
[1] With this patch, the built `viewer.js` size is 391 kB and `viewer-geckoview.js` is 285 kB.
This is very old code, where we loop through the user-provided options and build an internal parameter object. To prevent errors we also need to ensure that the parameters are correct/valid, which is especially important for the ones that are sent to the worker-thread such that structured cloning won't fail.[1]
Over the years this has led to more and more code being added in `getDocument` to validate the user-provided options, and at this point *most* of them have at least basic validation. However the way that this is implemented feels slightly backwards, since we first build the internal parameter object and only *afterwards* validate those parameters.[2]
Hence this patch changes the `getDocument` function to instead check/validate the supported options upfront, and then *explicitly* build the internal parameter object with only the needed properties.
---
[1] Note the supported types at https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm#supported_types
[2] The internal parameter object may also, because of the loop, end up with lots of unnecessary properties since anything that the user provides is being copied.
These functions invoke the `PDFViewer.currentPageNumber` setter, which already checks that a `pdfDocument` is currently active. Also, given that they're event handlers for the First/Last-page buttons (in the SecondaryToolbar) they can't be invoked before the viewer has been fully initalized.
The default value of the `--scale-select-width` CSS variable has been choosen such that it should be large enough for most locales. This means that in many locales we don't even update the CSS variable at all, and for those locales where we do the update happens *one time* early during the viewer initialization (i.e. before the PDF document has loaded).
*Please note:* Compared to other recent PRs, the effect of these changes ought to be really tiny and are mostly done to promote better coding patterns.
The reasons for making this change are:
- There's no UI available to toggle the cursor-tools in the GeckoView-specific viewer.
- The `HandTool`-implementation basically *simulates* touch scrolling, and is thus unlikely to be helpful/useful anyway.
- PR 15831 already changed the relevant call-sites to handle `PDFViewerApplication.pdfCursorTools` being undefined.
Some of the code in this method is *very* old, and we could thus modernize it a little bit by removing a couple of the loops used to build the `getDocument` argument.
These `@media` rules were most likely just copy-pasted from the regular viewer, however none of them are currently necessary since the GeckoView-specific viewer doesn't have any toolbars.
Note that the whole purpose of these CSS rules are to make the toolbar, of the regular viewer, responsive. If we in the future add toolbars for the GeckoView-specific viewer, these rules most likely wouldn't be usable as-is anyway.
This option was added specifically for third-party users, but has never been used in the PDF.js project itself. Furthermore there's no preference that can be used to enable it, and you need to provide the `removePageBorders` option when initializing a `PDFViewer`-instance.
This patch thus get rid of a little bit more unused code in the Firefox PDF Viewer.
*Unfortunately I missed this during testing/reviewing of PR 15992.*
With the changes in PR 15992 we're now only adding the `loadingIcon`-class when rendering is actually `RUNNING`, in order to improve overall performance.
However when resetting the page, i.e. the `INITIAL` state, we also need to remove the `loadingIcon` completely. Without this patch if you scroll through a document where the pages don't load instantaneously, see e.g. issue 2504, we'll leave the `loadingIcon`-class attached to pages that have had their rendering cancelled *and* also been evicted from the `PDFPageViewBuffer`-instance.
This way we don't have a lot of useless divs and we let the css engine handle the
creation/destruction of the :after pseudo-element.
It'll help to slightly improve performance when zooming.
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)
Until PR 12563 is deemed safe to land, I'd still like to be able to use worker-modules in the viewer during local development.
Hence this patch which *temporarily* adds a new `workerModules` hash-parameter, only available in non-PRODUCTION mode, that allows using worker-modules in the development viewer.
To enable this functionality, simply use http://localhost:8888/web/viewer.html#workerModules=true
If this method was added today, I really can't imagine that we'd support anything *except* objects. Unfortunately we cannot just remove this now, since the code has existed since "forever", however we can deprecate this and limit it to only the GENERIC build.
Furthermore, we can avoid a redundant `PDFViewerApplication.setTitleUsingUrl` call in the Firefox PDF Viewer since the title has already been set previously in that case.
This function is only used in PresentationMode these days, but we can still improve it a little bit:
- Use the existing web-platform `deltaMode` constants, rather than defining our own constants for those values.
- Access the `deltaMode` first, before the `delta{X, Y}` properties, to avoid being affected by bug 1392460 (similar to the default viewer).
It seems nicer overall, since we're exporting the `ProgressBar` in the viewer-components, to move this functionality into the `ProgressBar`-class itself rather than handling it "manually" in the default-viewer.
At the beginning of a search we can an update can be triggered with 0 over 0
found matches.
In the GeckoView context, we can't update the finder whenever we want but only
when it has been required.
Rather than adding `@media (forced-colors: active) { ... }`-blocks throughout the CSS code, we should utilize CSS variables instead as in our other CSS files.
When a css variable is update in a node then all the children under this
node are updated.
In order to avoid to update all the UI when a page is rescaling, this
patch moves the --scale-factor from the :root to the viewer container.
After the changes in PR 15812 we'll now *intermittently* display completely black canvases during zooming. To reproduce this, try switching to wrapped-scrolling and zoom in/out very quickly using either the mouse-wheel or pinching.
This patch removes the recently introduced `transferPdfData` API-option, and simply enables transferring of TypedArray data *by default* instead of copying it. This will help reduce main-thread memory usage, however it will take ownership of the TypedArrays. Currently this only applies to the following cases:
- TypedArrays passed to the `getDocument`-function in the API, in order to open PDF documents from binary data.
- TypedArrays passed to a `PDFDataRangeTransport`-instance, used to support custom PDF document fetching/loading (see e.g. the Firefox PDF Viewer).
*PLEASE NOTE:* To avoid being affected by this, please simply *copy* any TypedArray data before passing it to either of the functions/methods mentioned above.
Now that we transfer TypedArray data that we previously only copied, we need to be more careful with input validation. Given how the `{IPDFStreamReader, IPDFStreamRangeReader}.read` methods will always return ArrayBuffer data, which is then transferred to the worker-thread[1], the actual TypedArray data passed to the API thus need to have the same exact size as its underlying ArrayBuffer to prevent issues.
Hence we'll check for this and only allow transferring of *safe* TypedArray data, and fallback to simply copying the data just as before. This obviously shouldn't be an issue in the Firefox PDF Viewer, but for the general PDF.js library we need to be more careful here.
---
[1] See e09ad99973/src/display/api.js (L2492-L2506) respectively e09ad99973/src/display/api.js (L2578-L2590)
- Scale factor is rounded to only scale by integer percent, hence the unused
ticks are accumulated (like we already do for zoom with the mouse wheel).
- Use the same thing for the pinch-to-zoom on a touchscreen: it led to slightly
refactor the code because it happened to ignore a not so small scale which
led to a not so smooth zooming.
Also, removes the `initialData`-parameter JSDocs for the `getDocument`-function given that this parameter has been completely unused since PR 8982 (over five years ago). Note that the `initialData`-parameter is, and always was, intended to be provided when initializing a `PDFDataRangeTransport`-instance.
After the changes in PR 15850, the `background-color` of the sidebar is now unnecessarily dark in the light-theme. Hence, we can simply remove this CSS rule to improve things overall (and these changes don't affect the dark-theme much at all).
This is even an overall consistency improvement, given the existing `--sidebar-narrow-bg-color` values.
In most of the cases, showing the loading icon is useless because
it's for a very short time, consequently it doesn't bring any useful
information for the user.
After a delay (400ms), the icon is shown in order to inform the user
that the viewer isn't stuck but it's doing something.
In GeckoView, on an event, a callback must be executed with the result of an action,
but the callback can be used only one time.
So for each FindInPage event, we must trigger only one matches count update.
This option/preference was disabled in GENERIC builds, see PR 15812, to avoid landing it *just before* a new release. Hence it should be fine to enable this now.
This tweaks a few name that originated in PR 15812, to improve overall consistency:
- Use the `drawingDelay` parameter-name in all methods that accept a delay.
- Use the `postponeDrawing` variable-name in all relevant methods.
With upcoming background changes elsewhere in the viewer, this should be helpful in separating the styling of the loadingBar. These changes also means that both the "regular" and the "indeterminate" loadingBar now uses the same `background-color` value.
Also, shortens the related CSS variables a little bit since that can't hurt.
*This makes the same kind of changes as in the previous patch, but for the pageNumber-loadingIcon in the main toolbar.*
To display the pageNumber-loadingIcon when rendering starts, if the page is the most visible one, we'll utilize the existing "pagerender" event.
To toggle the pageNumber-loadingIcon as the user moves through the document we'll now instead utilize the "pagechanging" event, which should actually be slightly more efficient overall[1]. Note how we'd, in the old code, only consider the most visible page anyway when toggling the pageNumber-loadingIcon.
---
[1] Even in a PDF document as relatively short/simple as `tracemonkey.pdf`, scrolling through the entire document can easily trigger the "updateviewarea" event more than a thousand times.
Given that we only render one page at a time, this will lead to only *one* page-loadingIcon being displayed at a time even if multiple pages are visible in the viewer. However, this will make it clearer which page is the currently parsing/rendering one.
To simplify toggling of the page-loadingIcon visibility, the existing `PDFPageView.renderingState` is changed into a getter/setter-pair with the latter also handling the page-loadingIcon state.
An additional benefit of these changes is that the `PDFViewer` no longer needs to handling toggling of page-loadingIcon visibility during rendering, since there can only ever be *one* page rendering.
Finally, this may also simplify future changes w.r.t. page-loadingIcon visibility toggling (using e.g. a show-timeout).
The rotation-caching added in PR 15812 completely breaks initialization of PDF documents with varying page sizes, causing all pages to wrongly get the same size; see e.g. `sizes.pdf` from the test-suite.
To fix that without having to e.g. add a new parameter, which feels error prone, this patch changes the `PDFPageView.#setDimensions` method to completely ignore the rotation-caching until the `setPdfPage`-method has been called.
Right now, the visible pages are redrawn for each scale change.
Consequently, zooming with mouse wheel or in pinching can be pretty janky
(even on a desktop machine but with a hdpi screen).
So the main idea in this patch is to draw the visible pages only once zooming
is finished.
After the changes in PR 15829 the `loadingIconDiv` is no longer always visible when it should be, specifically in the case where we cancel and re-render a partially parsed/rendered page.
To reproduce this, try opening https://github.com/mozilla/pdf.js/files/1522715/wuppertal_2012.pdf in the viewer and change the zoom level while rendering is ongoing. In this case the `loadingIconDiv` doesn't actually become visible, despite being present in the DOM, since it's no longer at the end of the page-div.
I don't know to what extent this renders PR 15829 "pointless", however we're not repeatedly re-creating and re-inserting the `loadingIconDiv` but rather just *move* the existing element in the DOM.
We'll no longer import the `SimpleLinkService` dependency unconditionally in the file, since it's only used in COMPONENTS-builds.
Furthermore, for the COMPONENTS-builds, we'll create a `SimpleLinkService`-instance only for those layers that actually need it.
Please note that this functionality has never really mattered for the Firefox PDF Viewer, the GENERIC viewer, or even the "simpleviewer"/"singlepageviewer" component-examples. Hence, in practice this means that only the "pageviewer" component-example[1] have ever really utilized this.
Using factories to initialize various layers in the viewer, rather than simply invoking the relevant code directly, seems (at least to me) like a somewhat roundabout way of doing things.
Not only does this lead to more code, both to write and maintain, but since many of the layers have common parameters (e.g. an `AnnotationStorage`-instance) there's also some duplication.
Hence this patch, which removes the `xfaLayerFactory` and instead uses a lookup-function in the `PDFPageView`-class to access the external viewer-properties as necessary.
Note that this should even be an improvement for the "pageviewer" component-example, since most layers will now work by default rather than require manual configuration.
---
[1] In practice we generally suggest using the "simpleviewer", or "singlepageviewer", since it does *most* things out-of-the-box and given that a lot of functionality really require *a viewer* and not just a single page in order to work.
Please note that this functionality has never really mattered for the Firefox PDF Viewer, the GENERIC viewer, or even the "simpleviewer"/"singlepageviewer" component-examples. Hence, in practice this means that only the "pageviewer" component-example[1] have ever really utilized this.
Using factories to initialize various layers in the viewer, rather than simply invoking the relevant code directly, seems (at least to me) like a somewhat roundabout way of doing things.
Not only does this lead to more code, both to write and maintain, but since many of the layers have common parameters (e.g. an `AnnotationStorage`-instance) there's also some duplication.
Hence this patch, which removes the `textLayerFactory` and instead uses a lookup-function in the `PDFPageView`-class to access the external viewer-properties as necessary.
Note that this should even be an improvement for the "pageviewer" component-example, since most layers will now work by default rather than require manual configuration.
---
[1] In practice we generally suggest using the "simpleviewer", or "singlepageviewer", since it does *most* things out-of-the-box and given that a lot of functionality really require *a viewer* and not just a single page in order to work.
Please note that this functionality has never really mattered for the Firefox PDF Viewer, the GENERIC viewer, or even the "simpleviewer"/"singlepageviewer" component-examples. Hence, in practice this means that only the "pageviewer" component-example[1] have ever really utilized this.
Using factories to initialize various layers in the viewer, rather than simply invoking the relevant code directly, seems (at least to me) like a somewhat roundabout way of doing things.
Not only does this lead to more code, both to write and maintain, but since many of the layers have common parameters (e.g. an `AnnotationStorage`-instance) there's also some duplication.
Hence this patch, which removes the `textHighlighterFactory` and instead uses a lookup-function in the `PDFPageView`-class to access the external viewer-properties as necessary.
Note that this should even be an improvement for the "pageviewer" component-example, since most layers will now work by default rather than require manual configuration.
---
[1] In practice we generally suggest using the "simpleviewer", or "singlepageviewer", since it does *most* things out-of-the-box and given that a lot of functionality really require *a viewer* and not just a single page in order to work.
Please note that this functionality has never really mattered for the Firefox PDF Viewer, the GENERIC viewer, or even the "simpleviewer"/"singlepageviewer" component-examples. Hence, in practice this means that only the "pageviewer" component-example[1] have ever really utilized this.
Using factories to initialize various layers in the viewer, rather than simply invoking the relevant code directly, seems (at least to me) like a somewhat roundabout way of doing things.
Not only does this lead to more code, both to write and maintain, but since many of the layers have common parameters (e.g. an `AnnotationStorage`-instance) there's also some duplication.
Hence this patch, which removes the `structTreeLayerFactory` and instead uses a lookup-function in the `PDFPageView`-class to access the external viewer-properties as necessary.
Note that this should even be an improvement for the "pageviewer" component-example, since most layers will now work by default rather than require manual configuration.
---
[1] In practice we generally suggest using the "simpleviewer", or "singlepageviewer", since it does *most* things out-of-the-box and given that a lot of functionality really require *a viewer* and not just a single page in order to work.
Please note that this functionality has never really mattered for the Firefox PDF Viewer, the GENERIC viewer, or even the "simpleviewer"/"singlepageviewer" component-examples. Hence, in practice this means that only the "pageviewer" component-example[1] have ever really utilized this.
Using factories to initialize various layers in the viewer, rather than simply invoking the relevant code directly, seems (at least to me) like a somewhat roundabout way of doing things.
Not only does this lead to more code, both to write and maintain, but since many of the layers have common parameters (e.g. an `AnnotationStorage`-instance) there's also some duplication.
Hence this patch, which removes the `annotationLayerFactory` and instead uses a lookup-function in the `PDFPageView`-class to access the external viewer-properties as necessary.
Note that this should even be an improvement for the "pageviewer" component-example, since most layers will now work by default rather than require manual configuration.
---
[1] In practice we generally suggest using the "simpleviewer", or "singlepageviewer", since it does *most* things out-of-the-box and given that a lot of functionality really require *a viewer* and not just a single page in order to work.
Please note that this functionality has never really mattered for the Firefox PDF Viewer, the GENERIC viewer, or even the "simpleviewer"/"singlepageviewer" component-examples. Hence, in practice this means that only the "pageviewer" component-example[1] have ever really utilized this.
Using factories to initialize various layers in the viewer, rather than simply invoking the relevant code directly, seems (at least to me) like a somewhat roundabout way of doing things.
Not only does this lead to more code, both to write and maintain, but since many of the layers have common parameters (e.g. an `AnnotationStorage`-instance) there's also some duplication.
Hence this patch, which removes the `annotationEditorLayerFactory` and instead uses a lookup-function in the `PDFPageView`-class to access the external viewer-properties as necessary.
Note that this should even be an improvement for the "pageviewer" component-example, since most layers will now work by default rather than require manual configuration.
---
[1] In practice we generally suggest using the "simpleviewer", or "singlepageviewer", since it does *most* things out-of-the-box and given that a lot of functionality really require *a viewer* and not just a single page in order to work.
Currently we'll only initialize and render the `annotationEditorLayer` once the regular `annotationLayer` has been rendered.
While it obviously makes sense to render the `annotationEditorLayer` *last*, the way that the code is currently written means that if a third-party user disables the `annotationLayer` then the editing-functionality indirectly becomes disabled as well.
Given that this seems like a somewhat arbitrary limitation, this patch simply decouples these two layers while still keeping the rendering order consistent.
By moving this code the "pageviewer"-component example will become slightly more usable on its own, it may simplify a future addition of XFA Foreground document support, and finally also serves as preparation for the following patches.
The container position and dimensions should be almost constant, hence
it's pretty useless to query them on each rescale.
Finally it avoids to trigger some reflows.
First of all, given the screen-sizes of most mobile phones using Spread modes is unlikely to be useful.
Secondly, and more importantly, since there's (currently) no UI available for the user to override a PDF document-specified Spread mode this would result in a bad UX otherwise.
Also, removes an outdated comment from the `apiPageLayoutToViewerModes` helper function.
Depending on e.g. the `textLayerMode` option it might not actually be necessary to always initialize this eagerly.
*Please note:* Unfortunately we cannot `shadow` a private field, hence why this is only made semi-"private".
It's a follow-up of #14950: some format actions are ran when the document is open
but we must be sure we've everything ready for that, hence we have to run some
named actions before runnig the global format.
In playing with the form, I discovered that the blur event wasn't triggered when
JS called `setFocus` (because in such a case the mouse was never down). So I removed
the mouseState thing to just use the correct commitKey when blur is triggered by a
TAB key.
For pages without any annotations, applies e.g. to the `tracemonkey.pdf` document, we'll repeatedly try to re-create the `annotationLayer` on every zoom and rotation operation.
The reason that this happens is because we don't insert the `annotationLayer`-div into the DOM unless there's annotations present on the page, which thus means that we miss the existing `annotationLayer`-caching present in the `PDFPageView` implementation.
This is a very old issue, and the easiest solution is to simply always insert an *empty* (and hidden) `annotationLayer`-div such that the existing code/caching starts working for the "no annotations" case as well.
Note that this is consistent with other layers, since e.g. the `textLayer` and/or `annotationEditorLayer` may be empty. Given that only a limited, by default ten, number of pages are ever active at once the additional DOM-elements shouldn't effect things negatively here.
This is consistent with the `render` methods of the other layers, and reduces overall indentation in the method.
Furthermore, don't "swallow" errors since the `PDFPageView._renderXfaLayer` method is already able to deal with that.
It doesn't seem necessary to have a *separate* `destroy` method given that the `cancel` method always invokes it unconditionally.
In the `PDFPageView.reset` method we currently attempt to call `destroy` directly, however that'll never actually happen since either:
- We're keeping the annotationEditorLayer, in which case we're just hiding the layer and nothing more (and the relevant branch is never entered).
- We're removing the annotationEditorLayer, in which case the `PDFPageView.cancelRendering` method has already cancelled *and* nulled it (and there's thus nothing left to `destroy` here).
*Please note:* Hopefully I'm not overlooking something obvious here, since both reading through the code *and* also adding `console.log(this.annotationEditorLayer);` [before this line](9d4aadbf7a/web/pdf_page_view.js (L438)) suggests that it's indeed unnecessary.
In PR 14877 I forgot to update the horizontal padding, used when computing the scale of the pages, for the case where SpreadModes and PresentationMode are being used together.
Steps to reproduce:
1. Open the viewer with the default `tracemonkey.pdf` document.
1. Enable any SpreadMode.
2. Rotate the document *once*, either clockwise or counterclockwise.
3. Enter PresentationMode.
4. Try swithching page, e.g. by clicking on the document.
Expected result:
The visible pages change as you click.
Actual result:
The visible pages are "stuck" in the current view.
The `PDFPageProxy._pageIndex` property is a "private" one that shouldn't be accessed, since it could theoretically break tomorrow if we re-factor the relevant API code.
Also, try to clean-up and improve consistency in a couple of JSDoc comments.
This change was made in PR 5552, however I cannot tell why we needed to disable searching in PresentationMode. Furthermore, with the changes in PR 13908 which effectively moved where this code is invoked, searching has now (accidentally) been working in PresentationMode in e.g. the Firefox PDF Viewer for well over a year.
So, let's just enable searching unconditionally in PresentationMode to simplify the code.
An annotation editor layer can be destroyed when it's invisible, hence some
annotations can have a null parent but when printing/saving or when changing
font size, color, ... of all added annotations (when selected with ctrl+a) we
still need to have some parent properties especially the page dimensions, global
scale factor and global rotation angle.
This patch aims to remove all the references to the parent in the editor instances
except in some cases where an editor should obviously have one.
It fixes#15780.
Rather than handling these parameters separately, which is a left-over from back when streaming of textContent was originally added, we can simply pass either data directly to the `TextLayer` and let it handle things accordingly.
Also, improves a few JSDoc comments and `typedef`-imports.
Compared to the recent PR 15722 for the `textLayer` this one should be a (comparatively) much a smaller win overall, since most documents don't have any structTree-data and the required parsing should be cheaper. However, it seems to me that it cannot hurt to improve this nonetheless.
Note that by moving the `structTreeLayer` initialization we remove the need for the "textlayerrendered" event listener, which thus simplifies the code a little bit.
Also, removes the API-caching of the structTree-data since this was basically done to offset the lack of caching in the viewer.
The idea is just to resuse what we got on the first draw.
Now, we only update the scaleX of the different spans and the other values
are dependant of --scale-factor.
Move some properties in the CSS in order to avoid any updates in JS.
The deprecation is included in the current release, i.e. version `3.1.81`, and given the edge-case nature of this option I really don't think that we need to keep it deprecated for multiple releases.
This patch has been successfully tested in a local, artifact, Firefox build.
*Please note:* The only thing that'll no longer work for PDF documents opened using "data:"-URLs is middle-clicking on internal/outline links, in order to open the destination in a new tab. This is however an extremely small loss of functionality, and as can be seen in the bug the alternative (i.e. doing nothing) is surely much worse.
Add a deprecation notification for PDFDocumentLoadingTask.onUnsupportedFeature and PDFDocumentProxy.stats
which are likely useless.
The unsupported feature stuff have initially been added in (#4048) in order to be able to display a
warning bar and to help to have some numbers to know how a feature was used.
Those data are no more used in Firefox.
The reason for the issue is that we use the generic `getFilenameFromUrl` helper function, which was originally intended for regular URLs.
For the filenames we're dealing with in FileAttachments, we really only want to strip the path when one exists[1].
---
[1] See [bug 1230933](https://bugzilla.mozilla.org/show_bug.cgi?id=1230933) for an example of such a case.
With the changes made in PR 14564 this *should* no longer be necessary now, however we still need to keep the `scrollMatches` parameter to handle textLayers with markedContent correctly when searching.
Adding some logging with `console.{time, timeEnd}` around all the constant definitions at the top of the `web/pdf_find_controller.js` file, I noticed that computing `DIACRITICS_EXCEPTION_STR` took close to half the total time.
My first idea was just to try and make it slightly more efficient, by reducing the amount of iterations and intermediate allocations. However, with this constant only being used during "match diacritics" searches it thus seemed like a good candidate for lazy initialization.
*Please note:* Given that this is a micro optimization, I fully understand if the patch is rejected.
Given that the new sidebar icon is slightly shorter than the old one, it cannot hurt to ever so slightly tweak the vertical position of the notification icon.
(While the patch also changes the CSS rule used for the horizontal position, this is a no-op and was done to improve consistency between the two values.)
The way that we set the width of the `dropdownToolbarButton`-select is very old, and despite some improvements over the years this is still somewhat hacky.
In particular, note how we're assigning the select-element a larger width than its containing `dropdownToolbarButton`-element. This was done to prevent displaying *two* separate icons, i.e. the native and the PDF.js one, since it's the only way to handle this in older browsers (particularly Internet Explorer).
Given the currently supported browsers, there's however a better solution available: use `appearance: none;` to disable native styling of the select-element. [According to MDN](https://developer.mozilla.org/en-US/docs/Web/CSS/appearance#browser_compatibility), this is supported in all reasonably modern browsers.
This way we're able to simplify both the CSS rules and the JS-code that's used to adjust the `dropdownToolbarButton` width in a localization aware way.
Because of https://bugzilla.mozilla.org/show_bug.cgi?id=1582545, the padding-inline is by default 0.
0 is not really enough because of the outline, so just set it to 2px (it was 4px before the patch)
in order to have something visually correct.
This patch first of all makes `isOffscreenCanvasSupported` configurable, defaulting to `true` in browsers and `false` in Node.js environments, with a new `getDocument` parameter. While you normally want to use this, in order to improve performance, it should still be possible for users to control it (similar to e.g. `isEvalSupported`).
The specific problem, as reported in issue 14952, is that the SVG back-end doesn't support the new ImageMask data-format that's introduced in PR 14754. In particular:
- When the SVG back-end is used in Node.js environments, this patch will "just work" without the user needing to make any code changes.
- If the SVG back-end is used in browsers, this patch will require that `isOffscreenCanvasSupported: false` is added to the `getDocument`-call.
While it can't hurt to localize the main error-messages, also localizing the error *details* has always seemed somewhat unnecessary since those are only intended for debugging/development purposes. However, I can understand why that's done since the GENERIC viewer used to expose this information in the UI; via the `errorWrapper` UI that's removed in PR 15533.
At this point, when any errors are simply logged in the console, it no longer seems necessary to keep localizing the error *details* in the default viewer.
*Please note:* The referenced issue is the only mention that I can find, in either GitHub or Bugzilla, of "GoToE" actions.
Hence why I've purposely settled for a very simple, and partial, "GoToE" implementation to avoid complicating things initially.[1] In particular, this patch only supports "GoToE" actions that references the /EmbeddedFiles-dict in the PDF document.
See https://web.archive.org/web/20220309040754if_/https://www.adobe.com/content/dam/acom/en/devnet/pdf/pdfs/PDF32000_2008.pdf#G11.2048909
---
[1] Usually I always prefer having *real-world* test-cases to work with, whenever I'm implementing new features.
In the Firefox PDF Viewer this has never been used, with the error message simply printed in the web-console, and (somewhat) recently we've also updated the viewer code to avoid bundling the relevant code there. Furthermore, in the Firefox PDF Viewer we're not even display the *browser* fallback bar any more; see https://bugzilla.mozilla.org/show_bug.cgi?id=1705327.
Hence it seems slightly strange to keep this UI around in the GENERIC viewer, and this patch proposes that we simply remove it to simplify/unify the relevant code in the viewer. In particular this also allows us to remove a couple of l10n-strings, which have always been unused in the Firefox PDF Viewer.
Rather than "manually" looking up the l10n-string and then updating the button, we can (and probably even should) just update the l10n-id and then trigger proper translation for the button DOM-element.
This extends the approach used in PresentationMode to also cover the AnnotationEditor, and tries to handle the combination of both cases correctly.
In order to simplify the overall implementation we simply track the *first* seen "previous" cursorTool, and don't allow it to be reset as long as either PresentationMode or an AnnotationEditor is being used.
Currently the `viewBookmark`-button, which is actually a `href`-element, gets an inconsistent `outline`.
Similarly, the `dialog`-buttons also have an inconsistent `outline` after the changes in PR 15438.
Finally, simplifies a couple of `border` rules since setting a border-width when "none" is being used doesn't seem meaningful.
The changes in PR 15438 added a `border-radius` when input-elements are focused, however there's no radius when the same elements are hovered. Having the radius change, and not just the `border-color`, when input goes from hovered to focused feels a bit inconsistent (at least to me).
Some z-index have been added in the annotation layer because the elements inside are re-ordered
in order to improve accessibility.
Hence we must add a "high" z-index on the annotation editor layer in order to avoid any bad
interaction between the different layers.
The default outline for a focused text input is not that bad but for any reason when changing
the background color, all the good default border/outline properties are lost (it's the same
behaviour in Edge).
So in order have something consistent in HCM/non-HCM, a 2px-border+1px-outline (on @MReschenberg
advices) is added when an input is focused with different colors depending on HCM.
While working on the above issue, I noticed few bugs I fixed when in HCM:
- input, button and select have some default properties which have been created at a time where
annotation layer didn't exist, hence this patch remove them and set those properties where
they should live;
- some elements (like the main toolbar) is using a box-shadow which is invisible in HCM, hence
it's replaced by a border-bottom in HCM;
- some separators are invisible in HCM, hence use GrayText color to render them correctly;
- the options for the zoom selection were invisible in HCM with Desert (one of the Windows 11
themes).
By force-quitting the browser while the FullScreen API is active, we don't get a chance to exit PresentationMode *cleanly* and some of its state thus remains (via the `ViewHistory`).
To try and improve things here we can skip updating the Scroll/Spread-mode while PresentationMode is active, since they will be changed when entering PresentationMode, which seems to help and is really the best that we can do here (and what the issue describes is very much an edge-case anyway).
There's no point in having this variable defined (implicitly) as `undefined` in e.g. the Firefox PDF Viewer.
By defining it with `var` and using an ESLint ignore, rather than `let`, we can move it into the relevant pre-processor block instead. Note that since the entire viewer-code is placed, by Webpack, in a top-level closure this variable will thus not become globally accessible.
