- Simplify how we look-up the DOM-element, which should also be a tiny bit more efficent.
- Use private class-fields, rather than property-names prefixed with underscores.
- Inline the `#updateBar` helper-method directly in the `percent`-setter, since having a separate method doesn't seem necessary in this case.
- Set the `indeterminate`-class on the ProgressBar DOM-element, to simplify the code.
Finally, also (slightly) re-factors the `PDFViewerApplication.progress`-method to make it a bit smaller.
Similar to other recent patches, see e.g. PR 15057, we don't want to add these kind of `id`s to DOM-elements since they shouldn't become "linkable" through the URL hash.
*Please note:* This patch can be tested, in the viewer, with e.g. `bug1737260.pdf` from the test-suite.
- Approximate the drawn curve by a set of Bezier curves in using
js code from https://github.com/soswow/fit-curves.
The code has been slightly modified in order to make the linter
happy.
Given the differences between XFA documents and "normal" PDF documents, we don't support editing of the former ones. Hence, when a XFA-document is opened, we temporarily disable the editor-buttons.
The original `ProgressBar`-functionality is very old, and could thus do with some general clean-up.
In particular, while it currently accepts various options those have never really been used in either the default viewer or in any examples. The sort of "styling" that these options provided are *much better*, not to mention simpler, done directly with CSS rules.
As part of these changes, the "progress" is now updated using CSS variables rather than by directly modifying the `style` of DOM elements. This should hopefully simplify future changes to this code, see e.g. PR 14898.
Finally, this also fixes a couple of other small things in the "mobile viewer" example.
An old shortcoming of the `preprocessCSS`-function is its complete lack of support for our "normal" defines, which makes it very difficult to have build-specific CSS rules. Recently we've started using specially crafted comments to remove CSS rules from the MOZCENTRAL build, but (ab)using the `preprocessCSS`-function in this way really doesn't feel great.
However, it turns out to be surprisingly simple to instead use the "regular" `preprocess`-function for the CSS files as well. The only special-handling that's still necessary is the helper-function for dealing with CSS-imports, but apart from that everything seems to just work.
One reason, as far as I can tell, for having a separate `preprocessCSS`-function was likely that we originally used *lots* of vendor-prefixed CSS rules in our CSS files. With improvements over the years, especially thanks to Autoprefixer and PostCSS, we've been able to remove *almost* all non-standard CSS rules and the need for special-casing the CSS parsing has mostly vanished.
*Please note:* As part of testing this patch I've diffed the output of `gulp generic`, `gulp mozcentral`, and `gulp chromium` against the `master`-branch to check that there was no obvious breakage.
This new CSS variable will allow us to simplify a couple of different viewer components, since we no longer need to use JavaScript-based hacks and can directly set the CSS rules instead. In particular:
- The `BaseViewer`-handling, used as part of the code that will center pages *vertically* in PresentationMode, can be simplified.
By using CSS to control the height of the "dummy"-page we avoid unnecessarily invalidating the scale-value, which can reduce *some* unneeded re-rendering while PresentationMode is active.
- The `SecondaryToolbar.#setMaxHeight`-method, and its associated parameters, are no longer necessary and can be completely removed.
Note that in order for things to work correctly in general, the new `--viewer-container-height` CSS variable must potentially be updated on any window-based "resize"-event (even when there's no zooming). While this is currently only done in the default viewer, that shouldn't be an issue since neither PresentationMode nor Toolbar-functionality is included in the "viewer components".
Currently we're using *both* ids and classes when specifying the button icons, which seems completely unnecessary and only serves to bloat the CSS code.
In general you always need to be careful about CSS specificity, but in these cases that should not actually be a problem.
Also, while not a button, this patch makes a similar simplification for the `pageNumber`-input.
Note how in the `viewer.html` file we're specifying class names for most buttons, despite that not really being necessary. First of all, in many cases those class names are *identical* to the element-ids. Secondly, looking through the CSS rules they are only ever used when specifying button icons.
All-in-all, we should be able to simplify the HTML-code and use the element-ids in the CSS rules instead. Obviously ids have a higher CSS specificity than classes, but given how the old classes were being used that shouldn't be a problem here.
Also, the patch generalizes the styling for buttons (e.g. `viewBookmark`) that are *actually* link-elements.
Finally, while slightly unrelated, this patch also removes a little bit of duplication when specifying the border for `toolbarField`s.
Searching through all of the files in the `web/`-folder has no *other* hits for the "textButton" string. Hence it's clear that there are no DOM elements actually using this class, and it's thus dead code.
This re-factors the various toolbar separators to *explicitly* specify both their dimensions and margins. Also, for the `horizontalToolbarSeparator`-class we can just set the `background-color` rather than using `border-top`.
Note that the `splitToolbarButtonSeparator`-class currently sets a number of unnecessary CSS rules, since as mentioned by the Firefox Devtools both the `display`- and `z-index`-properties are being ignored because `float` is used.
Finally, there's also no need to set a `z-index` for some of the `:hover`-rules. It's possible that this *was* necessary before the re-design, since the buttons had borders then.
Note how both of the openFile-buttons are always hidden during viewer initialization in the MOZCENTRAL build, i.e. the *built-in* Firefox PDF Viewer. Despite that we still include HTML, CSS, and JavaScript code for these buttons in the build.
This patch *reduces* the size of the `gulp mozcentral` output by `1679` bytes, which isn't a lot but still cannot hurt.
These rules became unnecessary with PR 7697, over five years ago, since printing is now done from a `printContainer`-element rather than "directly" using the viewer.
Note how the *entire* `outerContainer`, which contains all of the DOM elements that were being manually hidden, is now being hidden during printing. Furthermore, note also how the print-canvases/images and their containers are using custom CSS-classes[1] rather than re-using the ones from the viewer.
---
[1] See the `printedPage` respectively `xfaPrintedPage` classes.
Rather than modifying the leading/trailing `margin` on the actual toolbar buttons, to achieve appropriate spacing at the left/right edges of the toolbar(s), it seems much more appropriate (and simpler) to just specify an explicit `padding` for the relevant toolbar containers.
Also, for toolbar buttons placed in `splitToolbarButton`-classes we can reduce some complexity around setting the `margin` (since it should always be zero now).
With these changes, we're thus able to get rid of a couple more `!important`-rules.
- Remove a redundant `margin-top` rule for the `.dropdownToolbarButton`. After the (somewhat) recent UI-refresh all buttons now use `margin: 2px 1px;`, which renders the override unnecessary (and getting rid of an `!important`-rule can't hurt).
- Combine two `.toggled::before` rules, since they're identical.
Note that both the `errorWrapper` HTML and JavaScript code is being ignored in the MOZCENTRAL build, i.e. the *built-in* Firefox PDF Viewer, however the CSS rules are still being included.
That seems totally unnecessary, and while we currently don't have full build-target support in the CSS pre-processor we can actually improve things quite easily anyway. By (ab)using the existing CSS pre-processor, which will remove any non-Firefox CSS rules for the MOZCENTRAL build, it's possible to easily stop bundling any CSS rules by using comments that include a `-webkit`-string.
*Please note:* To easily test that this doesn't break the `errorWrapper` in GENERIC builds, try running e.g. `PDFViewerApplication._otherError("test");` in the web-console.
The styling of the previous/next-buttons and the findInput, with the elements being "glued" together, was supposed to mimic the styling used in the Firefox *browser* findbar. However, after the most recent re-styling of the Firefox browser UI these elements are now visually separated.
Hence it makes sense, as far as I'm concerned, to try and follow this styling for the findbar used in the GENERIC viewer. One benefit of doing this is that we get more consistent styling, since the buttons now look/behave identically in both the main toolbar and the findbar. Additionally this also simplifies the CSS a bit, since a lot of the existing findbar-specific rules can be removed.
Given that no HTML element has used the `loadingBox`-id for many years, we obviously don't need to try and hide a non-existent element during printing.
Furthermore, we also shouldn't need to change the `overflow`-value for the `viewerContainer`-element during printing. Originally, many years ago now, we printed "directly" using the viewer and then this apparently made sense.
Given that none of these CSS rules are used at all, unless debugging is enabled, it seems completely unnecessary to load them *unconditionally* for all users.[1]
Note that if *both* the `textLayer` and `pdfBug` debugging hash-parameters are specified simultaneously, we'll now load the `PDFBug`-file *twice* (since the code is simpler that way). However, given first of all that none of this is enabled by default and secondly that using those parameters together isn't helpful[2], potentially loading that file twice is hopefully not an issue.
For the `gulp mozcentral` target, the size of the *built* `viewer.css` file is reduced `> 3%` with this patch.
---
[1] For the Firefox built-in PDF Viewer, in order to even be able to access the `PDFBug` functionality, you need to first of all set `pdfjs.pdfBugEnabled = true` manually in `about:config`. Secondly, you then also need to append the `pdfBug=...` hash-parameter to the URL when *initially* loading the document.
[2] Note how the `textLayer`-settings are already, since essentially forever, overriding the highlighting-features of the "FontInspector"-tab.
According to the CSS, there should be a visible "divider" after the "Page Width" zoom-option. However, this is being ignored in both Mozilla Firefox[1] and Google Chrome hence the rule is effectively useless now.
Furthermore, the "custom" zoom-option is already being `hidden` using the attribute (in the HTML code) and there should thus be no reason to duplicate this in the CSS as well.
---
[1] Support for *detailed* styling of `<select>`-elements was removed as part of the E10s project.
With just a couple of exceptions, for the `thumbnailView`, all of the sidebarViews share the same basic styling which thus allows for some simplification.
- For the findbar/secondaryToolbar case, the `min-width` rule doesn't really make sense since it's way too small to be useful. Furthermore, the findbar is already specifying its own `min-width` and the secondaryToolbar will (thanks to its buttons) receive a correct/useful width.
- The pageNumber-input already has an *explicit* `width` set, hence setting the `min-width` rule as well is completely unnecessary.
- The treeItem-links are supposed to *compute* their `min-width`, and the static value was only added as a fallback for older browsers without `calc()` support.
In a couple of spots, mostly related to the debugging tools, we're unnecessarily using a somewhat "complex" `background`-format only to specify a solid color. This can be simplified by using `background-color` instead, and the patch also removes a `color`-rule that's being ignored anyway.
After the changes in https://bugzilla.mozilla.org/show_bug.cgi?id=1761839, we no longer need this CSS work-around to prevent the entire `<dialog>` contents from becoming selected when the backdrop is clicked.
With the changes in PR 8993, a number of the `@media`-related CSS rules became unnecessary. However, it appears that some of these rule were *accidentally* left behind despite being unused now.
Note that previously, when opening the sidebar shifted the position of the main toolbar, we had to take both the sidebar opened *and* closed cases into account in these `@media` rules.
*This is yet another installment in a never-ending series of patches that attempt to simplify and improve old code.*
The `fileInput`-element is used to support the "Open file"-button in the `GENERIC` viewer, however this is very old code.
Rather than creating the element dynamically in JavaScript, we can simply define it conditionally in the HTML code thanks to the pre-processor. Furthermore, the `fileInput`-element currently has a number of unnecessary CSS rules, since the element is *purposely* never made visibly.
Note that with these changes, the `fileInput`-element will now *always* have `display: none;` set. This shouldn't matter, since we can still trigger the `click`-event of the element just fine (via JavaScript) and this patch has been successfully tested in both Mozilla Firefox and Google Chrome.
This replaces our *custom* overlays with standard `<dialog>` DOM elements, see https://developer.mozilla.org/en-US/docs/Web/HTML/Element/dialog, thus simplifying the related CSS, HTML, and JavaScript code.
With these changes, some of the functionality of the `OverlayManager` class is now handled natively (e.g. `Esc` to close the dialog). However, since we still need to be able to prevent dialogs from overlaying one another, it still makes sense to keep this functionality (as far as I'm concerned).
After the recent round of patches, I figured that we'd gone as far as possible in replacing `dir`-dependent CSS rules for the viewer.
However, it occurred that me that we could actually use a bit of CSS-trickery to get rid of the remaining ones. More specifically, this was done by defining a CSS variable whose value depends on the document direction and then using that variable together with `calc()` in the affected rules.
*Please note:* I suppose that this could perhaps be seen as a bit too "magical", hence I understand if this patch is ultimately rejected, however this is probably the only simple way to get rid of the remaining `dir`-dependent CSS rules.
*Please note:* This is another step in what will, time permitting, become a series of patches to simplify/modernize the viewer CSS.
Rather than having to manually specify ltr/rtl-specific float-values in the CSS, we can use logical `inline-start`/`inline-end` instead (and similar for some related left/right occurrences).
These logical properties depend on, among other things, the direction of the HTML document which we *always* specify in the viewer.
Given that most of these logical CSS properties are fairly new, and that cross-browser support is thus somewhat limited (see below), we rely on PostCSS plugins in order to support this in the GENERIC viewer.
- https://developer.mozilla.org/en-US/docs/Web/CSS/float#browser_compatibility
- https://developer.mozilla.org/en-US/docs/Web/CSS/inset-inline-end#browser_compatibility
