Commit Graph

19 Commits

Author SHA1 Message Date
Jonas Jenwald
4f82dd3932 Create a GrabToPan-instance lazily in the PDFCursorTools class
Unless the user enables the "HandTool" we don't actually need to create a `GrabToPan`-instance.
2023-06-28 12:43:36 +02:00
Jonas Jenwald
789e318cf7 A couple of small tweaks of the PDFCursorTools class
- Introduce a few private fields for internal state.

 - Inline a small method at its only call-site.
2023-06-28 12:43:34 +02:00
Jonas Jenwald
a98e80c4ff [GeckoView] Reduce the size of the *built* viewer
Given that the GV-viewer isn't using most of the UI-related components of the default-viewer, we can avoid including them in the *built* viewer to save space.[1]
The least "invasive" way of implementing this, at least that I could come up with, is to leverage import maps with suitable stubs for the GV-viewer.

The one slightly annoying thing is that we now have larger import maps across multiple html-files, and you'll need to remember to update all of them when making future changes.

---
[1] With this patch, the built `viewer.js` size is 391 kB and `viewer-geckoview.js` is 285 kB.
2023-02-05 14:12:32 +01:00
Jonas Jenwald
205ab95819 [Editing] Disable the HandTool during editing (bug 1792422)
This extends the approach used in PresentationMode to also cover the AnnotationEditor, and tries to handle the combination of both cases correctly.
In order to simplify the overall implementation we simply track the *first* seen "previous" cursorTool, and don't allow it to be reset as long as either PresentationMode or an AnnotationEditor is being used.
2022-09-29 10:44:06 +02:00
Jonas Jenwald
71f8680d66 Don't try to update the cursorTool when switching to PresentationMode failed
Currently this may print an Error message in the console, and while nothing breaks (since no actual Error is thrown) we should probably avoid this.
2022-09-12 13:36:37 +02:00
Tim van der Meij
a1d106dc5d
Use proper private methods in web/pdf_cursor_tools.js 2022-03-06 16:06:53 +01:00
Jonas Jenwald
4c107d8d7c Remove the useless PresentationModeState.CHANGING-case in PDFCursorTools (PR 12788 follow-up)
For reasons that I now can't for the life of me understand, I included handling of the `PresentationModeState.CHANGING`-case despite it not actually doing anything.
2021-02-14 10:39:49 +01:00
Jonas Jenwald
c185061757 Include the state in the "presentationmodechanged" event, and remove the separate active/switchInProgress properties
Given that we already have a `PresentationModeState`-enumeration, we should use that with the "presentationmodechanged" event rather than including separate properties. Note that this new behaviour, of including an enumeration-value in the event, is consistent with lots of other existing viewer-events.

To hopefully avoid issues in custom implementations of the default viewer, any attempt to access the removed properties will now throw.
2020-12-28 20:31:17 +01:00
Jonas Jenwald
4a1b056c82 Re-factor the EventBus to allow servicing of "external" event listeners *after* the viewer components have updated
Since the goal has always been, essentially since the `EventBus` abstraction was added, to remove all dispatching of DOM events[1] from the viewer components this patch tries to address one thing that came up when updating the examples:
The DOM events are always dispatched last, and it's thus guaranteed that all internal event listeners have been invoked first.
However, there's no such guarantees with the general `EventBus` functionality and the order in which event listeners are invoked is *not* specified. With the promotion of the `EventBus` in the examples, over DOM events, it seems like a good idea to at least *try* to keep this ordering invariant[2] intact.

Obviously this won't prevent anyone from manually calling the new *internal* viewer component methods on the `EventBus`, but hopefully that won't be too common since any existing third-party code would obviously use the `on`/`off` methods and that all of the examples shows the *correct* usage (which should be similarily documented on the "Third party viewer usage" Wiki-page).

---
[1] Looking at the various Firefox-tests, I'm not sure that it'll be possible to (easily) re-write all of them to not rely on DOM events (since getting access to `PDFViewerApplication` might be generally difficult/messy depending on scopes).
In any case, even if technically feasible, it would most likely add *a lot* of complication that may not be desireable in the various Firefox-tests. All-in-all, I'd be fine with keeping the DOM events only for the `MOZCENTRAL` target and gated on `Cu.isInAutomation` (or similar) rather than a preference.

[2] I wouldn't expect any *real* bugs in a custom implementation, simply based on event ordering, but it nonetheless seem like a good idea if any "external" events are still handled last.
2020-02-27 19:38:13 +01:00
Jonas Jenwald
36881e3770 Ensure that all import and require statements, in the entire code-base, have a .js file extension
In order to eventually get rid of SystemJS and start using native `import`s instead, we'll need to provide "complete" file identifiers since otherwise there'll be MIME type errors when attempting to use `import`.
2020-01-04 13:01:43 +01:00
Jonas Jenwald
5d14e68bec Enable the ESLint prefer-const rule in the web/ directory
Please find additional details about the ESLint rule at https://eslint.org/docs/rules/prefer-const

Note that this patch is generated automatically, by using the ESLint `--fix` argument, and will thus require some additional clean-up (which is done separately).
2019-12-27 01:03:58 +01:00
Jonas Jenwald
a63f7ad486 Fix the linting errors, from the Prettier auto-formatting, that ESLint --fix couldn't handle
This patch makes the follow changes:
 - Remove no longer necessary inline `// eslint-disable-...` comments.
 - Fix `// eslint-disable-...` comments that Prettier moved down, thus causing new linting errors.
 - Concatenate strings which now fit on just one line.
 - Fix comments that are now too long.
 - Finally, and most importantly, adjust comments that Prettier moved down, since the new positions often is confusing or outright wrong.
2019-12-26 12:35:12 +01:00
Jonas Jenwald
de36b2aaba Enable auto-formatting of the entire code-base using Prettier (issue 11444)
Note that Prettier, purposely, has only limited [configuration options](https://prettier.io/docs/en/options.html). The configuration file is based on [the one in `mozilla central`](https://searchfox.org/mozilla-central/source/.prettierrc) with just a few additions (to avoid future breakage if the defaults ever changes).

Prettier is being used for a couple of reasons:

 - To be consistent with `mozilla-central`, where Prettier is already in use across the tree.

 - To ensure a *consistent* coding style everywhere, which is automatically enforced during linting (since Prettier is used as an ESLint plugin). This thus ends "all" formatting disussions once and for all, removing the need for review comments on most stylistic matters.

Many ESLint options are now redundant, and I've tried my best to remove all the now unnecessary options (but I may have missed some).
Note also that since Prettier considers the `printWidth` option as a guide, rather than a hard rule, this patch resorts to a small hack in the ESLint config to ensure that *comments* won't become too long.

*Please note:* This patch is generated automatically, by appending the `--fix` argument to the ESLint call used in the `gulp lint` task. It will thus require some additional clean-up, which will be done in a *separate* commit.

(On a more personal note, I'll readily admit that some of the changes Prettier makes are *extremely* ugly. However, in the name of consistency we'll probably have to live with that.)
2019-12-26 12:34:24 +01:00
Tim van der Meij
8b4ae6f3eb
Consistently use @type for getter data types in JSDoc comments
Sometimes we also used `@return` or `@returns`, but `@type` is what
the JSDoc documentation recommends. This also improves the documentation
because before this commit the types were not shown and now they are.
2019-10-13 13:58:17 +02:00
Tim van der Meij
f4daafc077
Consistently use square brackets for optional parameters in JSDoc comments
Square brackets are recommended to indicate optional parameters. Using
them helps for automatically generating correct documentation.
2019-10-13 13:58:17 +02:00
Jonas Jenwald
46e1d5daa4 Simplify resetting of the SecondaryToolbar Scroll/Spread mode buttons, and add a missing comment in PDFCursorTools
The names 'resetscrollmode'/'resetspreadmode' were probably *not* great choices, given that the only thing being reset are toolbar buttons and not the actual Scroll/Spread modes. Furthermore, there's really no need for two separate events here.

The patch also adds a comment that ought to have been included in PR 9040, to prevent future refactoring/removing of what may appear to be an unnecessary `Promise.resolve` call.
2018-07-08 10:55:56 +02:00
Jonas Jenwald
c2f1523f06 Move the cursorToolOnLoad preference handling into AppOptions (PR 9493 follow-up)
Since no other viewer component is currently reading preferences itself, this patch thus unifies the behaviour across the viewer.
2018-03-19 23:24:56 +01:00
Jonas Jenwald
47448c27c3 Remove the enableHandToolOnLoad preference migration code in web/pdf_cursor_tools.js 2017-10-17 18:34:33 +02:00
Jonas Jenwald
36c2791296 Unify handling of various cursor tools, e.g. the current Hand Tool and a possible future Zoom Tool, in a new PDFCursorTools module
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.
2017-05-22 00:51:01 +02:00