Given that the outlineView/attachmentsView/layersView all share a common base-class and CSS rules, see PRs 12169 and 12170, the names of the CSS variables in question feels slightly strange now.
This patch purposely starts small, by removing IE-specific code from various JS/CSS files in the `web/` folder.
There's obviously lots of potential for additional clean-up, especially the removal of no longer necessary polyfills in `src/shared/compatibility.js`, however that will require some care considering that certain polyfills may also be necessary for e.g. Node.js or the Chromium-extension as well.
Generally speaking, once we start removing polyfills it's probably a good idea to consult the compatibility information on https://developer.mozilla.org/ and also https://caniuse.com/ first. (Deciding on the lowest supported Chromium version, for the extension, would also seem like a good idea.)
This is *similar* to the existing linting for JavaScript files, but covers CSS files instead.
While there's a lot of rules that could potentially be used, the main advantage of using Stylelint is that it has Prettier integration which means that we can automatically enforce a *consistent* style for our CSS files as well.
As a proof of concept, this patch is purposely limited to:
- Adding a simple rule, here `block-no-empty` is chosen; see https://stylelint.io/user-guide/rules/block-no-empty
- Adding Prettier integration, to unify the style of our CSS files.
Please find additional information at https://stylelint.io/
*Besides, obviously, adding viewer support:* This patch attempts to improve the general API for Optional Content Groups slightly, by adding a couple of new methods for interacting with the (more complex) data structures of `OptionalContentConfig`-instances. (Thus allowing us to mark some of the data as "private", given that it probably shouldn't be manipulated directly.)
By utilizing not just the "raw" Optional Content Groups, but the data from the `/Order` array when available, we can thus display the Layers in a proper tree-structure with collapsible headings for PDF documents that utilizes that feature.
Note that it's possible to reset all Optional Content Groups to their default visibility state, simply by double-clicking on the Layers-button in the sidebar.
(Currently that's indicated in the Layers-button tooltip, which is obviously easy to overlook, however it's probably the best we can do for now without adding more buttons, or even a dropdown-toolbar, to the sidebar.)
Also, the current Layers-button icons are a little rough around the edges, quite literally, but given that the viewer will soon have its UI modernized anyway they hopefully suffice in the meantime.
To give users *full* control of the visibility of the various Optional Content Groups, even those which according to the `/Order` array should not (by default) be toggleable in the UI, this patch will place those under a *custom* heading which:
- Is collapsed by default, and placed at the bottom of the Layers-tree, to be a bit less obtrusive.
- Uses a slightly different formatting, compared to the "regular" headings.
- Is localizable.
Finally, note that the thumbnails are *purposely* always rendered with all Optional Content Groups at their default visibility state, since that seems the most useful and it's also consistent with other viewers.
To ensure that this works as intended, we'll thus disable the `PDFThumbnailView.setImage` functionality when the Optional Content Groups have been changed in the viewer. (This obviously means that we'll re-render thumbnails instead of using the rendered pages. However, this situation ought to be rare enough for this to not really be a problem.)
This patch:
- Removes the :hover effect from the `findMsg` element, since it's a simple span and clicking it *obviously* does nothing.
- Given the way that the checkboxes are visually hidden, with `opacity: 0;` and absolute positioning, they are unfortunately still focusable (fixed by adding `pointer-events: none;`). To reproduce this, in `master`: Place the mouse pointer over the upper left-hand corner of the "Highlight all"-option, and notice that the :hover effect vanishes and clicking toggles the "Match case"-option instead.
This special progressBar is only used in the (fortunately) rare case when a server doesn't provide a valid `Content-Length` header. Since this progressBar isn't normally seen, when testing the default viewer, it's certainly very easy to see why these CSS rules were missed during review.
Furthermore, this patch also makes a couple of *small* progressBar CSS tweaks not related to the colours.
With the changes in PR 11077, these panels are no longer aligned exactly with the *center* of the corresponding toolbar buttons. This is especially noticeable for the `findbar` at narrow viewer width.
Currently there's enough leading padding that the `numPages` span feels somewhat "disconnected" from the `pageNumber` input, which seems unfortunate when they contain related state.
With the changes in PR 11077, the zoom dropdown now looks "squashed" in locales with longer than average zoom-strings[1]. The reason is that the zoom-value and the dropdown-icon are too close together, which doesn't look good in affected locales.
To fix this, the following changes are made:
- Increase the calculated dropdown width, in `Toolbar._adjustScaleWidth`, to account for the much wider icon (7 px -> 16 px) and the increased padding.
- Move the dropdown-icon *slightly* outwards, and also *slightly* reduce the left (right in RTL locales) padding of the dropdown-contents.
- Finally, remove the right (left in RTL locales) padding to reduce the chance of the *default* browser dropdown-icon being visible.
---
[1] This affects e.g. the `de` and `nl` locales, but there's probably other examples as well.
These two classes are unsurprisingly quite similar, and with upcoming changes[1] the amount of (essentially) duplicated code will increase even further.
Notable changes:
- Collect shared functionality in the `BaseTreeViewer` class, reducing both current and future code-duplication.
- Reduce unnecessary duplication in the CSS rules, which will be particularly useful with upcoming changes.
- Tweak the attachmentsView to use links, rather than buttons, to simplify (primarily) the CSS rules.
---
[1] Once API support for "Optional Content" lands, I've got more-or-less finished patches to add viewer support as well.
*Please note:* Most of the necessary API work was done in PR 10033, and the only remaining thing to do here was to implement it in the viewer.
The new preference should thus allow e.g. enterprise users to disable copying in the viewer, for PDF documents whose permissions specify that.
In order to simplify things the "copy"-permission was implemented using CSS, as suggested in https://bugzilla.mozilla.org/show_bug.cgi?id=792816#c55, which should hopefully suffice.[1]
The advantage of this approach, as opposed to e.g. disabling the `textLayer` completely, is first of all that it ensures that searching still works correctly even in copy-protected documents. Secondly this also greatly simplifies the overall implementation, since it doesn't require a lot of code for something that's disabled by default.
---
[1] As the discussion in the bug shows, this kind of copy-protection is not very strong and is also generally easy to remove/circumvent in various ways. Hence a simple solution, targeting "regular"-users rather than "power"-users is hopefully deemed acceptable here.
This avoids unnecessary duplication of many images, thus reducing the size of PDF.js image resources slightly.
Note that since the images should only be flipped horizontally, this required specifying the horizontal/vertical scaling separately for the hiDPI-images.
While Firefox originally used transition effects for browser UI toolbar buttons, that was removed years ago in https://bugzilla.mozilla.org/show_bug.cgi?id=1393057
Since the PDF.js viewer toolbar transitions were likely based on the Firefox ones, it seems reasonable that these transition effects are removed from PDF.js as well. Besides removing a bunch of CSS, this also makes the toolbar feel ever so slightly more "snappy" without these delays on mouse interaction.
(In order to make it more feasible to modernize/improve the viewer UI, trying to clean-up/simplify existing rules iteratively seems like the most reasonble way to make any progress here w.r.t. being able to test/review things.)
This changes the dropdown icon from being set using the `background` CSS property, to being set with `::after` which is *similar* to all the other toolbar button icons (which use `::before`).
Also tweaks the dropdown `background-color` on `:hover` slightly, since the other changes made it too light.
This patch contains some *much* needed clean-up of, and improvements to, this old code thus addressing a number of issues:
- Set more reasonable *default* values for the widths, in `web/viewer.css`, since the current ones are actually too small even for the (default) `en-US` locale.
This obviously result in a slightly larger zoom dropdown width for many locales, but the more consistent UI does look good to me.
- Stop setting the `min-width`/`max-width` and just use `width` instead.
- Set an explicit `height` of the zoom dropdown, in an attempt to get Google Chrome to display it with the same height as the toolbar buttons.
- Remove additional `Element.setAttribute("style", ...)` usage.
- Actually check *all* of the predefined l10n strings, since the old implementation (implicitly) assumed that the currently selected one was the longest (note e.g. the `ja-JP` locale where one string is considerably longer than the rest).
- Stop invalidating the DOM multiple times when doing the measurements. This was achieved by using a temporary in-memory `canvas`, and we now only need to query the DOM once in order to get the current font properties of the zoom dropdown.
This should hopefully be useful in environments where restrictive CSPs are in effect.
In most cases the replacement is entirely straighforward, and there's only a couple of special cases:
- For the `src/display/font_loader.js` and `web/pdf_outline_viewer.js `cases, since the elements aren't appended to the document yet, it shouldn't matter if the style properties are set one-by-one rather than all at once.
- For the `web/debugger.js` case, there's really no need to set the `padding` inline at all and the definition was simply moved to `web/viewer.css` instead.
*Please note:* There's still *a single* case left, in `web/toolbar.js` for setting the width of the zoom dropdown, which is left intact for now.
The reasons are that this particular case shouldn't matter for users of the general PDF.js library, and that it'd make a lot more sense to just try and re-factor that very old code anyway (thus fixing the `setAttribute` usage in the process).
Having thought *briefly* about using `css-vars-ponyfill`, I'm no longer convinced that it'd be a good idea. The reason is that if we actually want to properly support CSS variables, then that functionality should be available in *all* of our CSS files.
Note in particular the `pdf_viewer.css` file that's built as part of the `COMPONENTS` target, in which case I really cannot see how a rewrite-at-the-client solution would ever be guaranteed to always work correctly and without accidentally touching other CSS in the surrounding application.
All-in-all, simply re-writing the CSS variables at build-time seems much easier and is thus the approach taken in this patch; courtesy of https://github.com/MadLittleMods/postcss-css-variables
By using its `preserve` option, the built files will thus include *both* a fallback and a modern `var(...)` format[1]. As a proof-of-concept this patch removes a couple of manually added fallback values, and converts an additional sidebar related property to use a CSS variable.
---
[1] Comparing the `master` branch with this patch, when using `gulp generic`, produces the following diff for the built `web/viewer.css` file:
```diff
@@ -408,6 +408,7 @@
:root {
--sidebar-width: 200px;
+ --sidebar-transition-duration: 200ms;
}
* {
@@ -550,27 +551,28 @@
position: absolute;
top: 32px;
bottom: 0;
- width: 200px; /* Here, and elsewhere below, keep the constant value for compatibility
- with older browsers that lack support for CSS variables. */
+ width: 200px;
width: var(--sidebar-width);
visibility: hidden;
z-index: 100;
border-top: 1px solid rgba(51, 51, 51, 1);
-webkit-transition-duration: 200ms;
transition-duration: 200ms;
+ -webkit-transition-duration: var(--sidebar-transition-duration);
+ transition-duration: var(--sidebar-transition-duration);
-webkit-transition-timing-function: ease;
transition-timing-function: ease;
}
html[dir='ltr'] #sidebarContainer {
-webkit-transition-property: left;
transition-property: left;
- left: -200px;
+ left: calc(-1 * 200px);
left: calc(-1 * var(--sidebar-width));
}
html[dir='rtl'] #sidebarContainer {
-webkit-transition-property: right;
transition-property: right;
- right: -200px;
+ right: calc(-1 * 200px);
right: calc(-1 * var(--sidebar-width));
}
@@ -640,6 +642,8 @@
#viewerContainer:not(.pdfPresentationMode) {
-webkit-transition-duration: 200ms;
transition-duration: 200ms;
+ -webkit-transition-duration: var(--sidebar-transition-duration);
+ transition-duration: var(--sidebar-transition-duration);
-webkit-transition-timing-function: ease;
transition-timing-function: ease;
}
```
This is not only useful to have one format for consistency, but also to
be able to quickly search for colors for e.g., finding duplicates or
when tweaking the CSS for custom deployments.
If the current viewer is a `PDFSinglePageViewer` instance the Scroll/Spread modes are no-ops, hence displaying buttons that do *nothing* when clicked will probably do very little besides confuse users.
*The danger of fixing one bug is that it can, sometimes too easily, cause another one in the process; sorry for not catching this when testing PR 9816 locally.*
When the *entire* viewer becomes narrow enough, as controlled by the `@media all and (max-width: 840px)` media query, the sidebar should (semi-transparently) overlay the `viewerContainer` instead of moving it horizontally. Unfortunately the changes made in PR 9816 caused the relevant CSS rules to be skipped, because of how the inheritance model works in CSS.
I'm well aware that `!important` is usually advised against, since it "breaks" the CSS inheritance model. However in this case it seemed reasonable to use it, to not only fix the bug at hand but to also prevent similar bugs from occurring in the future.
This is a regression from PR 8993; it causes the pages to be offset horizontally in Presentation Mode, if and only if the sidebar is currently open when the user triggers Presentation Mode.
Please note that while this doesn't seem to affect Firefox, both Chrome and IE are however affected.
Interestingly enough, despite the Chrome extension being affected as well, I cannot find any issue filed about this. (Either Presentation Mode isn't used much at all, or users don't open the sidebar before entering Presentation Mode.)
This builds on the scrolling mode work to add three buttons for joining
page spreads together: one for the default view, with no page spreads,
and two for spreads starting on odd-numbered or even-numbered pages.
By making use of modern CSS features, in this case [CSS variables](https://developer.mozilla.org/en-US/docs/Web/CSS/Using_CSS_variables), implementing sidebar resizing is actually quite simple. Not only will the amount of added code be fairly small, but it should also be easy to maintain since there's no need for complicated JavaScript hacks in order to update the CSS. Another benefit is that the JavaScript code doesn't need to make detailed assumptions about the exact structure of the HTML/CSS code.
Obviously this will not work in older browsers, such as IE, that lack support for CSS variables. In those cases sidebar resizing is simply disabled (via feature detection), and the resizing DOM element hidden, and the behaviour is thus *identical* to the current (fixed-width) sidebar.
However, considering the simplicity of the implementation, I really don't see why limiting this feature to "modern" browsers is a problem.
Finally, note that a few edge-cases meant that the patch is a bit larger than what the basic functionality would dictate. Among those is first of all proper RTL support, and secondly (automatic) resizing of the sidebar when the width of the *entire* viewer changes. Another, pre-existing, issue fixed here is the incomplete interface of `NullL10n`.
*Please note:* This patch has been successfully tested in both LTR and RTL viewer locales, in recent versions of Firefox and Chrome.
Fixes 2072.
If we want to (eventually) make it possible to resize the sidebar, then having its width indirectly affect the toolbar is going to wreck havoc on the media queries used to show/hide buttons in the main toolbar (since many of them depend on the toolbar state, and thus its width).
Updating all of the media queries dynamically with JavaScript seems like a non-starter, given that it'd cause *very* messy code. It thus seem to me that we'd need to fix the position of the sidebar, to have any hope of (in the short term) addressing issue 2072.
Hence, I'm suggesting that the we always layout the sidebar in a consistent vertical position, and only animate the `viewerContainer` rather than the entire `mainContainer`.
Fixes 4052.
Fixes bug 850591.
With the current way that the `HandTool` is implemented, if someone would try to also add a Zoom tool (as issue 1260 asks for) that probably wouldn't work very well given that you'd then have two cursor tools which may not play nice together.
Hence this patch, which attempts to refactor things so that it should be simpler to add e.g. a Zoom tool as well (given that that issue is marked as "good-beginner-bug", and I'm not sure if that really applies considering the current state of the code).
Note that I personally have no interest in implementing a Zoom tool (similar to Adobe Reader) since I wouldn't use it, but I figured that it can't hurt to make this code a bit more future proof.
I noticed that we have a `.toolbarButton.group` CSS class, which is currently applied to some buttons in the viewer. Since it attempts to adjust the `margin-right` property, I was initially a bit puzzled as to why there wasn't different rules for LTR/RTL locales.
However, checking the viewer with the DevTools inspector, in both LTR and RTL locales, I quickly found that the rule in question is *always* being overridden by other CSS rules.
It thus seem to me that while this rule was probably useful at some point, it has been dead for years and could now be removed.
The find functionality is currently not available for small devices
because the find dialog is not responsive. This patch fixes that.
To achieve this goal, the HTML is changed to always show the find
button. To prevent issues because of the addition of an extra button for
small views, the previous/next page buttons are hidden if the view
becomes small. These buttons are not useful anyway because on small
devices navigation is usually done via scrolling. The find functionality
is much more useful to have in this case. Moreover, we wrap the existing
elements into separate `div`s so that the browser can position the
elements itself when the view becomes smaller and logically connected
elements stay together when this happens.
In the CSS, extra rules for the find bar have been added to ensure that
the dialog's doorhanger is always below the find button. All findbar
`div`s are forced to be 32 pixels high to prevent the find message text
being aligned under the checkboxes. Finally, the find message is only
visible when there is actually text to display. This prevents wrapping
issues because, by default, the label has padding and margin even if
there is no text.
A longstanding issue with the viewer is that you cannot tell if a PDF document includes an outline and/or attachments without actually opening the sidebar.
This patch contains a suggested solution for that, by displaying an hide-on-interaction notification on the `sidebarToggle` button (and the relevant sidebar view buttons). Note that this was inspired by e.g. the update notification that is displayed on the menu button in Firefox.
For an initial implementation, I've tried to do this in such a way that the notification isn't too distracting. Without being an UX expert, I don't think that we'd want something too in-your-face, in order to keep the viewer toolbars reasonable clean. (We probably do *not* want e.g. an entire notification bar in these situations, since that would take up unnecessary screen space and require actions from the user to close.)
However it's certainly possible that the current notification might simply be *too* inconspicuous to be truly helpful to users, but we could probably iterate on that if the feature itself is deemed useful.
This patch resolves the responsiveness issues for the toolbar in the
viewer. Depending on the language (for example the Dutch language),
elements could overlap when the viewport size is reduced.
The main issue here is that the CSS rules are unnecessarily complex and
handle lots of different cases (LTR/RTL, displacements for specific
viewport widths, et cetera). By removing this complexity and letting the
browser handle the responsiveness, we not only get simpler CSS rules and
HTML mark-up, but the responsiveness issues are mostly fixed at the same
time. We no longer have to position the elements manually (by setting
their `left` attribute value) anymore.
The following reasoning was used for deciding to remove the "Page: " label, and replace it with a tooltip, from the main toolbar:
- We have no other visible labels in the *main* toolbar (e.g. the Zoom dropdown doesn't have a label, but only a tooltip).
- We already hide the "Page: " label when the viewer is narrow.
- The varying width of the "Page: " label in different locales is already causing issues for many languages, with overlap in the main toolbar as a result.
Trying to create responsive CSS styles that works well in all locales is already difficult, and if we add support for page labels that will only further compound the issues.
- Some PDF viewers (e.g. Adobe Reader, pdfium in Chrome) doesn't show labels in the UI by default.
This patch contains an alternative solution (see the referenced PRs in 4718), in order for the indeterminate loading bar to have a consistent speed regardless of width.
Fixes 4718.
Prior to PR 6242, the width of all outline items was set such that their right (or left, in RTL locales) edges lined up vertically. In my opinion that looked more consistent, therefore this patch adjusts the CSS to make sure that this will be the case again.
The patch also makes the `border-radius` values of outline items a bit more consistent.
It appears to me that the `expanded/collapsed` part of the names got switched around, since I'd expect the following convention:
- `v` == expanded
- `>` == collapsed
- This commit adds a '>' before every outline item that has subitems.
- Click on the '>' to collapse all subitems under that item (which turns
the '>' in a 'v').
- Shift + click expands/collapses all descendant items under that tree.
- Double-clicking on the "Show Document Outline" button in the toolbar
expands/collapses all outline items.
During A/B testing we found a significant preference (increased engagement
time, reduced downloads of original PDF for local viewing) for the version that
doesn't animate loading thumbnails.
Profiling shows that the transition effects hit at a pessimal time when the
compositor is busy rendering the main PDF page view, and rendering thumbnail
previews, causing the main scroll view to stutter. With the transition removed
scrolling is perceptibly smoother in Chrome and Firefox. We also hypothesize
that the transition effects added to a perception of slowness but presumably
the aforementioned stutter is the primary issue here.
Having recently spent some time staring at the PasswordPrompt, while fixing issue 6010, I felt that the current border style does not really give a good visual indication that the input field actually has focus.
The current appearance was first introduced in PR 3527; but I don't know if having a different border style in Overlay dialogs was intentional, or if it just "happened".
However, given the colour palette used in the viewer UI, I think that using the same border style for all .toolbarFields makes sense.
In Blink-based browsers, there is a mismatch between document size and
paper size. Even if exactly the same values and unit are used, it is
possible that the printed results takes more pages than expected.
To solve the issue, the page size is set via @page size, and the canvas
and ancestor nodes are assigned a width+height of 100% (=relative to the
page). This change resolves bugs such as blank pages and split pages.
Now that [bug 649849](https://bugzilla.mozilla.org/show_bug.cgi?id=649849) has been fixed, adding support for `-moz-appearance: none`, the arrow is now too close to the text in the zoom box. This is currently only an issue in Nightly, but assuming the patch doesn't get backed out, this will soon affect all versions of Firefox.
The only simple solution I could find seems to be removing `*-appearance: none` rules from the CSS. I haven't been able to find any easier solutions that still looks the same with/without bug 649849.
When a search term isn't found, the background color of the findInput is supposed to change (to red). This is currently not working as intended, because the CSS rule is not being applied correctly. (It seems that this broke in PR 2208.)
This patch also changes the background color to match the one used in the native Firefox findbar, since the old color seemed a bit too pink.
After PR #5282, the text highlighting in the Font Inspector is very hard to see against a white background. That seems to be an oversight from the mentioned PR, hence this patch fixes that.
As a consequence of merging #5221 it is more likely to have multiple
overlapping selection divs inside the text layer. Because each individual
element gets the selection style applied, the 30%opacity stacks, making a
60% bar visible where the overlap happens.
As proposed by @rocallahan, this can be fixed by making the selection
style solid and setting opacity for the overall layer.
I assume also that this should make the work for the renderer easier, but
was unable to bench it.
Currently in RTL locales, the loading indicator is placed such that it is in the way when entring a search term. Hence this patch moves it to the other side of the input field to fix this.
transformOrigin is set to 0% 0% in all cases. This adds extra memory
impact into the dom tree. It also involves the CustomStyles workaround
to determine the correct css rule for the browser.
By setting all vendor and standard variants in css, the rule is applied
without the dom memory overhead and without the minor computation
overhead to set the value.
Added an "InteractiveAnnotation" class to homogenize the annotations' structure (highlighting) and user interactions (for now, used for text and link annotations).
Text annotations:
The appearance (AP) has priority over the icon (Name).
The popup extends horizontally (up to a limit) as well as vertically.
Reduced the title's font size.
The annotation's color (C) is used to color the popup's background.
On top of the mouseover show/hide behavior, a click on the icon will lock the annotation open (for mobile purposes). It can be closed with another click on either the icon or the popup.
An annotation printing is conditioned by its "print" bit
Unsupported annotations are not displayed at all.
The logic for the hand tool is implemented in a separate project,
maintained at https://github.com/Rob--W/grab-to-pan.js
Integration notes
- Added toggle as an entry under the Secondary toolbar
- Added shortcut "h" to toggle hand tool (to-do: document this in wiki
after merge). This shortcut is also used in Adobe's Acrobat Reader.
To-do: localizations for:
hand_tool_enable.title=
hand_tool_enable_label=
hand_tool_disable.title=
hand_tool_disable_label=
To-do (wish): persistence of hand tool preference, preferably a global setting.
secondaryToolbarButton-handTool.png created by Stephen Horlander <shorlander@mozilla.com>
Before this commit there were two main issues:
- In small windows, the zoom controls visually floated above the page number
(e.g. 733px).
- In small windows, the (transparent) zoom container covered the go-to-page
input box, which prevented one from using the input field to quickly navigate
to a different page.
