Merge pull request #20580 from calixteman/reorg_link_outline

Fix links and outline after reorganizing a pdf

Author: timvandermeij

test/integration/reorganize_pages_spec.mjs

@@ -19,9 +19,11 @@ import {
   closePages,
   createPromise,
   dragAndDrop,
+  getAnnotationSelector,
   getRect,
   getThumbnailSelector,
   loadAndWait,
+  scrollIntoView,
   waitForDOMMutation,
 } from "./test_utils.mjs";
 
@@ -415,4 +417,73 @@ describe("Reorganize Pages View", () => {
       );
     });
   });
+
+  describe("Links and outlines", () => {
+    let pages;
+
+    beforeEach(async () => {
+      pages = await loadAndWait(
+        "page_with_number_and_link.pdf",
+        "#viewsManagerToggleButton",
+        "page-fit",
+        null,
+        { enableSplitMerge: true }
+      );
+    });
+
+    afterEach(async () => {
+      await closePages(pages);
+    });
+
+    it("should check that link is updated after moving pages", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          await waitForThumbnailVisible(page, 1);
+          await movePages(page, [2], 10);
+          await scrollIntoView(page, getAnnotationSelector("107R"));
+          await page.click(getAnnotationSelector("107R"));
+          await page.waitForSelector(
+            ".page[data-page-number='10'] + .page[data-page-number='2']",
+            {
+              visible: true,
+            }
+          );
+
+          const currentPage = await page.$eval(
+            "#pageNumber",
+            el => el.valueAsNumber
+          );
+          expect(currentPage).withContext(`In ${browserName}`).toBe(10);
+        })
+      );
+    });
+
+    it("should check that outlines are updated after moving pages", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          await waitForThumbnailVisible(page, 1);
+          await movePages(page, [2, 4], 10);
+
+          await page.click("#viewsManagerSelectorButton");
+          await page.click("#outlinesViewMenu");
+          await page.waitForSelector("#outlinesView", { visible: true });
+
+          await page.click("#outlinesView .treeItem:nth-child(2)");
+          await page.waitForSelector(
+            ".page[data-page-number='10'] + .page[data-page-number='2']",
+            {
+              visible: true,
+            }
+          );
+
+          const currentPage = await page.$eval(
+            "#pageNumber",
+            el => el.valueAsNumber
+          );
+          // 9 because 2 and 4 were moved after page 10.
+          expect(currentPage).withContext(`In ${browserName}`).toBe(9);
+        })
+      );
+    });
+  });
 });

test/pdfs/.gitignore

@@ -869,3 +869,4 @@
 !bomb_giant.pdf
 !bug2009627.pdf
 !page_with_number.pdf
+!page_with_number_and_link.pdf

test/pdfs/page_with_number_and_link.pdf

@@ -16,8 +16,8 @@
 /** @typedef {import("./event_utils").EventBus} EventBus */
 /** @typedef {import("./interfaces").IPDFLinkService} IPDFLinkService */
 
+import { PagesMapper, parseQueryString } from "./ui_utils.js";
 import { isValidExplicitDest } from "pdfjs-lib";
-import { parseQueryString } from "./ui_utils.js";
 
 const DEFAULT_LINK_REL = "noopener noreferrer nofollow";
 
@@ -50,6 +50,8 @@ const LinkTarget = {
 class PDFLinkService {
   externalLinkEnabled = true;
 
+  #pagesMapper = PagesMapper.instance;
+
   /**
    * @param {PDFLinkServiceOptions} options
    */
@@ -138,7 +140,7 @@ class PDFLinkService {
     if (!this.pdfDocument) {
       return;
     }
-    let namedDest, explicitDest, pageNumber;
+    let namedDest, explicitDest, pageId;
     if (typeof dest === "string") {
       namedDest = dest;
       explicitDest = await this.pdfDocument.getDestination(dest);
@@ -156,13 +158,13 @@ class PDFLinkService {
     const [destRef] = explicitDest;
 
     if (destRef && typeof destRef === "object") {
-      pageNumber = this.pdfDocument.cachedPageNumber(destRef);
+      pageId = this.pdfDocument.cachedPageNumber(destRef);
 
-      if (!pageNumber) {
+      if (!pageId) {
         // Fetch the page reference if it's not yet available. This could
         // only occur during loading, before all pages have been resolved.
         try {
-          pageNumber = (await this.pdfDocument.getPageIndex(destRef)) + 1;
+          pageId = (await this.pdfDocument.getPageIndex(destRef)) + 1;
         } catch {
           console.error(
             `goToDestination: "${destRef}" is not a valid page reference, for dest="${dest}".`
@@ -171,20 +173,25 @@ class PDFLinkService {
         }
       }
     } else if (Number.isInteger(destRef)) {
-      pageNumber = destRef + 1;
+      pageId = destRef + 1;
     }
-    if (!pageNumber || pageNumber < 1 || pageNumber > this.pagesCount) {
+    if (!pageId || pageId < 1 || pageId > this.pagesCount) {
       console.error(
-        `goToDestination: "${pageNumber}" is not a valid page number, for dest="${dest}".`
+        `goToDestination: "${pageId}" is not a valid page number, for dest="${dest}".`
       );
       return;
     }
 
+    const pageNumber = this.#pagesMapper.getPageNumber(pageId);
+    if (pageNumber === null) {
+      return;
+    }
+
     if (this.pdfHistory) {
       // Update the browser history before scrolling the new destination into
       // view, to be able to accurately capture the current document position.
       this.pdfHistory.pushCurrentPosition();
-      this.pdfHistory.push({ namedDest, explicitDest, pageNumber });
+      this.pdfHistory.push({ namedDest, explicitDest, pageNumber: pageId });
     }
 
     this.pdfViewer.scrollPageIntoView({
@@ -197,7 +204,7 @@ class PDFLinkService {
     this.eventBus._on(
       "textlayerrendered",
       evt => {
-        if (evt.pageNumber === pageNumber) {
+        if (evt.pageNumber === pageId) {
           evt.source.textLayer.div.focus();
           ac.abort();
         }