*Another small piece of clean-up of code I've previously written; follow-up to PR 8775.*
Importing `createPromiseCapability`, and then using it in just *one* spot, seems unnecessary since the `waitOnEventOrTimeout` function may just as well return a regular `Promise` directly.
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.
Prior to this commit, if the vertical scroll bar is absent and the horizontal
scroll bar is present, a link to a particular point on the page which should
induce a horizontal scroll did not do so, because the absence of a vertical
scroll bar meant that the viewer was not recognized as the nearest scrolling
ancestor. This commit adds a check for horizontal scroll bars when searching
for the scrolling ancestor.
A couple of basic unit-tests are added, and a manual `isLandscape` check (in `web/base_viewer.js`) is also converted to use the helper function instead.
Rather than having two different (but connected) options for the textLayer, I think that it makes sense to try and unify this. For example: currently if `disableTextLayer === true`, then the value of `enhanceTextSelection` is simply ignored.
Since PDF.js version `2.0` already won't be backwards compatible in lots of ways, I don't think that we need to worry about migrating existing preferences here.
Unfortunately, as far as I can tell, we still need the ability to adjust certain viewer options depending on the browser environment in PDF.js version `2.0`. However, we should be able to separate this from the general compatibility code in the `src/shared/compatibility.js` file.
This rule is available from https://www.npmjs.com/package/eslint-plugin-mozilla, and is enforced in mozilla-central. Note that we have the necessary `Array`/`String` polyfills and that most cases have already been fixed, see PRs 9032 and 9434.
For sufficiently large page sizes, always limiting the minimum zoom level to 25% seem a bit too high. One example is pages, e.g. the first one, in:
Hence I think that it makes sense to lower `MIN_SCALE` slightly, since other PDF viewers (e.g. Adobe Reader) isn't limiting the minimum zoom level as aggressively. Obviously this will allow a greater number of pages to be visible at the same time in the viewer, but given that they will be small that shouldn't be an issue.
Note also that e.g. the `page-fit`/`page-width` zoom levels already allow `< MIN_SCALE` values, so I don't see why we shouldn't allow users the same functionality directly.
The only reason for adding this parameter in the first place, all the way back in PR 4074, was that the "maintain document position on zooming" feature was landed and backed out a couple of times before it finally stuck.
Hence it seemed, at the time, like a good idea to have a simple way to disable that behaviour. However, that was almost four years ago, and it's just not likely that we'd want/need to ever disable it now.
Furthermore I really cannot imagine why anyone would actually *want* to reset the position whenever zooming occurs, since it results in a quite annoying UX.
*So, to summarize:* Based on the above, I think that we should try to remove this parameter now. On the off chance that anyone complains, re-adding it shouldn't be difficult.
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.
This patch introduces an abstract `BaseViewer` class, that the existing `PDFViewer` then extends. *Please note:* This lays the necessary foundation for the next patch.
This changes both `PDFViewer` and `PDFThumbnailViewer` to return early in the `pagesRotation` setters if the rotation doesn't change.
It also fixes an existing issue, in `PDFViewer`, that would cause errors if the rotation changes *before* the scale has been set to a non-default value.
Finally, in preparation for subsequent patches, it also refactors the rotation code in `web/app.js` to update the thumbnails and trigger rendering with the new `rotationchanging` event.
These changes consists mainly of replacing `var` with `let`/`const`, adding a couple of default parameters to function signatures, and finally converting `EventBus`/`ProgressBar` to proper classes.
This patch contains the following improvements:
- Only fetch the various document properties *once* per PDF file opened, and cache the result (in a frozen object).
- Always update the *entire* dialog at once, to prevent inconsistent UI state (issue 8371).
- Ensure that the dialog, and all its internal properties, are reset when `PDFViewerApplication.close` is called.
- Inline, and re-factor, the `getProperties` method in `open`, since that's the only call-site.
- Always overwrite the fileSize with the value obtained from `pdfDocument.getDownloadInfo`, to ensure that it's correct.
- ES6-ify the code that's touched in this patch.
Fixes 8371.
Please see http://eslint.org/docs/rules/object-shorthand.
For the most part, these changes are of the search-and-replace kind, and the previously enabled `no-undef` rule should complement the tests in helping ensure that no stupid errors crept into to the patch.
With the exception of just one test-case, all the current `ui_utils` unit-tests can run successfully on Node.js (since most of them doesn't rely on the DOM).
To get this working, I had to first of all add a new `LIB` build flag such that `gulp lib` produces a `web/pdfjs.js` file that is able to load `pdf.js` successfully.
Second of all, since neither `document` nor `navigator` is available in Node.js, `web/ui_utils.js` was adjusted slightly to avoid errors.
The patch also changes the `defaultFilename` to use the ES6 default parameter notation, and fixes the formatting of the JSDoc comment.
Finally, since `getPDFFileNameFromURL` currently has no unit-tests, a few basic ones are added to avoid regressions.
Note that as discussed on IRC, this makes the viewer slightly slower to load *only* in `gulp server` mode, however the difference seem slight enough that I think it will be fine.
The download method (and the PDF document properties) detect the
file name using `getPDFFileNameFromURL`. The title ought to also
display the PDF filename if available.
The regular expression incorrectly marked a group as capturing.
For `http://example.com/#file.pdf`, the expected result is "file.pdf",
but instead "document.pdf" was returned.
Using `else` after `return` is not necessary, and can often lead to unnecessarily cluttered code. By using the `no-else-return` rule in ESLint we can avoid this pattern, see http://eslint.org/docs/rules/no-else-return.
