Currently the background-color of the `editorParamsToolbar`s don't match that of the arrow, which is especially noticable in dark mode (see zoomed-in screen-shots below).
The simplest solution seem to be to just style the `editorParamsToolbar`s like the `secondaryToolbar`, to limit the amount of CSS changes required.
This should *hopefully* fix 17228, by tweaking the build scripts to give the GENERIC viewer something to await to avoid breaking third-party users of the standalone viewer components.
This button is *only* used in the GENERIC viewer, and will currently be visible either in the main or secondary toolbars (depending on the viewer width).
To simplify upcoming changes, and to avoid then having to complicate the relevant CSS rules unnecessarily, let's place the "Open file"-button permanently in the secondary toolbar instead.
(Note that the GENERIC viewer also, since five years, supports drag-and-drop in order to open local files.)
The `viewerCssTheme`-implementation has always been somewhat hacky, and now it's also *partially* broken ever since we've started using CSS nesting.
Trying to support nested media queries would thus require a lot more parsing of the CSS rules, which seems inefficient and thus generally undesirable.[1]
As discussed on Matrix, let's try to remove the `viewerCssTheme`-option and see if there's any (significant) fallout from this.
---
[1] If this option is brought back, it seems to me that it (in Firefox) should probably be set through the platform-code that handles theming.
Depending on the structure of the outline we could potentially need to expand a few levels, especially in long PDF documents, hence it cannot hurt to pause translation in that case as well.
With the changes in PR 17208, where browser-preferences are now handled as "regular" viewer-options, we can tweak the definition of `canvasMaxAreaInBytes` to slightly simplify things in the `PDFViewerApplication.open` method.
Given that this branch is only necessary in development mode and *during* building, but is never actually used in the final viewer-bundles, we can utilize the pre-processor to ignore this code.
Currently we *synchronously* fetch a number of browser preferences/options, from the platform code, during the viewer respectively PDF document initialization paths.
This seems unnecessary, and we can re-factor the code to instead include the relevant data when fetching the regular viewer preferences.
Note that we must append the textLayer to the DOM *before* enabling the `highlighter` and `accessibilityManager`, to avoid breaking e.g. a pending searching operation.
The least invasive solution, that I was able to come up with, is to introduce a new `TextLayerBuilder` callback-function for this purpose.
After PR 17177 the interface of `XfaLayerBuilder` is now inconsistent, since whether or not we directly append the xfaLayer to the DOM now depends on the rendering intent.
I forgot to include `web/l10n_utils.js` in PR 17161, which currently breaks `ConstL10n` since there's no longer a method called `setL10n`; sorry about that!
Most of the strings shouldn't contain special chars (<= 0x1F) so we can
have a fast path which just checks if the string contains at least one such
a char.
To prevent the *standalone* viewer-components from breaking, we need to ensure that the `NullL10n`-interface won't accidentally diverge from the actual `L10n`-implementations.
Looking at the `PDFThumbnailView.setPageLabel` method you'll see that we update e.g. the "aria-label" of the thumbnail-image for documents that contain (valid) pageLabels.
This isn't done in `PDFPageView`, which seems inconsistent, hence this patch.
This patch changes almost all viewer-components[1] to use "data-l10n-id"/"data-l10n-args" for localization, which means that in many cases we no longer need to pass around the `L10n`-instance any more.
One part of the code-base where the `L10n`-instance is still being used "directly" is the AnnotationEditors, however while it might be possible to convert (most of) that code as well that's not attempted in this patch.
---
[1] The one exception is the `PDFDocumentProperties` dialog, since the way it's currently implemented makes that less straightforward to fix without a lot of code changes.
*Please note:* In the Firefox PDF Viewer this findbar is only used for PDF documents placed in e.g. `<iframe>` elements.
By registering a `ResizeObserver` when the `PDFFindBar` is open we slightly unify and simplify how the findbar layout (row vs column) is handled.
This will be especially helpful with upcoming changes, where we'll make use of "data-l10n-id"/"data-l10n-args" to trigger translation in the viewer.
- The old translation engine handled language code casing slightly differently, hence we need to tweak the non-metric locale check in `PDFDocumentProperties` to account for that.
- Use only lowercase names for the pre-defined page names, to improve overall consistency.
*Please note:* These changes only affect the GENERIC build, since `NullL10n` is only a stub elsewhere (see PR 17135).
After the changes in PR 17115, which modernized and improved l10n-handling, the `NullL10n`-implementation is no longer a good fallback for the "proper" `L10n`-classes.
To improve this situation, especially for the *standalone* viewer-components, this patch makes the following changes:
- Let the `NullL10n`-implementation extend an actual `L10n`-class, which is constant and lazily initialized, to ensure that it works *exactly* like the "proper" ones.
- Automatically bundle the "en-US" l10n-strings in the build, via the pre-processor, such that we don't need to remember to manually update them.
- Ensure that the *standalone* viewer-components register their DOM-elements for translation, similar to the default viewer, since this will allow future code improvements by using "data-l10n-id"/"data-l10n-args" in most (if not all) parts of the viewer.
- Remove the `NullL10n` from the `AnnotationLayer`, to avoid affecting bundle size too much.
For third-party users that access the `AnnotationLayer`, as exposed in the main PDF.js library, they'll now need to *manually* register it for translation. (However, the *standalone* viewer-components still works given the point above.)
Given that there's now a bit more asynchronicity in the l10n-initialization in the Firefox PDF Viewer, after PR 17115, try to limit the impact of that by moving it to occur a tiny bit earlier in the default viewer initialization.
In Firefox debug builds, there is an assertion to check that we don't connect
a subelement of an already connected root. Thanks to this assertion, we can see
that the root has already been added to Fluent, hence we don't need to do it
a second time.
We don't need to await anymore on the translation in order to update the
toolbar: it'll be done by Fluent, so we can safely remove the "localized"
event and avoid to wait for it.
*Please note:* This patch contains a couple of micro-optimizations, hence I understand if it's deemed unnecessary.
Move the `AppOptions` initialization into the `Preferences` constructor, since that allows us to remove a couple of function calls, a bit of asynchronicity and one loop that's currently happening in the early stages of the default viewer initialization.
Finally, move the `Preferences` initialization to occur a *tiny* bit earlier since that cannot hurt given that the entire viewer initialization depends on it being available.
Note that CSS-features such as e.g. `flex` didn't exist, or had poor cross-browser support, back when the JavaScript-based solution was initially implemented.
- For the generic viewer we use @fluent/dom and @fluent/bundle
- For the builtin pdf viewer in Firefox, we set a localization url
and then we rely on document.l10n which is a DOMLocalization object.
Given that we only use standard `import`/`export` statements now, after recent PRs, the "exports" global is unused.
Instead we add "__non_webpack_import__" to the `globals` to avoid having to sprinkle disable statements throughout the code.
Finally, the way that `globals` are defined has changed in ESLint and we should thus explicitly specify them as "readonly"; please find additional details at https://eslint.org/docs/latest/use/configure/language-options#specifying-globals