ZenBrowserMirrors/pdf.js
1
0
Fork 0
mirror of https://github.com/zen-browser/pdf.js.git synced 2025-07-10 02:05:37 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

Re-factor the EventBus to allow servicing of "external" event listeners *after* the viewer components have updated

Browse source 
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.
This commit is contained in:
Jonas Jenwald 2020-02-26 23:33:27 +01:00
parent 9a437a158f
commit 4a1b056c82
16 changed files with 168 additions and 124 deletions
Download patch file Download diff file Expand all files Collapse all files

10
web/base_viewer.js
View file

@ -426,7 +426,7 @@ class BaseViewer {
// evicted from the buffer and destroyed even if we pause its rendering.
this._buffer.push(pageView);
};
this.eventBus.on("pagerender", this._onBeforeDraw);
this.eventBus._on("pagerender", this._onBeforeDraw);
this._onAfterDraw = evt => {
if (evt.cssTransform || onePageRenderedCapability.settled) {
@ -434,10 +434,10 @@ class BaseViewer {
}
onePageRenderedCapability.resolve();
this.eventBus.off("pagerendered", this._onAfterDraw);
this.eventBus._off("pagerendered", this._onAfterDraw);
this._onAfterDraw = null;
};
this.eventBus.on("pagerendered", this._onAfterDraw);
this.eventBus._on("pagerendered", this._onAfterDraw);
// Fetch a single page so we can get a viewport that will be the default
// viewport for all pages
@ -582,11 +582,11 @@ class BaseViewer {
this._spreadMode = SpreadMode.NONE;
if (this._onBeforeDraw) {
this.eventBus.off("pagerender", this._onBeforeDraw);
this.eventBus._off("pagerender", this._onBeforeDraw);
this._onBeforeDraw = null;
}
if (this._onAfterDraw) {
this.eventBus.off("pagerendered", this._onAfterDraw);
this.eventBus._off("pagerendered", this._onAfterDraw);
this._onAfterDraw = null;
}
// Remove the pages from the DOM...