Skip to content

Commit

Permalink
Merge pull request #16247 from Snuffleupagus/issue-7442
Browse files Browse the repository at this point in the history
[api-minor] Add support, in `PDFFindController`, for mixing phrase/word searches (issue 7442)
  • Loading branch information
timvandermeij authored Apr 16, 2023
2 parents 67ce8f1 + 0e19c3a commit f46ed43
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 48 deletions.
24 changes: 20 additions & 4 deletions test/unit/pdf_find_controller_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ function testSearch({
query: null,
caseSensitive: false,
entireWord: false,
phraseSearch: true,
findPrevious: false,
matchDiacritics: false,
},
Expand Down Expand Up @@ -182,7 +181,6 @@ function testEmptySearch({ eventBus, pdfFindController, state }) {
query: null,
caseSensitive: false,
entireWord: false,
phraseSearch: true,
findPrevious: false,
matchDiacritics: false,
},
Expand Down Expand Up @@ -321,8 +319,7 @@ describe("pdf_find_controller", function () {
eventBus,
pdfFindController,
state: {
query: "alternate solution",
phraseSearch: false,
query: ["alternate", "solution"],
},
matchesPerPage: [0, 0, 0, 0, 0, 1, 0, 0, 4, 0, 0, 0, 0, 0],
selectedMatch: {
Expand All @@ -332,6 +329,25 @@ describe("pdf_find_controller", function () {
});
});

it("performs a multiple term (phrase) search", async function () {
// Page 9 contains 'alternate solution' and pages 6 and 9 contain
// 'solution'. Both should be found for multiple term (phrase) search.
const { eventBus, pdfFindController } = await initPdfFindController();

await testSearch({
eventBus,
pdfFindController,
state: {
query: ["alternate solution", "solution"],
},
matchesPerPage: [0, 0, 0, 0, 0, 1, 0, 0, 3, 0, 0, 0, 0, 0],
selectedMatch: {
pageIndex: 5,
matchIndex: 0,
},
});
});

it("performs a normal search, where the text is normalized", async function () {
const { eventBus, pdfFindController } = await initPdfFindController(
"fraction-highlight.pdf"
Expand Down
1 change: 0 additions & 1 deletion web/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -2575,7 +2575,6 @@ function webViewerFindFromUrlHash(evt) {
source: evt.source,
type: "",
query: evt.query,
phraseSearch: evt.phraseSearch,
caseSensitive: false,
entireWord: false,
highlightAll: true,
Expand Down
1 change: 0 additions & 1 deletion web/firefoxcom.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,6 @@ class MozL10n {
source: window,
type: type.substring(findLen),
query: detail.query,
phraseSearch: true,
caseSensitive: !!detail.caseSensitive,
entireWord: !!detail.entireWord,
highlightAll: !!detail.highlightAll,
Expand Down
1 change: 0 additions & 1 deletion web/pdf_find_bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ class PDFFindBar {
source: this,
type,
query: this.findField.value,
phraseSearch: true,
caseSensitive: this.caseSensitive.checked,
entireWord: this.entireWord.checked,
highlightAll: this.highlightAll.checked,
Expand Down
110 changes: 71 additions & 39 deletions web/pdf_find_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,8 @@ function getOriginalIndex(diffs, pos, len) {
* Provides search functionality to find a given string in a PDF document.
*/
class PDFFindController {
#state = null;

#updateMatchesCountOnProgress = true;

#visitedPagesCount = 0;
Expand Down Expand Up @@ -421,7 +423,7 @@ class PDFFindController {
}

get state() {
return this._state;
return this.#state;
}

/**
Expand All @@ -445,13 +447,25 @@ class PDFFindController {
if (!state) {
return;
}
if (
(typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) &&
state.phraseSearch === false
) {
console.error(
"The `phraseSearch`-parameter was removed, please provide " +
"an Array of strings in the `query`-parameter instead."
);
if (typeof state.query === "string") {
state.query = state.query.match(/\S+/g);
}
}
const pdfDocument = this._pdfDocument;
const { type } = state;

if (this._state === null || this.#shouldDirtyMatch(state)) {
if (this.#state === null || this.#shouldDirtyMatch(state)) {
this._dirtyMatch = true;
}
this._state = state;
this.#state = state;
if (type !== "highlightallchange") {
this.#updateUIState(FindState.PENDING);
}
Expand Down Expand Up @@ -490,7 +504,7 @@ class PDFFindController {

// When the findbar was previously closed, and `highlightAll` is set,
// ensure that the matches on all active pages are highlighted again.
if (findbarClosed && this._state.highlightAll) {
if (findbarClosed && this.#state.highlightAll) {
this.#updateAllPages();
}
} else if (type === "highlightallchange") {
Expand Down Expand Up @@ -537,7 +551,7 @@ class PDFFindController {
this._pageMatches = [];
this._pageMatchesLength = [];
this.#visitedPagesCount = 0;
this._state = null;
this.#state = null;
// Currently selected match.
this._selected = {
pageIdx: -1,
Expand Down Expand Up @@ -565,22 +579,44 @@ class PDFFindController {
}

/**
* @type {string} The (current) normalized search query.
* @type {string|Array} The (current) normalized search query.
*/
get #query() {
if (this._state.query !== this._rawQuery) {
this._rawQuery = this._state.query;
[this._normalizedQuery] = normalize(this._state.query);
const { query } = this.#state;
if (typeof query === "string") {
if (query !== this._rawQuery) {
this._rawQuery = query;
[this._normalizedQuery] = normalize(query);
}
return this._normalizedQuery;
}
return this._normalizedQuery;
// We don't bother caching the normalized search query in the Array-case,
// since this code-path is *essentially* unused in the default viewer.
return (query || []).filter(q => !!q).map(q => normalize(q)[0]);
}

#shouldDirtyMatch(state) {
// When the search query changes, regardless of the actual search command
// used, always re-calculate matches to avoid errors (fixes bug 1030622).
if (state.query !== this._state.query) {
const newQuery = state.query,
prevQuery = this.#state.query;
const newType = typeof newQuery,
prevType = typeof prevQuery;

if (newType !== prevType) {
return true;
}
if (newType === "string") {
if (newQuery !== prevQuery) {
return true;
}
} else {
// Array
if (JSON.stringify(newQuery) !== JSON.stringify(prevQuery)) {
return true;
}
}

switch (state.type) {
case "again":
const pageNumber = this._selected.pageIdx + 1;
Expand Down Expand Up @@ -670,7 +706,7 @@ class PDFFindController {
}

#convertToRegExpString(query, hasDiacritics) {
const { matchDiacritics } = this._state;
const { matchDiacritics } = this.#state;
let isUnicode = false;
query = query.replaceAll(
SPECIAL_CHARS_REG_EXP,
Expand Down Expand Up @@ -741,36 +777,31 @@ class PDFFindController {

#calculateMatch(pageIndex) {
let query = this.#query;
if (!query) {
// Do nothing: the matches should be wiped out already.
return;
if (query.length === 0) {
return; // Do nothing: the matches should be wiped out already.
}

const { caseSensitive, entireWord, phraseSearch } = this._state;
const { caseSensitive, entireWord } = this.#state;
const pageContent = this._pageContents[pageIndex];
const hasDiacritics = this._hasDiacritics[pageIndex];

let isUnicode = false;
if (phraseSearch) {
if (typeof query === "string") {
[isUnicode, query] = this.#convertToRegExpString(query, hasDiacritics);
} else {
// Words are sorted in reverse order to be sure that "foobar" is matched
// before "foo" in case the query is "foobar foo".
const match = query.match(/\S+/g);
if (match) {
query = match
.sort()
.reverse()
.map(q => {
const [isUnicodePart, queryPart] = this.#convertToRegExpString(
q,
hasDiacritics
);
isUnicode ||= isUnicodePart;
return `(${queryPart})`;
})
.join("|");
}
query = query
.sort()
.reverse()
.map(q => {
const [isUnicodePart, queryPart] = this.#convertToRegExpString(
q,
hasDiacritics
);
isUnicode ||= isUnicodePart;
return `(${queryPart})`;
})
.join("|");
}

const flags = `g${isUnicode ? "u" : ""}${caseSensitive ? "" : "i"}`;
Expand All @@ -780,7 +811,7 @@ class PDFFindController {

// When `highlightAll` is set, ensure that the matches on previously
// rendered (and still active) pages are correctly highlighted.
if (this._state.highlightAll) {
if (this.#state.highlightAll) {
this.#updatePage(pageIndex);
}
if (this._resumePageIdx === pageIndex) {
Expand Down Expand Up @@ -876,7 +907,7 @@ class PDFFindController {
}

#nextMatch() {
const previous = this._state.findPrevious;
const previous = this.#state.findPrevious;
const currentPageIndex = this._linkService.page - 1;
const numPages = this._linkService.pagesCount;

Expand Down Expand Up @@ -911,7 +942,8 @@ class PDFFindController {
}

// If there's no query there's no point in searching.
if (!this.#query) {
const query = this.#query;
if (query.length === 0) {
this.#updateUIState(FindState.FOUND);
return;
}
Expand Down Expand Up @@ -948,7 +980,7 @@ class PDFFindController {
#matchesReady(matches) {
const offset = this._offset;
const numMatches = matches.length;
const previous = this._state.findPrevious;
const previous = this.#state.findPrevious;

if (numMatches) {
// There were matches for the page, so initialize `matchIdx`.
Expand Down Expand Up @@ -1021,7 +1053,7 @@ class PDFFindController {
}
}

this.#updateUIState(state, this._state.findPrevious);
this.#updateUIState(state, this.#state.findPrevious);
if (this._selected.pageIdx !== -1) {
// Ensure that the match will be scrolled into view.
this._scrollMatches = true;
Expand Down Expand Up @@ -1106,7 +1138,7 @@ class PDFFindController {
state,
previous,
matchesCount: this.#requestMatchesCount(),
rawQuery: this._state?.query ?? null,
rawQuery: this.#state?.query ?? null,
});
}
}
Expand Down
6 changes: 4 additions & 2 deletions web/pdf_link_service.js
Original file line number Diff line number Diff line change
Expand Up @@ -350,10 +350,12 @@ class PDFLinkService {
if (hash.includes("=")) {
const params = parseQueryString(hash);
if (params.has("search")) {
const query = params.get("search").replaceAll('"', ""),
phrase = params.get("phrase") === "true";

this.eventBus.dispatch("findfromurlhash", {
source: this,
query: params.get("search").replaceAll('"', ""),
phraseSearch: params.get("phrase") === "true",
query: phrase ? query : query.match(/\S+/g),
});
}
// borrowing syntax from "Parameters for Opening PDF Files"
Expand Down

0 comments on commit f46ed43

Please sign in to comment.