feat(pdf): Refactor find controller to use event bus to update UI

.travis.yml

@@ -1,6 +1,10 @@
 language: node_js
 node_js:
     - '10'
+addons:
+    apt:
+        packages:
+            - libgconf-2-4
 cache:
     yarn: true
     directories:

src/lib/util.js

@@ -13,6 +13,18 @@ const HEADER_CLIENT_NAME = 'X-Box-Client-Name';
 const HEADER_CLIENT_VERSION = 'X-Box-Client-Version';
 const PROMISE_MAP = {};
 
+/**
+ * Clears the promise map of any active promises
+ *
+ * @private
+ * @return {void}
+ */
+export function clearPromises() {
+    Object.keys(PROMISE_MAP).forEach(promiseKey => {
+        delete PROMISE_MAP[promiseKey];
+    });
+}
+
 /**
  * Creates an empty iframe or uses an existing one
  * for the purposes of downloading or printing
@@ -339,7 +351,7 @@ export function loadScripts(urls, disableAMD = false) {
     }
 
     urls.forEach(url => {
-        if (!PROMISE_MAP[url]) {
+        if (!head.querySelector(`script[src="${url}"]`) && !PROMISE_MAP[url]) {
             const script = createScript(url);
             PROMISE_MAP[url] = new Promise((resolve, reject) => {
                 script.addEventListener('load', resolve);
@@ -357,16 +369,15 @@ export function loadScripts(urls, disableAMD = false) {
             if (disableAMD && amdPresent) {
                 define = defineRef;
             }
+
+            clearPromises();
         })
         .catch(() => {
             if (disableAMD && amdPresent) {
                 define = defineRef;
             }
-        })
-        .finally(() => {
-            Object.keys(PROMISE_MAP).forEach(promiseKey => {
-                delete PROMISE_MAP[promiseKey];
-            });
+
+            clearPromises();
         });
 }
 

src/lib/viewers/doc/DocBaseViewer.js

@@ -149,6 +149,7 @@ class DocBaseViewer extends BaseViewer {
      */
     destroy() {
         this.unbindDOMListeners();
+        this.unbindEventBusListeners();
 
         // Clean up print blob
         this.printBlob = null;
@@ -207,6 +208,31 @@ class DocBaseViewer extends BaseViewer {
         super.destroy();
     }
 
+    /**
+     * Unbinds all events on the internal PDFJS event bus
+     *
+     * @private
+     * @return {void}
+     */
+    unbindEventBusListeners() {
+        if (!this.pdfEventBus) {
+            return;
+        }
+
+        const eventBusListeners = this.pdfEventBus._listeners || {};
+
+        // EventBus does not have a destroy method, so iterate over and remove all subscribed event handlers
+        Object.keys(eventBusListeners).forEach(eventName => {
+            const eventListeners = eventBusListeners[eventName];
+
+            if (Array.isArray(eventListeners)) {
+                eventListeners.forEach(eventListener => {
+                    this.pdfEventBus.off(eventName, eventListener);
+                });
+            }
+        });
+    }
+
     /**
      * Converts a value and unit to page number
      *
@@ -583,18 +609,19 @@ class DocBaseViewer extends BaseViewer {
     initViewer(pdfUrl) {
         this.bindDOMListeners();
 
-        this.pdfJsEventBus = new this.pdfjsViewer.EventBus();
-        this.pdfJsEventBus.on('pagechanging', this.pagechangingHandler);
-        this.pdfJsEventBus.on('pagerendered', this.pagerenderedHandler);
-        this.pdfJsEventBus.on('pagesinit', this.pagesinitHandler);
+        this.pdfEventBus = new this.pdfjsViewer.EventBus();
+        this.pdfEventBus.on('pagechanging', this.pagechangingHandler);
+        this.pdfEventBus.on('pagerendered', this.pagerenderedHandler);
+        this.pdfEventBus.on('pagesinit', this.pagesinitHandler);
 
         this.pdfLinkService = new this.pdfjsViewer.PDFLinkService({
-            eventBus: this.pdfJsEventBus,
+            eventBus: this.pdfEventBus,
             externalLinkRel: 'noopener noreferrer nofollow', // Prevent referrer hijacking
             externalLinkTarget: this.pdfjsLib.LinkTarget.BLANK, // Open links in new tab
         });
 
         this.pdfFindController = new this.pdfjsViewer.PDFFindController({
+            eventBus: this.pdfEventBus,
             linkService: this.pdfLinkService,
         });
 
@@ -686,7 +713,7 @@ class DocBaseViewer extends BaseViewer {
         return new this.pdfjsViewer.PDFViewer({
             container: this.docEl,
             enhanceTextSelection: !this.isMobile, // Uses more memory, so disable on mobile
-            eventBus: this.pdfJsEventBus,
+            eventBus: this.pdfEventBus,
             findController: this.pdfFindController,
             linkService: this.pdfLinkService,
         });
@@ -710,7 +737,7 @@ class DocBaseViewer extends BaseViewer {
             return;
         }
 
-        this.findBar = new DocFindBar(this.findBarEl, this.pdfFindController, canDownload);
+        this.findBar = new DocFindBar(this.findBarEl, this.pdfFindController, this.pdfEventBus, canDownload);
         this.findBar.addListener(VIEWER_EVENT.metric, this.emitMetric);
     }
 

src/lib/viewers/doc/DocFindBar.js

@@ -19,18 +19,23 @@ class DocFindBar extends EventEmitter {
      *
      * @param {string|HTMLElement} findBar - Find bar selector or element
      * @param {Object} findController - Document find controller to use
+     * @param {Object} eventBus - Document event bus to use
      * @param {boolean} canDownload - Whether user can download document or not
      * @return {DocFindBar} DocFindBar instance
      */
-    constructor(findBar, findController, canDownload) {
+    constructor(findBar, findController, eventBus, canDownload) {
         super();
 
         this.opened = false;
         this.bar = findBar;
+        this.eventBus = eventBus;
         this.findController = findController;
-        this.currentMatch = 0;
         this.canDownload = canDownload;
 
+        if (this.eventBus === null) {
+            throw new Error('DocFindBar cannot be used without an EventBus instance.');
+        }
+
         if (this.findController === null) {
             throw new Error('DocFindBar cannot be used without a PDFFindController instance.');
         }
@@ -43,9 +48,9 @@ class DocFindBar extends EventEmitter {
         this.findPreviousHandler = this.findPreviousHandler.bind(this);
         this.close = this.close.bind(this);
 
-        // overriding some find controller methods to update match count
-        this.findController.updateUIState = this.updateUIState.bind(this);
-        this.findController.updateUIResultsCount = this.updateUIResultsCount.bind(this);
+        // attaching some listeners to update match count
+        this.eventBus.on('updatefindcontrolstate', this.updateUIState.bind(this));
+        this.eventBus.on('updatefindmatchescount', this.updateUIResultsCount.bind(this));
 
         // Default hides find bar on load
         this.bar.classList.add(CLASS_HIDDEN);
@@ -108,7 +113,6 @@ class DocFindBar extends EventEmitter {
      * @return {void}
      */
     destroy() {
-        this.currentMatch = 0;
         this.removeAllListeners();
         this.unbindDOMListeners();
 
@@ -136,10 +140,11 @@ class DocFindBar extends EventEmitter {
     /**
      * Update Find Bar UI to current match state
      *
-     * @param  {number} state FindState from PDFFindController
+     * @param {number} matchesCount - total number of matches from find controller
+     * @param {number} state - Find state from find controller
      * @return {void}
      */
-    updateUIState(state) {
+    updateUIState({ matchesCount, state }) {
         this.status = '';
 
         switch (state) {
@@ -160,21 +165,24 @@ class DocFindBar extends EventEmitter {
         }
 
         this.findFieldEl.setAttribute('data-status', this.status);
-        this.updateUIResultsCount();
+        this.updateUIResultsCount({ matchesCount });
     }
 
     /**
      * Update results count to current match count
      *
+     * @param {Object} matchesCount - matches from find controller
+     * @param {number} matchesCount.current - current match index
+     * @param {number} matchesCount.total - current total number of matches
      * @return {void}
      */
-    updateUIResultsCount() {
+    updateUIResultsCount({ matchesCount }) {
         if (!this.findResultsCountEl) {
             return; // no UI control is provided
         }
 
         // If there are no matches, hide the counter
-        if (!this.findController.matchCount) {
+        if (!matchesCount || !matchesCount.total) {
             this.findResultsCountEl.classList.add(CLASS_HIDDEN);
             return;
         }
@@ -184,7 +192,7 @@ class DocFindBar extends EventEmitter {
         this.findFieldEl.style.paddingRight = `${paddingRight}px`;
 
         // Create the match counter
-        this.findResultsCountEl.textContent = this.currentMatch + MATCH_SEPARATOR + this.findController.matchCount;
+        this.findResultsCountEl.textContent = `${matchesCount.current} ${MATCH_SEPARATOR} ${matchesCount.total}`;
 
         // Show the counter
         this.findResultsCountEl.classList.remove(CLASS_HIDDEN);
@@ -269,7 +277,6 @@ class DocFindBar extends EventEmitter {
      */
     findFieldHandler() {
         this.dispatchFindEvent('find');
-        this.currentMatch = 1;
     }
 
     /**
@@ -323,12 +330,6 @@ class DocFindBar extends EventEmitter {
                 this.findNextButtonEl.focus();
             } else {
                 this.dispatchFindEvent('findagain', false);
-                this.currentMatch = this.currentMatch + 1;
-
-                // Loops search to first match in document
-                if (this.currentMatch > this.findController.matchCount) {
-                    this.currentMatch = 1;
-                }
             }
 
             // Emit a metric that the user navigated forward in the find bar
@@ -351,12 +352,6 @@ class DocFindBar extends EventEmitter {
                 this.findPreviousButtonEl.focus();
             } else {
                 this.dispatchFindEvent('findagain', true);
-                this.currentMatch = this.currentMatch - 1;
-
-                // Loops search to last match in document
-                if (this.currentMatch <= 0) {
-                    this.currentMatch = this.findController.matchCount;
-                }
             }
 
             // Emit a metric that the user navigated back in the find bar
@@ -408,7 +403,6 @@ class DocFindBar extends EventEmitter {
         }
         this.opened = false;
         this.bar.classList.add(CLASS_HIDDEN);
-        this.findController.active = false;
     }
 }
 

src/lib/viewers/doc/SinglePageViewer.js

@@ -16,7 +16,7 @@ class SinglePageViewer extends DocumentViewer {
         return new this.pdfjsViewer.PDFSinglePageViewer({
             container: this.docEl,
             enhanceTextSelection: !this.isMobile, // Uses more memory, so disable on mobile
-            eventBus: this.pdfJsEventBus,
+            eventBus: this.pdfEventBus,
             findController: this.pdfFindController,
             linkService: this.pdfLinkService,
         });

src/lib/viewers/doc/__tests__/DocBaseViewer-test.js

@@ -255,11 +255,13 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
         describe('destroy()', () => {
             it('should unbind listeners and clear the print blob', () => {
                 const unbindDOMListenersStub = sandbox.stub(docBase, 'unbindDOMListeners');
+                const unbindEventBusListenersStub = sandbox.stub(docBase, 'unbindEventBusListeners');
                 docBase.printURL = 'someblob';
                 sandbox.stub(URL, 'revokeObjectURL');
 
                 docBase.destroy();
                 expect(unbindDOMListenersStub).to.be.called;
+                expect(unbindEventBusListenersStub).to.be.called;
                 expect(docBase.printBlob).to.equal(null);
                 expect(URL.revokeObjectURL).to.be.calledWith(docBase.printURL);
             });
@@ -647,9 +649,12 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
             });
         });
 
-        describe.skip('initFind()', () => {
-            // TODO: FIX TESTS
+        describe('initFind()', () => {
             beforeEach(() => {
+                docBase.pdfEventBus = {
+                    off: sandbox.stub(),
+                    on: sandbox.stub(),
+                };
                 docBase.pdfViewer = {
                     setFindController: sandbox.stub(),
                 };
@@ -661,11 +666,6 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
                 expect(docBase.docEl.parentNode).to.deep.equal(docBase.containerEl);
             });
 
-            it('should create and set a new findController', () => {
-                docBase.initFind();
-                expect(docBase.pdfViewer.setFindController).to.be.called;
-            });
-
             it('should not set find bar if viewer option disableFindBar is true', () => {
                 sandbox
                     .stub(docBase, 'getViewerOption')
@@ -1752,6 +1752,22 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
             });
         });
 
+        describe('unbindEventBusListeners', () => {
+            it('should remove all the event listeners on the internal PDFJS event bus', () => {
+                docBase.pdfEventBus = {
+                    _listeners: {
+                        event1: [() => {}],
+                        event2: [() => {}, () => {}],
+                    },
+                    off: sandbox.stub(),
+                };
+
+                docBase.unbindEventBusListeners();
+
+                expect(docBase.pdfEventBus.off).to.have.callCount(3);
+            });
+        });
+
         describe('pagesinitHandler()', () => {
             beforeEach(() => {
                 stubs.loadUI = sandbox.stub(docBase, 'loadUI');