[api-minor] Deprecate getGlobalEventBus and update the "viewer components" examples accordingly
#11631
Conversation
Snuffleupagus
force-pushed
the
getGlobalEventBus-deprecate
branch
from
1359a56
to
5a8b747
Compare
Snuffleupagus
force-pushed
the
getGlobalEventBus-deprecate
branch
4 times, most recently
from
44a0d42
to
1961cf5
Compare
…onents" examples accordingly To avoid outright breaking third-party usages of the "viewer components" the `getGlobalEventBus` functionality is left intact, but a deprecation message is printed if the function is invoked. The various examples are updated to *explicitly* initialize an `EventBus` instance, and provide that when initializing the relevant viewer components.
Snuffleupagus
force-pushed
the
getGlobalEventBus-deprecate
branch
3 times, most recently
from
cb3796b
to
f8760ec
Compare
…ners *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.
Snuffleupagus
force-pushed
the
getGlobalEventBus-deprecate
branch
from
f8760ec
to
4a1b056
Compare
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/6c5a9d5961f055f/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/6c5a9d5961f055f/output.txt Total script time: 2.55 mins Published |
|
/botio unittest |
From: Bot.io (Linux m4)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/fbab87390a3131a/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/a509b807393f2e8/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/fbab87390a3131a/output.txt Total script time: 2.69 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/a509b807393f2e8/output.txt Total script time: 4.81 mins
|
|
Nice step towards less DOM event usage. Thanks! |
|
I have also updated https://github.com/mozilla/pdf.js/wiki/Third-party-viewer-usage now. |
Thanks; I've also got a suggested update to the pre-existing example in that Wiki-page in a commit message for a future patch series (which should obviously not land until after the upcoming release): master...Snuffleupagus:rm-getGlobalEventBus Edit: Also, since the "EventBus" section seems more tailored to the components (rather than the default viewer as the rest of the page seems mainly concerned with) perhaps reference https://github.com/mozilla/pdf.js/tree/master/examples/components prominently in that section to avoid misunderstandings. |
[api-minor] Deprecate
getGlobalEventBusand update the "viewer components" examples accordinglyTo avoid outright breaking third-party usages of the "viewer components" the
getGlobalEventBusfunctionality is left intact, but a deprecation message is printed if the function is invoked.The various examples are updated to explicitly initialize an
EventBusinstance, and provide that when initializing the relevant viewer components.Re-factor the
EventBusto allow servicing of "external" event listeners after the viewer components have updatedSince the goal has always been, essentially since the
EventBusabstraction 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
EventBusfunctionality and the order in which event listeners are invoked is not specified. With the promotion of theEventBusin 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 theon/offmethods 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
PDFViewerApplicationmight 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
MOZCENTRALtarget and gated onCu.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.