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.
After the changes in PR 15391 one separator may now become visible too soon when the viewer is narrow, applies e.g. to the MOZCENTRAL viewer, since the wrong CSS class is being used.
The reason that this happens is that only the GENERIC viewer includes the "openFile"-buttons, and we thus need the separator to also be conditionally defined.
This is a slightly speculative change, based on something that I happened to notice while browsing MDN, to hopefully prevent PDF.js from outright breaking in older browsers.
According to the following information on MDN, Safari didn't implement support for the necessary features until version 14:
- https://developer.mozilla.org/en-US/docs/Web/API/MediaQueryList#browser_compatibility
- https://developer.mozilla.org/en-US/docs/Web/API/MediaQueryList/change_event#browser_compatibility
Given the browsers that we currently support only older versions of Safari should be affected, hence it seems reasonable to simply disable the functionality rather than trying to polyfill it.
(It's interesting how it's very often Safari which is *much* slower than the other browsers at implementing new features.)
After the changes in PR 14112 the `PDFViewer`-class is now "identical" to the `BaseViewer`-class and the `PDFSinglePageViewer`-class is just a very thin wrapper around the `BaseViewer`-class.
Hence we can rename these files, and also remove the abstract `BaseViewer`-class, which helps reduce some unnecessary "closures" in the *built* viewer.
*Please note:* These changes are made in two separate commits, to allow GitHub to preserve `blame` for the affected files.
After the changes in PR 14112 the `PDFViewer`-class is now "identical" to the `BaseViewer`-class and the `PDFSinglePageViewer`-class is just a very thin wrapper around the `BaseViewer`-class.
Hence we can rename these files, and also remove the abstract `BaseViewer`-class, which helps reduce some unnecessary "closures" in the *built* viewer.
*Please note:* These changes are made in two separate commits, to allow GitHub to preserve `blame` for the affected files.
This patch updates a bunch of older code, that makes conditional function calls, to use optional chaining rather than `if`-blocks.
These mostly mechanical changes reduce the size of the `gulp mozcentral` build by a little over 1 kB.
Note that this patch implements the `SetOCGState`-handling in `PDFLinkService`, rather than as a new method in `OptionalContentConfig`[1], since this action is nothing but a series of `setVisibility`-calls and that it seems quite uncommon in real-world PDF documents.
The new functionality also required some tweaks in the `PDFLayerViewer`, to ensure that the `layersView` in the sidebar is updated correctly when the optional-content visibility changes from "outside" of `PDFLayerViewer`.
---
[1] We can obviously move this code into `OptionalContentConfig` instead, if deemed necessary, but for an initial implementation I figured that doing it this way might be acceptable.
Apparently this is implemented in e.g. Adobe Reader, and the specification does support it, however it cannot be commonly used in real-world PDF documents since it took over ten years for this feature to be requested.
There's three notable exceptions here:
- The `saveDocument` one is converted into a permanent `warn`, since it still works when the `annotationStorage` is empty although it's (obviously) less efficient than `getData`.
- The `fallbackWorkerSrc` functionality (for browsers), since just removing it would risk too much third-party breakage.
- The SVG back-end, since a final decision is yet to be made. (It might be completely removed, or left as-is in an essentially "frozen" state.)
Note that this patch prepends the document title with "* ", rather than only "*" as suggested in the bug, since there's nothing that says that a PDF document cannot specify a title[1] beginning with an asterisk. To reduce possible confusion, having a space between the "editing marker" and the actual document title thus cannot hurt as far as I'm concerned.
In order to notify the viewer when all `AnnotationEditor`s have been removed, we utilize the existing `onAnnotationEditor`-callback to allow the document title to be updated as necessary.
Finally, this patch makes the following (slightly unrelated) changes:
- Rename the `AnnotationStorage.removeKey` method to just `AnnotationStorage.remove` instead. This is consistent with e.g. the `has`-method and should suffice to explain what it does.
- Remove the `AnnotationStorage.hasAnnotationEditors` getter, since the viewer now tracks the necessary state internally. This avoids unnecessarily having to iterate through the `AnnotationStorage`-instance when saving/printing the document.
---
[1] Using either an /Info dictionary or a /Metadata stream.
The password dialog can be cancelled in three different ways:
- By clicking on its "Cancel"-button.
- By pressing the Escape-key.
- By force-opening another dialog, although this shouldn't happen in practice.
Here the "Cancel"-button case is slightly special since it'll trigger `PasswordPrompt.#cancel` *twice*, first directly via the click and secondly via the "close" event on the `dialog`-element.
While this shouldn't, as far as I know, cause any bugs it's nonetheless inconsistent with the other cases outlined above. To improve this we can simply attempt to *close* the password dialog instead, and then rely on the "close" event to run the `PasswordPrompt.#cancel` method.
Currently in `disableWorker=true` mode it's possible that opening of password-protected PDF documents outright fails, if an *incorrect* password is entered. Apparently the event ordering is subtly different in the non-Worker case, which causes the password-callback to be updated *before* the dialog has been fully closed.
To avoid that we'll utilize a `PromiseCapability` to keep track of the state of the password dialog, such that we can delay both re-opening and (importantly) updating of the password-callback until doing so is safe.
This patch *may* also fix issue 15330, but it's impossible for me to tell.
*This is a follow-up to PR 14869.*
In the old code we're accidentally "swallowing" part of the event-details, which explains why the annotationLayer didn't render.
One thing that made debugging a lot harder was the lack of error messages, from the viewer, and a few `PDFPageView`-methods were updated to improve this situation.
This was moved into the `src/display/`-folder in PR 15110, for the initial editor-a11y patch. However, with the changes in PR 15237 we're again only using `binarySearchFirstItem` in the `web/`-folder and it thus seem reasonable to move it back there.
The primary reason for moving it back is that `binarySearchFirstItem` is currently exposed in the public API, and we always want to avoid that unless it's either PDF-related functionality or code that simply must be shared between the `src/`- and `web/`-folders. In this case, `binarySearchFirstItem` is a general helper function that doesn't really satisfy either of those alternatives.
Currently when the `TextAccessibilityManager.enabled` method is called, we'll update `aria-owns` for any pre-existing elements. This obviously makes sense when e.g. zooming/rotating in the viewer, since the annotationLayer/annotationEditorLayer is kept in those cases.
However when the page is *fully* reset, e.g. as result of going out-of-view and thus being evicted from the cache, we still keep the `#textNodes`-Map around. This causes us to set the `aria-owns` attribute (in the textLayer) for an element that doesn't actually exist any more, which as far as I'm concerned seems incorrect. In this case the element will simply, as already implemented, be re-inserted when the annotationLayer/annotationEditorLayer renders again.
This patch doesn't structurally change the text layer: it just adds some aria-owns
attributes to some spans.
The aria-owns attribute expect to have an element id, hence it's why it adds back an
id on the element rendering an annotation, but this id is built in using crypto.randomUUID
to avoid any potential issues with the hash in the url.
The elements in the annotation layer are moved into the DOM in order to have them in the
same "order" as they visually are.
The overall goal is to help screen readers to present to the user the annotations as
they visually are and as they come in the text flow.
It is clearly not perfect, but it should improve readability for some people with visual
disabilities.
This exports the same constants as the viewer components, but in the default viewer. To avoid bloating the global-scope the constants are added to a new `PDFViewerApplicationConstants` object[1], which also allows us to skip this in builds where it's not actually needed (e.g. the Firefox *built-in* PDF Viewer).
*Please note:* I'm not completely sold on this idea, and thus wouldn't mind the patch being rejected, since we probably don't want to export every single viewer constant this way. (And it may seem a bit arbitrary, to users, why some constants are exported and others are not.)
---
[1] Somewhat similar to the existing `PDFViewerApplicationOptions` structure.
In addition to the existing `LinkTarget` constant, used by the `PDFLinkService`-constructor, this patch exports the following constants in the viewer components:
- `ScrollMode` and `SpreadMode`, since the `BaseViewer` has getters/setters which work with those constants.
- `RenderingStates`, since that one may be helpful when using `PDFPageView` directly.
By invoking the `reset` methods *last* in the `Toolbar`/`SecondaryToolbar`-constructors, we ensure that the "toolbarreset"/"secondarytoolbarreset"-events are actually handle when the viewer loads. Note that previously those events were dispatched *before* the relevant event-listeners had been attached.
With this small change we can avoid inconsistent initial toolbar-state, specifically in the case when the viewer is *reloaded* (since Firefox keeps the HTML-state on "soft" reloads).
An annotation doesn't have to be in the text flow, hence it's likely a bad idea
to insert its text in the text layer. But the text must be visible from a screen
reader point of view so it must somewhere in the DOM.
So with this patch, the text from a FreeText annotation is extracted and added in
a div in its HTML counterpart, and with the patch #15237 the text should be visible
and positioned relatively to the text flow.
Given that this image is intended specifically for the default viewer, we simply use the CSS preprocessor to remove the image reference in the `gulp components` build.
Considering that the issue only affects a CSS file, I don't believe that replacing the *just released* PDF.js version is actually necessary here.
It doesn't make sense to use a page-canvas that's *smaller* than the resulting thumbnail, since that causes the image to be upscaled which results in a blurry thumbnail. Note that this doesn't normally happen, unless a very small zoom-level is used in the viewer.
