Merge pull request #19576 from nicolo-ribaudo/fix-linkannotation

Fix autolinking with highlighted search results

src/core/catalog.js

@@ -14,15 +14,7 @@
  */
 
 import {
-  collectActions,
+  _isValidExplicitDest,
-  isNumberArray,
-  MissingDataException,
-  PDF_VERSION_REGEXP,
-  recoverJsURL,
-  toRomanNumerals,
-  XRefEntryException,
-} from "./core_utils.js";
-import {
   createValidAbsoluteUrl,
   DocumentActionEventType,
   FormatError,
@@ -34,6 +26,15 @@ import {
   stringToUTF8String,
   warn,
 } from "../shared/util.js";
+import {
+  collectActions,
+  isNumberArray,
+  MissingDataException,
+  PDF_VERSION_REGEXP,
+  recoverJsURL,
+  toRomanNumerals,
+  XRefEntryException,
+} from "./core_utils.js";
 import {
   Dict,
   isDict,
@@ -53,52 +54,13 @@ import { FileSpec } from "./file_spec.js";
 import { MetadataParser } from "./metadata_parser.js";
 import { StructTreeRoot } from "./struct_tree.js";
 
-function isValidExplicitDest(dest) {
+const isRef = v => v instanceof Ref;
-  if (!Array.isArray(dest) || dest.length < 2) {
+
-    return false;
+const isValidExplicitDest = _isValidExplicitDest.bind(
-  }
+  null,
-  const [page, zoom, ...args] = dest;
+  /* validRef = */ isRef,
-  if (!(page instanceof Ref) && !Number.isInteger(page)) {
+  /* validName = */ isName
-    return false;
+);
-  }
-  if (!(zoom instanceof Name)) {
-    return false;
-  }
-  const argsLen = args.length;
-  let allowNull = true;
-  switch (zoom.name) {
-    case "XYZ":
-      if (argsLen < 2 || argsLen > 3) {
-        return false;
-      }
-      break;
-    case "Fit":
-    case "FitB":
-      return argsLen === 0;
-    case "FitH":
-    case "FitBH":
-    case "FitV":
-    case "FitBV":
-      if (argsLen > 1) {
-        return false;
-      }
-      break;
-    case "FitR":
-      if (argsLen !== 4) {
-        return false;
-      }
-      allowNull = false;
-      break;
-    default:
-      return false;
-  }
-  for (const arg of args) {
-    if (!(typeof arg === "number" || (allowNull && arg === null))) {
-      return false;
-    }
-  }
-  return true;
-}
 
 function fetchDest(dest) {
   if (dest instanceof Dict) {

src/core/chunked_stream.js

@@ -414,9 +414,7 @@ class ChunkedStreamManager {
       }
     }
 
-    chunksToRequest.sort(function (a, b) {
+    chunksToRequest.sort((a, b) => a - b);
-      return a - b;
-    });
     return this._requestChunks(chunksToRequest);
   }
 

src/core/fonts.js

@@ -576,9 +576,7 @@ function getRanges(glyphs, toUnicodeExtraMap, numGlyphs) {
   if (codes.length === 0) {
     codes.push({ fontCharCode: 0, glyphId: 0 });
   }
-  codes.sort(function fontGetRangesSort(a, b) {
+  codes.sort((a, b) => a.fontCharCode - b.fontCharCode);
-    return a.fontCharCode - b.fontCharCode;
-  });
 
   // Split the sorted codes into ranges.
   const ranges = [];
@@ -1777,9 +1775,7 @@ class Font {
       }
 
       // removing duplicate entries
-      mappings.sort(function (a, b) {
+      mappings.sort((a, b) => a.charCode - b.charCode);
-        return a.charCode - b.charCode;
-      });
       const finalMappings = [],
         seenCharCodes = new Set();
       for (const map of mappings) {

src/core/jbig2.js

@@ -374,9 +374,7 @@ function decodeBitmap(
   // Sorting is non-standard, and it is not required. But sorting increases
   // the number of template bits that can be reused from the previous
   // contextLabel in the main loop.
-  template.sort(function (a, b) {
+  template.sort((a, b) => a.y - b.y || a.x - b.x);
-    return a.y - b.y || a.x - b.x;
-  });
 
   const templateLength = template.length;
   const templateX = new Int8Array(templateLength);

src/core/run_length_stream.js

@@ -48,11 +48,9 @@ class RunLengthStream extends DecodeStream {
       }
     } else {
       n = 257 - n;
-      const b = repeatHeader[1];
       buffer = this.ensureBuffer(bufferLength + n + 1);
-      for (let i = 0; i < n; i++) {
+      buffer.fill(repeatHeader[1], bufferLength, bufferLength + n);
-        buffer[bufferLength++] = b;
+      bufferLength += n;
-      }
     }
     this.bufferLength = bufferLength;
   }

src/core/xml_parser.js

@@ -321,11 +321,7 @@ class SimpleDOMNode {
     if (!this.childNodes) {
       return this.nodeValue || "";
     }
-    return this.childNodes
+    return this.childNodes.map(child => child.textContent).join("");
-      .map(function (child) {
-        return child.textContent;
-      })
-      .join("");
   }
 
   get children() {

src/display/api.js

@@ -18,6 +18,7 @@
  */
 
 import {
+  _isValidExplicitDest,
   AbortException,
   AnnotationMode,
   assert,
@@ -595,15 +596,20 @@ function getFactoryUrlProp(val) {
   throw new Error(`Invalid factory url: "${val}" must include trailing slash.`);
 }
 
-function isRefProxy(ref) {
+const isRefProxy = v =>
-  return (
+  typeof v === "object" &&
-    typeof ref === "object" &&
+  Number.isInteger(v?.num) &&
-    Number.isInteger(ref?.num) &&
+  v.num >= 0 &&
-    ref.num >= 0 &&
+  Number.isInteger(v?.gen) &&
-    Number.isInteger(ref?.gen) &&
+  v.gen >= 0;
-    ref.gen >= 0
+
-  );
+const isNameProxy = v => typeof v === "object" && typeof v?.name === "string";
-}
+
+const isValidExplicitDest = _isValidExplicitDest.bind(
+  null,
+  /* validRef = */ isRefProxy,
+  /* validName = */ isNameProxy
+);
 
 /**
  * @typedef {Object} OnProgressParameters
@@ -2336,6 +2342,7 @@ class PDFWorker {
 
   /**
    * @param {PDFWorkerParameters} params - The worker initialization parameters.
+   * @returns {PDFWorker}
    */
   static fromPort(params) {
     if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("MOZCENTRAL")) {
@@ -3484,6 +3491,7 @@ const build =
 export {
   build,
   getDocument,
+  isValidExplicitDest,
   LoopbackPort,
   PDFDataRangeTransport,
   PDFDocumentLoadingTask,

src/pdf.js

@@ -45,6 +45,7 @@ import {
 import {
   build,
   getDocument,
+  isValidExplicitDest,
   PDFDataRangeTransport,
   PDFWorker,
   version,
@@ -117,6 +118,7 @@ export {
   InvalidPDFException,
   isDataScheme,
   isPdfFile,
+  isValidExplicitDest,
   noContextMenu,
   normalizeUnicode,
   OPS,

src/shared/util.js

@@ -1080,6 +1080,54 @@ function getUuid() {
 
 const AnnotationPrefix = "pdfjs_internal_id_";
 
+function _isValidExplicitDest(validRef, validName, dest) {
+  if (!Array.isArray(dest) || dest.length < 2) {
+    return false;
+  }
+  const [page, zoom, ...args] = dest;
+  if (!validRef(page) && !Number.isInteger(page)) {
+    return false;
+  }
+  if (!validName(zoom)) {
+    return false;
+  }
+  const argsLen = args.length;
+  let allowNull = true;
+  switch (zoom.name) {
+    case "XYZ":
+      if (argsLen < 2 || argsLen > 3) {
+        return false;
+      }
+      break;
+    case "Fit":
+    case "FitB":
+      return argsLen === 0;
+    case "FitH":
+    case "FitBH":
+    case "FitV":
+    case "FitBV":
+      if (argsLen > 1) {
+        return false;
+      }
+      break;
+    case "FitR":
+      if (argsLen !== 4) {
+        return false;
+      }
+      allowNull = false;
+      break;
+    default:
+      return false;
+  }
+  for (const arg of args) {
+    if (typeof arg === "number" || (allowNull && arg === null)) {
+      continue;
+    }
+    return false;
+  }
+  return true;
+}
+
 // TODO: Remove this once `Uint8Array.prototype.toHex` is generally available.
 function toHexUtil(arr) {
   if (Uint8Array.prototype.toHex) {
@@ -1119,6 +1167,7 @@ if (
 }
 
 export {
+  _isValidExplicitDest,
   AbortException,
   AnnotationActionEventType,
   AnnotationBorderStyleType,

test/downloadutils.mjs

@@ -42,6 +42,7 @@ function downloadFile(file, url, redirects = 0) {
       .get(url, async function (response) {
         if ([301, 302, 307, 308].includes(response.statusCode)) {
           if (redirects > 10) {
+            response.resume();
             reject(new Error("Too many redirects"));
             return;
           }
@@ -50,12 +51,14 @@ function downloadFile(file, url, redirects = 0) {
             await downloadFile(file, redirectTo, ++redirects);
             resolve();
           } catch (ex) {
+            response.resume();
             reject(ex);
           }
           return;
         }
 
         if (response.statusCode !== 200) {
+          response.resume();
           reject(new Error(`HTTP ${response.statusCode}`));
           return;
         }
@@ -88,7 +91,7 @@ async function downloadManifestFiles(manifest) {
     } catch (ex) {
       console.error(`Error during downloading of ${url}:`, ex);
       fs.writeFileSync(file, ""); // making it empty file
-      fs.writeFileSync(`${file}.error`, ex);
+      fs.writeFileSync(`${file}.error`, ex.toString());
     }
   }
 }

test/font/jasmine-boot.js

@@ -49,10 +49,7 @@ async function initializePDFJS(callback) {
       "pdfjs-test/font/font_os2_spec.js",
       "pdfjs-test/font/font_post_spec.js",
       "pdfjs-test/font/font_fpgm_spec.js",
-    ].map(function (moduleName) {
+    ].map(moduleName => import(moduleName)) // eslint-disable-line no-unsanitized/method
-      // eslint-disable-next-line no-unsanitized/method
-      return import(moduleName);
-    })
   );
 
   callback();

test/integration/autolinker_spec.mjs

@@ -13,10 +13,46 @@
  * limitations under the License.
  */
 
-import { closePages, createPromise, loadAndWait } from "./test_utils.mjs";
+import {
+  awaitPromise,
+  closePages,
+  createPromise,
+  loadAndWait,
+} from "./test_utils.mjs";
 
-function waitForLinkAnnotations(page) {
+function waitForLinkAnnotations(page, pageNumber) {
+  return page.evaluateHandle(
+    number => [
+      new Promise(resolve => {
+        const { eventBus } = window.PDFViewerApplication;
+        eventBus.on("linkannotationsadded", function listener(e) {
+          if (number === undefined || e.pageNumber === number) {
+            resolve();
+            eventBus.off("linkannotationsadded", listener);
+          }
+        });
+      }),
+    ],
+    pageNumber
+  );
+}
+
+function recordInitialLinkAnnotationsEvent(eventBus) {
+  globalThis.initialLinkAnnotationsEventFired = false;
+  eventBus.on(
+    "linkannotationsadded",
+    () => {
+      globalThis.initialLinkAnnotationsEventFired = true;
+    },
+    { once: true }
+  );
+}
+function waitForInitialLinkAnnotations(page) {
   return createPromise(page, resolve => {
+    if (globalThis.initialLinkAnnotationsEventFired) {
+      resolve();
+      return;
+    }
     window.PDFViewerApplication.eventBus.on("linkannotationsadded", resolve, {
       once: true,
     });
@@ -178,4 +214,49 @@ describe("autolinker", function () {
       );
     });
   });
+
+  describe("when highlighting search results", function () {
+    let pages;
+
+    beforeAll(async () => {
+      pages = await loadAndWait(
+        "issue3115r.pdf",
+        ".annotationLayer",
+        null,
+        { eventBusSetup: recordInitialLinkAnnotationsEvent },
+        { enableAutoLinking: true }
+      );
+    });
+
+    afterAll(async () => {
+      await closePages(pages);
+    });
+
+    it("must find links that overlap with search results", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          await awaitPromise(await waitForInitialLinkAnnotations(page));
+
+          const linkAnnotationsPromise = await waitForLinkAnnotations(page, 36);
+
+          // Search for "rich.edu"
+          await page.click("#viewFindButton");
+          await page.waitForSelector("#viewFindButton", { hidden: false });
+          await page.type("#findInput", "rich.edu");
+          await page.waitForSelector(".textLayer .highlight");
+
+          await awaitPromise(linkAnnotationsPromise);
+
+          const urls = await page.$$eval(
+            ".page[data-page-number='36'] > .annotationLayer > .linkAnnotation > a",
+            annotations => annotations.map(a => a.href)
+          );
+
+          expect(urls)
+            .withContext(`In ${browserName}`)
+            .toContain(jasmine.stringContaining("rich.edu"));
+        })
+      );
+    });
+  });
 });

test/stats/statcmp.js

@@ -64,9 +64,7 @@ function flatten(stats) {
   });
   // Use only overall results if not grouped by 'stat'
   if (!options.groupBy.includes("stat")) {
-    rows = rows.filter(function (s) {
+    rows = rows.filter(s => s.stat === "Overall");
-      return s.stat === "Overall";
-    });
   }
   return rows;
 }
@@ -129,9 +127,7 @@ function stat(baseline, current) {
   }
   const rows = [];
   // collect rows and measure column widths
-  const width = labels.map(function (s) {
+  const width = labels.map(s => s.length);
-    return s.length;
-  });
   rows.push(labels);
   for (const key of keys) {
     const baselineMean = mean(baselineGroup[key]);
@@ -162,9 +158,7 @@ function stat(baseline, current) {
   }
 
   // add horizontal line
-  const hline = width.map(function (w) {
+  const hline = width.map(w => new Array(w + 1).join("-"));
-    return new Array(w + 1).join("-");
-  });
   rows.splice(1, 0, hline);
 
   // print output

test/test.mjs

@@ -1051,9 +1051,7 @@ async function closeSession(browser) {
       await session.browser.close();
     }
     session.closed = true;
-    const allClosed = sessions.every(function (s) {
+    const allClosed = sessions.every(s => s.closed);
-      return s.closed;
-    });
     if (allClosed) {
       if (tempDir) {
         fs.rmSync(tempDir, { recursive: true, force: true });

test/unit/api_spec.js

@@ -4869,9 +4869,9 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`)
       // Issue 6205 reported an issue with font rendering, so clear the loaded
       // fonts so that we can see whether loading PDFs in parallel does not
       // cause any issues with the rendered fonts.
-      const destroyPromises = loadingTasks.map(function (loadingTask) {
+      const destroyPromises = loadingTasks.map(loadingTask =>
-        return loadingTask.destroy();
+        loadingTask.destroy()
-      });
+      );
       await Promise.all(destroyPromises);
     });
 

test/unit/jasmine-boot.js

@@ -100,10 +100,7 @@ async function initializePDFJS(callback) {
       "pdfjs-test/unit/xfa_serialize_data_spec.js",
       "pdfjs-test/unit/xfa_tohtml_spec.js",
       "pdfjs-test/unit/xml_spec.js",
-    ].map(function (moduleName) {
+    ].map(moduleName => import(moduleName)) // eslint-disable-line no-unsanitized/method
-      // eslint-disable-next-line no-unsanitized/method
-      return import(moduleName);
-    })
   );
 
   if (isNodeJS) {

test/unit/pdf_spec.js

@@ -36,6 +36,7 @@ import {
 import {
   build,
   getDocument,
+  isValidExplicitDest,
   PDFDataRangeTransport,
   PDFWorker,
   version,
@@ -94,6 +95,7 @@ const expectedAPI = Object.freeze({
   InvalidPDFException,
   isDataScheme,
   isPdfFile,
+  isValidExplicitDest,
   noContextMenu,
   normalizeUnicode,
   OPS,

web/autolinker.js

@@ -69,13 +69,55 @@ function calculateLinkPosition(range, pdfPageView) {
   return { quadPoints, rect };
 }
 
+/**
+ * Given a DOM node `container` and an index into its text contents `offset`,
+ * returns a pair consisting of text node that the `offset` actually points
+ * to, together with the offset relative to that text node.
+ * When the offset points at the boundary between two node, the result will
+ * point to the first text node in depth-first traversal order.
+ *
+ * For example, given this DOM:
+ * <p>abc<span>def</span>ghi</p>
+ *
+ * textPosition(p, 0) -> [#text "abc", 0] (before `a`)
+ * textPosition(p, 2) -> [#text "abc", 2] (between `b` and `c`)
+ * textPosition(p, 3) -> [#text "abc", 3] (after `c`)
+ * textPosition(p, 5) -> [#text "def", 2] (between `e` and `f`)
+ * textPosition(p, 6) -> [#text "def", 3] (after `f`)
+ */
+function textPosition(container, offset) {
+  let currentContainer = container;
+  do {
+    if (currentContainer.nodeType === Node.TEXT_NODE) {
+      const currentLength = currentContainer.textContent.length;
+      if (offset <= currentLength) {
+        return [currentContainer, offset];
+      }
+      offset -= currentLength;
+    } else if (currentContainer.firstChild) {
+      currentContainer = currentContainer.firstChild;
+      continue;
+    }
+
+    while (!currentContainer.nextSibling && currentContainer !== container) {
+      currentContainer = currentContainer.parentNode;
+    }
+    if (currentContainer !== container) {
+      currentContainer = currentContainer.nextSibling;
+    }
+  } while (currentContainer !== container);
+  throw new Error("Offset is bigger than container's contents length.");
+}
+
 function createLinkAnnotation({ url, index, length }, pdfPageView, id) {
   const highlighter = pdfPageView._textHighlighter;
   const [{ begin, end }] = highlighter._convertMatches([index], [length]);
 
   const range = new Range();
-  range.setStart(highlighter.textDivs[begin.divIdx].firstChild, begin.offset);
+  range.setStart(
-  range.setEnd(highlighter.textDivs[end.divIdx].firstChild, end.offset);
+    ...textPosition(highlighter.textDivs[begin.divIdx], begin.offset)
+  );
+  range.setEnd(...textPosition(highlighter.textDivs[end.divIdx], end.offset));
 
   return {
     id: `inferred_link_${id}`,

web/debugger.mjs

@@ -398,9 +398,7 @@ class Stepper {
   }
 
   getNextBreakPoint() {
-    this.breakPoints.sort(function (a, b) {
+    this.breakPoints.sort((a, b) => a - b);
-      return a - b;
-    });
     for (const breakPoint of this.breakPoints) {
       if (breakPoint > this.currentIdx) {
         return breakPoint;
@@ -487,9 +485,7 @@ const Stats = (function Stats() {
       statsDiv.textContent = stat.toString();
       wrapper.append(title, statsDiv);
       stats.push({ pageNumber, div: wrapper });
-      stats.sort(function (a, b) {
+      stats.sort((a, b) => a.pageNumber - b.pageNumber);
-        return a.pageNumber - b.pageNumber;
-      });
       clear(this.panel);
       for (const entry of stats) {
         this.panel.append(entry.div);

web/genericcom.js

@@ -53,6 +53,12 @@ class ExternalServices extends BaseExternalServices {
 }
 
 class MLManager {
+  static {
+    if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) {
+      this.getFakeMLManager = options => new FakeMLManager(options);
+    }
+  }
+
   async isEnabledFor(_name) {
     return false;
   }
@@ -68,13 +74,11 @@ class MLManager {
   guess(_data) {}
 
   toggleService(_name, _enabled) {}
-
-  static getFakeMLManager(options) {
-    return new FakeMLManager(options);
-  }
 }
 
-class FakeMLManager {
+if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) {
+  // eslint-disable-next-line no-var
+  var FakeMLManager = class {
     eventBus = null;
 
     hasProgress = false;
@@ -145,6 +149,7 @@ class FakeMLManager {
     toggleService(_name, enabled) {
       this.enableGuessAltText = enabled;
     }
+  };
 }
 
 export { ExternalServices, initCom, MLManager, Preferences };

web/pdf_link_service.js

@@ -16,6 +16,7 @@
 /** @typedef {import("./event_utils").EventBus} EventBus */
 /** @typedef {import("./interfaces").IPDFLinkService} IPDFLinkService */
 
+import { isValidExplicitDest } from "pdfjs-lib";
 import { parseQueryString } from "./ui_utils.js";
 
 const DEFAULT_LINK_REL = "noopener noreferrer nofollow";
@@ -415,7 +416,7 @@ class PDFLinkService {
       }
     } catch {}
 
-    if (typeof dest === "string" || PDFLinkService.#isValidExplicitDest(dest)) {
+    if (typeof dest === "string" || isValidExplicitDest(dest)) {
       this.goToDestination(dest);
       return;
     }
@@ -486,60 +487,6 @@ class PDFLinkService {
       optionalContentConfig
     );
   }
-
-  static #isValidExplicitDest(dest) {
-    if (!Array.isArray(dest) || dest.length < 2) {
-      return false;
-    }
-    const [page, zoom, ...args] = dest;
-    if (
-      !(
-        typeof page === "object" &&
-        Number.isInteger(page?.num) &&
-        Number.isInteger(page?.gen)
-      ) &&
-      !Number.isInteger(page)
-    ) {
-      return false;
-    }
-    if (!(typeof zoom === "object" && typeof zoom?.name === "string")) {
-      return false;
-    }
-    const argsLen = args.length;
-    let allowNull = true;
-    switch (zoom.name) {
-      case "XYZ":
-        if (argsLen < 2 || argsLen > 3) {
-          return false;
-        }
-        break;
-      case "Fit":
-      case "FitB":
-        return argsLen === 0;
-      case "FitH":
-      case "FitBH":
-      case "FitV":
-      case "FitBV":
-        if (argsLen > 1) {
-          return false;
-        }
-        break;
-      case "FitR":
-        if (argsLen !== 4) {
-          return false;
-        }
-        allowNull = false;
-        break;
-      default:
-        return false;
-    }
-    for (const arg of args) {
-      if (!(typeof arg === "number" || (allowNull && arg === null))) {
-        return false;
-      }
-    }
-    return true;
-  }
 }
 
 /**

web/pdfjs.js

@@ -39,6 +39,7 @@ const {
   InvalidPDFException,
   isDataScheme,
   isPdfFile,
+  isValidExplicitDest,
   noContextMenu,
   normalizeUnicode,
   OPS,
@@ -90,6 +91,7 @@ export {
   InvalidPDFException,
   isDataScheme,
   isPdfFile,
+  isValidExplicitDest,
   noContextMenu,
   normalizeUnicode,
   OPS,