Given that these HTML elements are not being used at all in `MOZCENTRAL`-builds, note the preprocessor check in `PDFViewerApplication._otherError`, we obviously don't need the HTML code either.
This feature was Firefox-specific, and it's now been removed from the HTML specification and it's disabled by default starting with Firefox 85. Hence it seems completely unnecessary to keep this code in the default viewer.
Please refer to https://groups.google.com/g/mozilla.dev.platform/c/tc11BCenm2c and the resources that it links to.
aria-controls state
In testing, screen readers such as JAWS have trouble understanding the expanded state of the buttons that expand hidden menus due to lacking aria-expanded attribute. Also, given that the buttons do not contain the controlled/shown element, they should also define the aria-controls attribute with associated element id per https://www.w3.org/TR/wai-aria-1.1/#aria-expanded
This fixes adds these requirements for the sidebar, find, and secondary toolbar buttons.
This implementation is inspired by the behaviour in (recent versions of) Adobe Reader, since it leads to reasonably simple and straightforward code as far as I'm concerned.
*Specifically:* We'll only consider *one* destination per page when finding/highlighting the current outline item, which is similar to e.g. Adobe Reader, and we choose the *first* outline item at the *lowest* level of the outline tree.
Given that this functionality requires not only parsing of the `outline`, but looking up *all* of the destinations in the document, this feature can when initialized have a non-trivial performance overhead for larger PDF documents.
In an attempt to reduce the performance impact, the following steps are taken here:
- The "find current outline item"-functionality will only be enabled once *one* page has rendered and *all* the pages have been loaded[1], to prevent it interfering with data regular fetching/parsing early on during document loading and viewer initialization.
- With the exception of a couple of small and simple `eventBus`-listeners, in `PDFOutlineViewer`, this new functionality is initialized *lazily* the first time that the user clicks on the `currentOutlineItem`-button.
- The entire "find current outline item"-functionality is disabled when `disableAutoFetch = true` is set, since it can easily lead to the setting becoming essentially pointless[2] by triggering *a lot* of data fetching from a relatively minor viewer-feature.
- Fetch the destinations *individually*, since that's generally more efficient than using `PDFDocumentProxy.getDestinations` to fetch them all at once. Despite making the overall parsing code *more* asynchronous, and leading to a lot more main/worker-thread message passing, in practice this seems faster for larger documents.
Finally, we'll now always highlight an outline item that the user manually clicked on, since only highlighting when the new "find current outline item"-functionality is used seemed inconsistent.
---
[1] Keep in mind that the `outline` itself already isn't fetched/parsed until at least *one* page has been rendered in the viewer.
[2] And also quite slow, since it can take a fair amount of time to fetch all of the necessary `destinations` data when `disableAutoFetch = true` is set.
For years the loadingBar and sidebarContainer has had a slightly annoying and unfortunate dependency, since the loadingBar width follows the main toolbar width[1].
To prevent the loadingBar from obscuring part of the sidebarContainer, especially the buttons, the sidebarContainer is thus moved down when the loadingBar is visible. This has always annoyed me[2], since it means that the buttons in the sidebar may thus move vertically which seems bad from a UX perspective.
Now that CSS variables are available in all supported browsers[3] however, fixing the loadingBar/sidebarContainer overlap issues are finally easy. The solution is simply to let the sidebarContainer, when visible, control the loadingBar left position (right in RTL locales) in the same way that the viewerContainer is handled. Hence the sidebarContainer can now have a *consistent* vertical postition, without the loadingBar overlapping it.
---
[1] Obviously the right position (left in RTL locales) of the loadingBar is, potentially, reduced to account for a scrollbar.
[2] I've tried to fix this a few times, but it always seemed like more trouble than it's worth.
[3] https://developer.mozilla.org/en-US/docs/Web/CSS/Using_CSS_custom_properties#Browser_compatibility
*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.)
With these changes SystemJS is now only used, during development, on the worker-thread and in the unit/font-tests, since Firefox is currently missing support for worker modules; please see https://bugzilla.mozilla.org/show_bug.cgi?id=1247687
Hence all the JavaScript files in the `web/` and `src/display/` folders are now loaded *natively* by the browser (during development) using standard `import` statements/calls, thanks to a nice `import-maps` polyfill.
*Please note:* As soon as https://bugzilla.mozilla.org/show_bug.cgi?id=1247687 is fixed in Firefox, we should be able to remove all traces of SystemJS and thus finally be able to use every possible modern JavaScript feature.
What the user did:
Open the PDF Viewer in Chrome;
Mouse click into the “Page number” input field;
What they saw:
A pop-up list with seemingly random numbers
What you were expecting to see:
Nothing
What they see is the Chrome “Autofill” feature at work – that is suggesting values that you have previously entered into number fields in forms, as possible values you may want to enter into this field. The list has nothing to do with the PDF currently open but the user does not know this.
After PR 9566, which removed all of the old Firefox extension code, the `FIREFOX` build flag is no longer used for anything.
It thus seems to me that it should be removed, for a couple of reasons:
- It's simply dead code now, which only serves to add confusion when looking at the `PDFJSDev` calls.
- It used to be that `MOZCENTRAL` and `FIREFOX` was *almost* always used together. However, ever since PR 9566 there's obviously been no effort put into keeping the `FIREFOX` build flags up to date.
- In the event that a new, Webextension based, Firefox addon is created in the future you'd still need to audit all `MOZCENTRAL` (and possibly `CHROME`) build flags to see what'd make sense for the addon.
As outlined in https://bugzilla.mozilla.org/show_bug.cgi?id=1282759 the internal Firefox name for the feature is `entireWord`, hence that name is used here as well for consistency (with "Whole words" being limited to the UI).
Given existing limitations of the PDF.js search functionality, e.g. the existing problems of searching across "new lines", there's some edge-cases where "Whole words" searching will ignore (valid) results.
However, considering that this is a pre-existing issue related to the way that the find controller joins text-content together, that shouldn't have to block this new feature in my opionion.
*Please note:* In order to enable this feature in the `MOZCENTRAL` version, a small follow-up patch for [PdfjsChromeUtils.jsm](https://hg.mozilla.org/mozilla-central/file/tip/browser/extensions/pdfjs/content/PdfjsChromeUtils.jsm) will be required once this has landed in `mozilla-central`.
For the `PDFFindBar` implementation, similar to the native Firefox findbar, the matches count displayed is now limited to a (hopefully) reasonable value.
*Please note:* In order to enable this feature in the `MOZCENTRAL` version, a follow-up patch will be required once this has landed in `mozilla-central`.
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.
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.
This uses a whitelist, based on the locale, to determine where non-metric units should be used.
Note that the behaviour implemented here seem consistent with desktop PDF viewers (e.g. Adobe Reader), where the pageSizes are *always* displayed with locale dependent units rather than pageSize dependent ones (since the latter would probably be quite confusing).
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.
At this point, the default viewer is already not usable in older browsers in `gulp server` mode, since we only run the code through Babel as part of the build step.
Hence there shouldn't be much point in manually loading `compatibility.js` in `viewer.html` the way that we've been doing, especially considering that it's already being loaded by `src/shared/util.js`.
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.
The browsers have become smarter and made this hack no longer
functional. Since this is now enforced by practically all browsers,
there is nothing more we can do about this. It is up to the user to
serve the viewer over HTTPS or deal with the warning.
Note that this is in no way specific for PDF.js. Any site with password
inputs served over HTTP has this problem right now. This hack was
provided as a convenience for the users, but nothing more than that.
The point of this patch is to fix a couple of inconsistencies in `viewer.html`, compared to the locale files, such that the viewer would work correctly even without the `l10n/` files present.
*Note:* Since this isn't changing any of the locale files, we should *not* need to update any of the string names.
Looking through the history of the findbar code, it seems that the `findPrevious`/`findNext` buttons have never had a `title` set (note PR 2168, which was the initial findbar implementation).
Furthermore, the `placeholder` of the `findInput` didn't agree 100% with locale file either, so this is also adjusted.
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.
Given that the `customScaleOption` should already be hidden, provided that the browser supports that, this patch also prevents it from being accessible via the keyboard.
As far as my testing goes in various browsers, this doesn't seem to have any ill effects, and note that we're already explicitly ignoring the `custom` value in the `select` event handler.
Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1315608.
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.
The outline toggle button has a feature where it can be double-clicked
to expand/collapse all items shown therein. Although this is described
in the FAQ, can go potentially unnoticed. This, however, being a useful
feature, advertise on the tool tip itself.
l10n translation for en-US and IDs updated.
Signed-off-by: Jeenu Viswambharan <jeenuv@gmail.com>
When Firefox is run in e10s mode, which will soon be the default, the PDF.js zoom dropdown menu doesn't look right. This is apparently because the `<select>` DOM element is rendered in the parent, and that all the necessary style information isn't sent up from the child. See the discussion in https://bugzilla.mozilla.org/show_bug.cgi?id=910022.
Besides this causing the PDF.js UI to *look* worse in e10s, notably it also means that the `customScaleOption` isn't hidden like it ought to be.
To work-around that, this patch utilizes the `hidden` attribute, since https://bugzilla.mozilla.org/show_bug.cgi?id=1242450 at least made that work in e10s.
Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1194700.
The sidebar code has, except for minor fixes/additions (such as attachments), been largely untouch for years.
To avoid having a bunch of sidebar code sprinkled throughout viewer.js, this patch moves the sidebar code into a separate file (pdf_sidebar.js), similar to how most other functionality has been moved in the last few years.
Besides simply moving code around, this patch also has the added benefit that we now keep track of the sidebar state (not just opened/closed).
This now makes it possible to handle both `Preferences` *and* `ViewHistory` settings for the sidebar state in a cleaner way, preventing strange and confusing interactions between the two.
Changes `PDFOutlineView`/`PDFAttachmentView` to be initialized once, since we're always creating them, and refactor their `render` methods to instead pass in the `outline`/`attachments`.
For consistency with other "classes", the `PDFOutlineView`/`PDFAttachmentView` are renamed to `PDFOutlineViewer`/`PDFAttachmentViewer`.
Also, make sure that the outline/attachments are reset when the document is closed. Currently we keep the old ones around until the `getOutline`/`getAttachments` API calls are resolved for a new document.
Fixes 6898.
Note that this doesn't prevent the warning for PDF files that *do* ask for a password, e.g. http://async5.org/moz/passwordOU.pdf, but it's not clear to me if/how we could avoid that.
This patch makes the naming consistent with the `TextLayerBuilder`, and also the new `AnnotationLayer`, and should thus help reduce possible confusion when working with the code.
Please note that the files were renamed using `git mv`, in order to preserve blame.
If your Internet Explorer 11 default compatibility settings are set to "I.E 7 Compatibility", the PDF plugin will not load. This fix is the same one used by AngularJS to force the browser to use HTML5 mode.
The line that this patch removes has been commented out since 2012 (!), see commit c8a6a9ba00.
Also, since the patch is trivial, I'm fixing the formatting of a comment to stop [GitHub from marking it with a red color](https://github.com/mozilla/pdf.js/blob/master/web/viewer.html#L394) (and IE from complaining).
The current way that the tabindices are defined, has the unfortunate consequence that adding a single button to the UI might mean having to change almost all tabindices. (This was the case in e.g. PR 4329.)
This patch attempts to avoid that situation happening again, by re-defining the tabindices slightly.
"text/javascript" is not a correct MIME type (the correct one is
"application/javascript") but it's not even needed; all browsers default
to the correct type and treat it as executable JS when type is ommited.
Since not all browsers recognize the "application/javascript" MIME type
the only way to both stay compliant and to support all popular browsers
is to omit the type. It's also shorter this way.
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>
