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.
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.
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.
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).
- 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.
