Requests/Improvements for web/pdf_find_bar.js

[Jshollow]

Hi, this is in reference to #10120

First and foremost, I want to thank you for all the effort you've put into making pdf.js a great solution. The pdf viewer, the pdf highlighting, and more perform very well and take only a few lines of code to integrate and implement - you've provided a great service. What I am here for is to see how I can help you provide even greater service

I am using pdf_find_bar.js for a third-party integration. In order to get it working, I had to create a modified version of your pdf_find_bar.js file

I would like to work with you to add a few changes/improvements to pdf_find_bar.js so that my team can work with pdf_find_bar.js out of the box. I appreciate your time, input, and suggestions

Attach (recommended) or Link to PDF file here: http://www.pdf995.com/samples/pdf.pdf

Configuration:

• Web browser and its version: Google Chrome (at least version 60)
• Operating system and its version: Windows 7
• PDF.js version: v2.0.943
• Is a browser extension: No

Steps to reproduce the problem:

1. Set up the linkService, findController, and viewer

var pdfLinkService = new pdfjsViewer.PDFLinkService();

// (Optionally) enable find controller.
var pdfFindController = new pdfjsViewer.PDFFindController({
  linkService: pdfLinkService,
  eventBus: pdfLinkService.eventBus
});

var pdfViewer = new pdfjsViewer.PDFViewer({
  container: container,
  linkService: pdfLinkService,
  findController: pdfFindController,
});

2. Create findBar options
   (notice that there is no toggleButton in the options below - this is intentional because we don't use the toggleButton)

const options = { bar: document.getElementById('findbar'), findField: document.getElementById('findInput'), highlightAllCheckbox: document.getElementById('findHighlightAll'), caseSensitiveCheckbox: document.getElementById('findMatchCase'), entireWordCheckbox: document.getElementById('findEntireWord'), findMsg: document.getElementById('findMsg'), findResultsCount: document.getElementById('findResultsCount'), findPreviousButton: document.getElementById('findPrevious'), findNextButton: document.getElementById('findNext'), findController: pdfFindController, eventBus: pdfLinkService.eventBus }
3. Reference web/pdf_find_bar.js and create new instance
var pdfFindBar = new PDFFindBar(options, pdfLinkService.l10n);
4. Load and set the document

var loadingTask = pdfjsLib.getDocument({
  url: 'http://www.pdf995.com/samples/pdf.pdf',
  cMapUrl: .'./../node_modules/pdfjs-dist/cmaps/',
  cMapPacked: true,
});
loadingTask.promise.then(function(pdfDocument) {
  // Document loaded, specifying document for the viewer and
  // the (optional) linkService.
  pdfViewer.setDocument(pdfDocument);

  pdfLinkService.setDocument(pdfDocument, null);
});

What is the expected behavior? (add screenshot)

The expected behavior is that the document search should work as in the pdfjs online demo

What went wrong? (add screenshot)

No matches are found

Here are my suggested changes/improvements in the pdf_find_bar.js file:

1. Please add a conditional check for the toggleButton before adding the click listener (line 78):

    if (this.toggleButton) {
    this.toggleButton.addEventListener('click', function () {
      _this.toggle();
    });
    }

2. Please add event listeners for 'updatefindmatchescount' and 'updatefindcontrolstate' below the listener for 'resize' in pdf_find_bar.js (line 118):

    this.eventBus.on('updatefindmatchescount', function(obj) {
      _this.updateUIState(obj.state, obj.previous, obj.matchesCount);
    });

    this.eventBus.on('updatefindcontrolstate', function(obj) {
      _this.updateUIState(obj.state, obj.previous, obj.matchesCount);
    });

3. The dispatchEvent() function is not calling the executeCommand() function of the passed-in findController. To get this working I've done the following:

A. (line 58) set the script findController to the passed-in findController (options.findController):

(line 33) var _pdf_find_controller = null;
(line 58)_pdf_find_controller = pdfFindController;

B. (line 134) update the dispatchEvent() function to call the findController's executeCommand() function:

      _pdf_find_controller.executeCommand('find' + type, 
        { 
          query: this.findField.value,
          phraseSearch: true,
          caseSensitive: this.caseSensitive.checked,
          entireWord: this.entireWord.checked,
          highlightAll: this.highlightAll.checked,
          findPrevious: findPrev,
        });

C. (line 35) add _findState object and update references to findState (I know this doesn't make sense, but the findController's findState object was missing):

const _FindState = {
  FOUND: 0,
  NOT_FOUND: 1,
  WRAPPED: 2,
  PENDING: 3,
};

D. (line 48) add MATCHES_COUNT_LIMIT (this was also missing):

const MATCHES_COUNT_LIMIT = 1000;

4. (line 295) Optionally add default export PDFFindBar for ES6 compatibility

5. I commented out the following lines:

(lines 24-31):

/*Object.defineProperty(exports, "__esModule", {
  value: true
});
exports.PDFFindBar = void 0;

var _ui_utils = require("./ui_utils");

var _pdf_find_controller = require("./pdf_find_controller");*/

(lines 55-56):

    //var eventBus = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : (0, _ui_utils.getGlobalEventBus)();
    //var l10n = arguments.length > 2 && arguments[2] !== undefined ? arguments[2] : _ui_utils.NullL10n;

I know I don't have all of the answers and I may have done some things incorrectly. What I did above was done to get the pdfFindBar working for my team, based on the pdfjs documentation available and from looking through the pdfjs source code of working examples. The solution works. Now, I'd like to work with you to make this a better solution for all of us

Have a great day,

Jabari

[Snuffleupagus]

I would like to work with you to add a few changes/improvements to pdf_find_bar.js so that my team can work with pdf_find_bar.js out of the box. I appreciate your time, input, and suggestions

In general, please keep the following in mind (quoting from https://mozilla.github.io/pdf.js/getting_started/):

The viewer is built on the display layer and is the UI for PDF viewer in Firefox and the other browser extensions within the project. It can be a good starting point for building your own viewer. However, we do ask if you plan to embed the viewer in your own site, that it not just be an unmodified version. Please re-skin it or build upon it.

---

Please add a conditional check for the toggleButton before adding the click listener (line 78):

Something along these lines were rejected previously, please refer to #5803 (comment) in particular.

Please add event listeners for 'updatefindmatchescount' and 'updatefindcontrolstate' below the listener for 'resize' in pdf_find_bar.js (line 118)

That would cause issues with the Firefox version of the default viewer (which is the primary reason for this library existing in the first place), please see

pdf.js/web/app.js

Lines 2000 to 2018 in e1b01a6

	function webViewerUpdateFindMatchesCount({ matchesCount, }) {
	if (PDFViewerApplication.supportsIntegratedFind) {
	PDFViewerApplication.externalServices.updateFindMatchesCount(matchesCount);
	} else {
	PDFViewerApplication.findBar.updateResultsCount(matchesCount);
	}
	}
	function webViewerUpdateFindControlState({ state, previous, matchesCount, }) {
	if (PDFViewerApplication.supportsIntegratedFind) {
	PDFViewerApplication.externalServices.updateFindControlState({
	result: state,
	findPrevious: previous,
	matchesCount,
	});
	} else {
	PDFViewerApplication.findBar.updateUIState(state, previous, matchesCount);
	}
	}

The dispatchEvent() function is not calling the executeCommand() function of the passed-in findController.

(line 58) set the script findController to the passed-in findController (options.findController):

Not having PDFFindBar depend on PDFFindController, but utilize the EventBus instead, was a very deliberate description made in PR #10099 since the previous situation caused significant problems with cross-dependencies between classes.

(line 35) add _findState object and update references to findState (I know this doesn't make sense, but the findController's findState object was missing):

(line 48) add MATCHES_COUNT_LIMIT (this was also missing):

It's not clear what either of this actually means.

(line 295) Optionally add default export PDFFindBar for ES6 compatibility

This would be a more general issue, not really relevant/fixable in individual files, which is tracked in #10317.

[timvandermeij]

Closing as answered since I also don't see anything that we can improve here that should be part of the library.

[Jshollow]

Thank you for your responses and for your help. Just to understand, can PDFFindbar, PDFFindController, PDFViewer, and PDFLinkService all be referenced from PDFViewerApplication after importing pdf.js/web/app.js?

Also, does pdf.js v2.0.943 include app.js as well as the eventBus changes you added last Fall?

Have a good evening,

Jabari

[timvandermeij]

Yes, the version 2.0 release contains all changes after the find controller refactoring. You may also want to read through #10120 since that contains information about what we improved to decouple the the find controller and find bar so it's easier to integrate for third-party applications.

[Jshollow]

Great. I will explore the version 2.0 release and read through #10120 to see how we can use the find bar without modifying the find bar source code. Thank you again for your help. I'll let you know if I have questions