Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[api-minor] Remove the getGlobalEventBus viewer functionality, and the eventBusDispatchToDOM option/preference (PR 11631 follow-up) #11655

Merged
merged 2 commits into from
Mar 30, 2020

Conversation

Snuffleupagus
Copy link
Collaborator

  • [api-minor] Remove the getGlobalEventBus viewer functionality (PR 11631 follow-up)

    The correct/intended way of working with the "viewer components" is by providing an EventBus instance upon initialization, and the getGlobalEventBus was only added for backwards compatibility.
    Note, for example, that using getGlobalEventBus doesn't really work at all well with a use-case where there's multiple PDFViewer instances on a one page, since it may then be difficult/impossible to tell which viewer a particular event originated from.

    All of the "viewer components" examples have been previously updated, such that there's no longer any code/examples which relies on the now removed getGlobalEventBus functionality.

  • [api-minor] Remove the eventBusDispatchToDOM option/preference, and thus the general ability to dispatch "viewer components" events to the DOM

    This functionality was only added to the default viewer for backwards compatibility and to support the various PDF viewer tests in mozilla-central, with the intention to eventually remove it completely.
    While the different mozilla-central tests cannot be easily converted from DOM events, it's however possible to limit that functionality to only MOZCENTRAL builds and when tests are running.

    Rather than depending of the re-dispatching of internal events to the DOM, the default viewer can instead be used in e.g. the following way:

    document.addEventListener("webviewerloaded", function() {
      PDFViewerApplication.initializedPromise.then(function() {
        // The viewer has now been initialized, and its properties can be accessed.
    
        PDFViewerApplication.eventBus.on("pagerendered", function(event) {
          console.log("Has rendered page number: " + event.pageNumber);
        });
      });
    });

@Snuffleupagus Snuffleupagus force-pushed the rm-getGlobalEventBus branch from 16597fe to 84b3903 Compare March 3, 2020 11:54
@pdfjsbot
Copy link

pdfjsbot commented Mar 3, 2020

From: Bot.io (Windows)


Received

Command cmd_unittest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.215.176.217:8877/37b2c40e3b80499/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Mar 3, 2020

From: Bot.io (Linux m4)


Received

Command cmd_unittest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.67.70.0:8877/883b2a08c119dd4/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Mar 3, 2020

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/883b2a08c119dd4/output.txt

Total script time: 2.65 mins

  • Unit Tests: Passed

@pdfjsbot
Copy link

pdfjsbot commented Mar 3, 2020

From: Bot.io (Windows)


Failed

Full output at http://54.215.176.217:8877/37b2c40e3b80499/output.txt

Total script time: 4.68 mins

  • Unit Tests: FAILED

@pdfjsbot
Copy link

pdfjsbot commented Mar 3, 2020

From: Bot.io (Windows)


Received

Command cmd_unittest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.215.176.217:8877/f14ebe61632868d/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Mar 3, 2020

From: Bot.io (Linux m4)


Received

Command cmd_unittest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.67.70.0:8877/9c391ac7aa98334/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Mar 3, 2020

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/9c391ac7aa98334/output.txt

Total script time: 2.70 mins

  • Unit Tests: Passed

@pdfjsbot
Copy link

pdfjsbot commented Mar 3, 2020

From: Bot.io (Windows)


Success

Full output at http://54.215.176.217:8877/f14ebe61632868d/output.txt

Total script time: 4.60 mins

  • Unit Tests: Passed

@Snuffleupagus Snuffleupagus force-pushed the rm-getGlobalEventBus branch from 84b3903 to ceda04e Compare March 6, 2020 23:32
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request Mar 27, 2020
…Viewer. r=bdahl

As part of the work to clean-up the event dispatching in the PDF Viewer, particularily re-dispatching of internal `EventBus` events to the DOM, this patch is necessary to unblock the upstream work that's currently pending in [PR 11655](mozilla/pdf.js#11655).

Finally, the patch also removes the `ChromeActions.supportsDocumentColors` method given that it's been unused since [bug 1611175](https://bugzilla.mozilla.org/show_bug.cgi?id=1611175) and [PR 11521](mozilla/pdf.js#11521).

Differential Revision: https://phabricator.services.mozilla.com/D68533
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Mar 28, 2020
…Viewer. r=bdahl

As part of the work to clean-up the event dispatching in the PDF Viewer, particularily re-dispatching of internal `EventBus` events to the DOM, this patch is necessary to unblock the upstream work that's currently pending in [PR 11655](mozilla/pdf.js#11655).

Finally, the patch also removes the `ChromeActions.supportsDocumentColors` method given that it's been unused since [bug 1611175](https://bugzilla.mozilla.org/show_bug.cgi?id=1611175) and [PR 11521](mozilla/pdf.js#11521).

Differential Revision: https://phabricator.services.mozilla.com/D68533

--HG--
extra : moz-landing-system : lando
@Snuffleupagus Snuffleupagus marked this pull request as ready for review March 28, 2020 09:56
@Snuffleupagus
Copy link
Collaborator Author

With the necessary PdfStreamConverter.jsm method landed in bug 1625430, this PR should be ready for review now.

@Snuffleupagus
Copy link
Collaborator Author

/botio unittest

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_unittest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.67.70.0:8877/58523c4b812b501/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_unittest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.215.176.217:8877/7e794cb40ea3d4a/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://54.215.176.217:8877/7e794cb40ea3d4a/output.txt

Total script time: 2.08 mins

  • Unit Tests: FAILED

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/58523c4b812b501/output.txt

Total script time: 2.73 mins

  • Unit Tests: Passed

…1631 follow-up)

The correct/intended way of working with the "viewer components" is by providing an `EventBus` instance upon initialization, and the `getGlobalEventBus` was only added for backwards compatibility.
Note, for example, that using `getGlobalEventBus` doesn't really work at all well with a use-case where there's *multiple* `PDFViewer` instances on a one page, since it may then be difficult/impossible to tell which viewer a particular event originated from.

All of the "viewer components" examples have been previously updated, such that there's no longer any code/examples which relies on the now removed `getGlobalEventBus` functionality.
… thus the general ability to dispatch "viewer components" events to the DOM

This functionality was only added to the default viewer for backwards compatibility and to support the various PDF viewer tests in mozilla-central, with the intention to eventually remove it completely.
While the different mozilla-central tests cannot be *easily* converted from DOM events, it's however possible to limit that functionality to only MOZCENTRAL builds *and* when tests are running.

Rather than depending of the re-dispatching of internal events to the DOM, the default viewer can instead be used in e.g. the following way:
```javascript
document.addEventListener("webviewerloaded", function() {
  PDFViewerApplication.initializedPromise.then(function() {
    // The viewer has now been initialized, and its properties can be accessed.

    PDFViewerApplication.eventBus.on("pagerendered", function(event) {
      console.log("Has rendered page number: " + event.pageNumber);
    });
  });
});
```
@timvandermeij
Copy link
Contributor

/botio-linux preview

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://54.67.70.0:8877/8c0ef502ac37788/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/8c0ef502ac37788/output.txt

Total script time: 2.40 mins

Published

@timvandermeij timvandermeij merged commit ce17276 into mozilla:master Mar 30, 2020
@timvandermeij
Copy link
Contributor

Nice work!

@Snuffleupagus Snuffleupagus deleted the rm-getGlobalEventBus branch March 31, 2020 07:49
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Mar 31, 2020
…Viewer. r=bdahl

As part of the work to clean-up the event dispatching in the PDF Viewer, particularily re-dispatching of internal `EventBus` events to the DOM, this patch is necessary to unblock the upstream work that's currently pending in [PR 11655](mozilla/pdf.js#11655).

Finally, the patch also removes the `ChromeActions.supportsDocumentColors` method given that it's been unused since [bug 1611175](https://bugzilla.mozilla.org/show_bug.cgi?id=1611175) and [PR 11521](mozilla/pdf.js#11521).

Differential Revision: https://phabricator.services.mozilla.com/D68533

UltraBlame original commit: 535dc6cf908145933612e554ca3680013a154757
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Mar 31, 2020
…Viewer. r=bdahl

As part of the work to clean-up the event dispatching in the PDF Viewer, particularily re-dispatching of internal `EventBus` events to the DOM, this patch is necessary to unblock the upstream work that's currently pending in [PR 11655](mozilla/pdf.js#11655).

Finally, the patch also removes the `ChromeActions.supportsDocumentColors` method given that it's been unused since [bug 1611175](https://bugzilla.mozilla.org/show_bug.cgi?id=1611175) and [PR 11521](mozilla/pdf.js#11521).

Differential Revision: https://phabricator.services.mozilla.com/D68533

UltraBlame original commit: 535dc6cf908145933612e554ca3680013a154757
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Mar 31, 2020
…Viewer. r=bdahl

As part of the work to clean-up the event dispatching in the PDF Viewer, particularily re-dispatching of internal `EventBus` events to the DOM, this patch is necessary to unblock the upstream work that's currently pending in [PR 11655](mozilla/pdf.js#11655).

Finally, the patch also removes the `ChromeActions.supportsDocumentColors` method given that it's been unused since [bug 1611175](https://bugzilla.mozilla.org/show_bug.cgi?id=1611175) and [PR 11521](mozilla/pdf.js#11521).

Differential Revision: https://phabricator.services.mozilla.com/D68533

UltraBlame original commit: 535dc6cf908145933612e554ca3680013a154757
robertknight added a commit to hypothesis/client that referenced this pull request Aug 21, 2020
Since [1] PDF.js does not dispatch events to the DOM any more. Therefore
the client needs to listen for the `documentloaded` event from
`PDFApplicationViewer.eventBus`. The `eventBus` property is only
available once the viewer is initialized, so the client needs to wait
for `initializedPromise` to resolve before checking it.

[1] mozilla/pdf.js#11655
robertknight added a commit to hypothesis/client that referenced this pull request Aug 21, 2020
Since [1] PDF.js does not dispatch events to the DOM any more. Therefore
the client needs to listen for the `documentloaded` event from
`PDFApplicationViewer.eventBus`. The `eventBus` property is only
available once the viewer is initialized, so the client needs to wait
for `initializedPromise` to resolve before checking it.

[1] mozilla/pdf.js#11655
robertknight added a commit to hypothesis/client that referenced this pull request Aug 21, 2020
Since [1] PDF.js does not dispatch events to the DOM any more. Therefore
the client needs to listen for the `documentloaded` event from
`PDFApplicationViewer.eventBus`. The `eventBus` property is only
available once the viewer is initialized, so the client needs to wait
for `initializedPromise` to resolve before checking it.

[1] mozilla/pdf.js#11655
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jul 11, 2023
…om various tests. r=pdfjs-reviewers,perftest-reviewers,marco,sparky

This preference was removed over three years ago, please see the upstream patch in mozilla/pdf.js#11655

Differential Revision: https://phabricator.services.mozilla.com/D183153
vinnydiehl pushed a commit to vinnydiehl/mozilla-unified that referenced this pull request Jul 12, 2023
…om various tests. r=pdfjs-reviewers,perftest-reviewers,marco,sparky

This preference was removed over three years ago, please see the upstream patch in mozilla/pdf.js#11655

Differential Revision: https://phabricator.services.mozilla.com/D183153
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Jul 16, 2023
…om various tests. r=pdfjs-reviewers,perftest-reviewers,marco,sparky

This preference was removed over three years ago, please see the upstream patch in mozilla/pdf.js#11655

Differential Revision: https://phabricator.services.mozilla.com/D183153

UltraBlame original commit: 96f24646390be39f604f0815e8fdc2560d4608be
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Jul 16, 2023
…om various tests. r=pdfjs-reviewers,perftest-reviewers,marco,sparky

This preference was removed over three years ago, please see the upstream patch in mozilla/pdf.js#11655

Differential Revision: https://phabricator.services.mozilla.com/D183153

UltraBlame original commit: 96f24646390be39f604f0815e8fdc2560d4608be
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Jul 16, 2023
…om various tests. r=pdfjs-reviewers,perftest-reviewers,marco,sparky

This preference was removed over three years ago, please see the upstream patch in mozilla/pdf.js#11655

Differential Revision: https://phabricator.services.mozilla.com/D183153

UltraBlame original commit: 96f24646390be39f604f0815e8fdc2560d4608be
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants