Merge pull request #14092 from Snuffleupagus/xfa-addLinkAttributes

timvandermeij · web-flow · commit 7beb67af7b2a · 2021-10-02T12:39:10.000+02:00
[api-minor] Ensure that various URL-related options are applied in the `xfaLayer` too
diff --git a/src/core/xfa/template.js b/src/core/xfa/template.js
@@ -1094,15 +1094,14 @@ class Button extends XFAObject {
       if (!href) {
         continue;
       }
-      const target = jsURL.newWindow ? "_blank" : undefined;
 
       // we've an url so generate a <a>
       htmlButton.children.push({
         name: "a",
         attributes: {
           id: "link" + this[$uid],
           href,
-          target,
+          newWindow: jsURL.newWindow,
           class: ["xfaLink"],
           style: {},
         },
diff --git a/src/display/annotation_layer.js b/src/display/annotation_layer.js
@@ -13,13 +13,6 @@
  * limitations under the License.
  */
 
-import {
-  addLinkAttributes,
-  DOMSVGFactory,
-  getFilenameFromUrl,
-  LinkTarget,
-  PDFDateString,
-} from "./display_utils.js";
 import {
   AnnotationBorderStyleType,
   AnnotationType,
@@ -30,6 +23,11 @@ import {
   Util,
   warn,
 } from "../shared/util.js";
+import {
+  DOMSVGFactory,
+  getFilenameFromUrl,
+  PDFDateString,
+} from "./display_utils.js";
 import { AnnotationStorage } from "./annotation_storage.js";
 import { ColorConverters } from "../shared/scripting_utils.js";
 
@@ -443,14 +441,15 @@ class LinkAnnotationElement extends AnnotationElement {
     const link = document.createElement("a");
 
     if (data.url) {
-      addLinkAttributes(link, {
-        url: data.url,
-        target: data.newWindow
-          ? LinkTarget.BLANK
-          : linkService.externalLinkTarget,
-        rel: linkService.externalLinkRel,
-        enabled: linkService.externalLinkEnabled,
-      });
+      if (
+        (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) &&
+        !linkService.addLinkAttributes
+      ) {
+        warn(
+          "LinkAnnotationElement.render - missing `addLinkAttributes`-method on the `linkService`-instance."
+        );
+      }
+      linkService.addLinkAttributes?.(link, data.url, data.newWindow);
     } else if (data.action) {
       this._bindNamedAction(link, data.action);
     } else if (data.dest) {
diff --git a/src/display/display_utils.js b/src/display/display_utils.js
@@ -338,7 +338,7 @@ const LinkTarget = {
 
 /**
  * Adds various attributes (href, title, target, rel) to hyperlinks.
- * @param {HTMLLinkElement} link - The link element.
+ * @param {HTMLAnchorElement} link - The link element.
  * @param {ExternalLinkParameters} params
  */
 function addLinkAttributes(link, { url, target, rel, enabled = true } = {}) {
@@ -633,7 +633,6 @@ function getXfaPageViewport(xfaPage, { scale = 1, rotation = 0 }) {
 
 export {
   addLinkAttributes,
-  DEFAULT_LINK_REL,
   deprecated,
   DOMCanvasFactory,
   DOMCMapReaderFactory,
diff --git a/src/display/xfa_layer.js b/src/display/xfa_layer.js
@@ -13,6 +13,7 @@
  * limitations under the License.
  */
 
+import { warn } from "../shared/util.js";
 import { XfaText } from "./xfa_text.js";
 
 class XfaLayer {
@@ -84,8 +85,10 @@ class XfaLayer {
     }
   }
 
-  static setAttributes(html, element, storage, intent) {
+  static setAttributes({ html, element, storage = null, intent, linkService }) {
     const { attributes } = element;
+    const isHTMLAnchorElement = html instanceof HTMLAnchorElement;
+
     if (attributes.type === "radio") {
       // Avoid to have a radio group when printing with the same as one
       // already displayed.
@@ -105,13 +108,32 @@ class XfaLayer {
         } else if (key === "class") {
           html.setAttribute(key, value.join(" "));
         } else {
+          if (isHTMLAnchorElement && (key === "href" || key === "newWindow")) {
+            continue; // Handled below.
+          }
           html.setAttribute(key, value);
         }
       } else {
         Object.assign(html.style, value);
       }
     }
 
+    if (isHTMLAnchorElement) {
+      if (
+        (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) &&
+        !linkService.addLinkAttributes
+      ) {
+        warn(
+          "XfaLayer.setAttribute - missing `addLinkAttributes`-method on the `linkService`-instance."
+        );
+      }
+      linkService.addLinkAttributes?.(
+        html,
+        attributes.href,
+        attributes.newWindow
+      );
+    }
+
     // Set the value after the others to be sure overwrite
     // any other values.
     if (storage && attributes.dataId) {
@@ -121,11 +143,17 @@ class XfaLayer {
 
   static render(parameters) {
     const storage = parameters.annotationStorage;
+    const linkService = parameters.linkService;
     const root = parameters.xfa;
     const intent = parameters.intent || "display";
     const rootHtml = document.createElement(root.name);
     if (root.attributes) {
-      this.setAttributes(rootHtml, root);
+      this.setAttributes({
+        html: rootHtml,
+        element: root,
+        intent,
+        linkService,
+      });
     }
     const stack = [[root, -1, rootHtml]];
 
@@ -169,7 +197,13 @@ class XfaLayer {
 
       html.appendChild(childHtml);
       if (child.attributes) {
-        this.setAttributes(childHtml, child, storage, intent);
+        this.setAttributes({
+          html: childHtml,
+          element: child,
+          storage,
+          intent,
+          linkService,
+        });
       }
 
       if (child.children && child.children.length > 0) {
diff --git a/test/driver.js b/test/driver.js
@@ -334,6 +334,7 @@ var rasterizeXfaLayer = (function rasterizeXfaLayerClosure() {
             div,
             viewport: viewport.clone({ dontFlip: true }),
             annotationStorage,
+            linkService: new SimpleLinkService(),
             intent: isPrint ? "print" : "display",
           });
 
diff --git a/test/unit/xfa_tohtml_spec.js b/test/unit/xfa_tohtml_spec.js
@@ -640,10 +640,10 @@ describe("XFAFactory", function () {
     const pages = factory.getPages();
     let a = searchHtmlNode(pages, "name", "a");
     expect(a.attributes.href).toEqual("https://github.com/mozilla/pdf.js");
-    expect(a.attributes.target).toEqual("_blank");
+    expect(a.attributes.newWindow).toEqual(true);
 
     a = searchHtmlNode(pages, "name", "a", false, [1]);
     expect(a.attributes.href).toEqual("https://github.com/allizom/pdf.js");
-    expect(a.attributes.target).toBe(undefined);
+    expect(a.attributes.newWindow).toEqual(false);
   });
 });
diff --git a/web/base_viewer.js b/web/base_viewer.js
@@ -1366,6 +1366,7 @@ class BaseViewer {
       pdfPage,
       annotationStorage:
         annotationStorage || this.pdfDocument?.annotationStorage,
+      linkService: this.linkService,
     });
   }
 
diff --git a/web/interfaces.js b/web/interfaces.js
@@ -63,6 +63,13 @@ class IPDFLinkService {
    */
   goToPage(val) {}
 
+  /**
+   * @param {HTMLAnchorElement} link
+   * @param {string} url
+   * @param {boolean} [newWindow]
+   */
+  addLinkAttributes(link, url, newWindow = false) {}
+
   /**
    * @param dest - The PDF destination object.
    * @returns {string} The hyperlink to the PDF object.
@@ -191,9 +198,16 @@ class IPDFXfaLayerFactory {
   /**
    * @param {HTMLDivElement} pageDiv
    * @param {PDFPage} pdfPage
+   * @param {AnnotationStorage} [annotationStorage]
+   * @param {Object} [xfaHtml]
    * @returns {XfaLayerBuilder}
    */
-  createXfaLayerBuilder(pageDiv, pdfPage) {}
+  createXfaLayerBuilder(
+    pageDiv,
+    pdfPage,
+    annotationStorage = null,
+    xfaHtml = null
+  ) {}
 }
 
 /**
diff --git a/web/pdf_link_service.js b/web/pdf_link_service.js
@@ -15,6 +15,7 @@
 
 /** @typedef {import("./interfaces").IPDFLinkService} IPDFLinkService */
 
+import { addLinkAttributes, LinkTarget } from "pdfjs-lib";
 import { parseQueryString } from "./ui_utils.js";
 
 /**
@@ -227,6 +228,21 @@ class PDFLinkService {
     this.pdfViewer.scrollPageIntoView({ pageNumber });
   }
 
+  /**
+   * Wrapper around the `addLinkAttributes`-function in the API.
+   * @param {HTMLAnchorElement} link
+   * @param {string} url
+   * @param {boolean} [newWindow]
+   */
+  addLinkAttributes(link, url, newWindow = false) {
+    addLinkAttributes(link, {
+      url,
+      target: newWindow ? LinkTarget.BLANK : this.externalLinkTarget,
+      rel: this.externalLinkRel,
+      enabled: this.externalLinkEnabled,
+    });
+  }
+
   /**
    * @param {string|Array} dest - The PDF destination object.
    * @returns {string} The hyperlink to the PDF object.
@@ -514,10 +530,7 @@ function isValidExplicitDestination(dest) {
  */
 class SimpleLinkService {
   constructor() {
-    this.externalLinkTarget = null;
-    this.externalLinkRel = null;
     this.externalLinkEnabled = true;
-    this._ignoreDestinationZoom = false;
   }
 
   /**
@@ -561,6 +574,15 @@ class SimpleLinkService {
    */
   goToPage(val) {}
 
+  /**
+   * @param {HTMLAnchorElement} link
+   * @param {string} url
+   * @param {boolean} [newWindow]
+   */
+  addLinkAttributes(link, url, newWindow = false) {
+    addLinkAttributes(link, { url, enabled: this.externalLinkEnabled });
+  }
+
   /**
    * @param dest - The PDF destination object.
    * @returns {string} The hyperlink to the PDF object.
diff --git a/web/pdf_outline_viewer.js b/web/pdf_outline_viewer.js
@@ -13,12 +13,8 @@
  * limitations under the License.
  */
 
-import {
-  addLinkAttributes,
-  createPromiseCapability,
-  LinkTarget,
-} from "pdfjs-lib";
 import { BaseTreeViewer } from "./base_tree_viewer.js";
+import { createPromiseCapability } from "pdfjs-lib";
 import { SidebarView } from "./ui_utils.js";
 
 /**
@@ -115,12 +111,7 @@ class PDFOutlineViewer extends BaseTreeViewer {
     const { linkService } = this;
 
     if (url) {
-      addLinkAttributes(element, {
-        url,
-        target: newWindow ? LinkTarget.BLANK : linkService.externalLinkTarget,
-        rel: linkService.externalLinkRel,
-        enabled: linkService.externalLinkEnabled,
-      });
+      linkService.addLinkAttributes(element, url, newWindow);
       return;
     }
 
diff --git a/web/secondary_toolbar.js b/web/secondary_toolbar.js
@@ -31,8 +31,8 @@ import { PDFSinglePageViewer } from "./pdf_single_page_viewer.js";
  * @property {HTMLButtonElement} printButton - Button to print the document.
  * @property {HTMLButtonElement} downloadButton - Button to download the
  *   document.
- * @property {HTMLLinkElement} viewBookmarkButton - Button to obtain a bookmark
- *   link to the current location in the document.
+ * @property {HTMLAnchorElement} viewBookmarkButton - Button to obtain a
+ *   bookmark link to the current location in the document.
  * @property {HTMLButtonElement} firstPageButton - Button to go to the first
  *   page in the document.
  * @property {HTMLButtonElement} lastPageButton - Button to go to the last page
diff --git a/web/toolbar.js b/web/toolbar.js
@@ -43,8 +43,8 @@ const PAGE_NUMBER_LOADING_INDICATOR = "visiblePageIsLoading";
  * @property {HTMLButtonElement} presentationModeButton - Button to switch to
  *   presentation mode.
  * @property {HTMLButtonElement} download - Button to download the document.
- * @property {HTMLAElement} viewBookmark - Element to link current url of
- *   the page view.
+ * @property {HTMLAnchorElement} viewBookmark - Button to obtain a bookmark link
+ *   to the current location in the document.
  */
 
 class Toolbar {
diff --git a/web/xfa_layer_builder.js b/web/xfa_layer_builder.js
@@ -15,24 +15,27 @@
 
 /** @typedef {import("./interfaces").IPDFXfaLayerFactory} IPDFXfaLayerFactory */
 
+import { SimpleLinkService } from "./pdf_link_service.js";
 import { XfaLayer } from "pdfjs-lib";
 
 /**
  * @typedef {Object} XfaLayerBuilderOptions
  * @property {HTMLDivElement} pageDiv
  * @property {PDFPage} pdfPage
  * @property {AnnotationStorage} [annotationStorage]
+ * @property {IPDFLinkService} linkService
  * @property {Object} [xfaHtml]
  */
 
 class XfaLayerBuilder {
   /**
    * @param {XfaLayerBuilderOptions} options
    */
-  constructor({ pageDiv, pdfPage, annotationStorage, xfaHtml }) {
+  constructor({ pageDiv, pdfPage, annotationStorage, linkService, xfaHtml }) {
     this.pageDiv = pageDiv;
     this.pdfPage = pdfPage;
     this.annotationStorage = annotationStorage;
+    this.linkService = linkService;
     this.xfaHtml = xfaHtml;
 
     this.div = null;
@@ -54,6 +57,7 @@ class XfaLayerBuilder {
         xfa: this.xfaHtml,
         page: null,
         annotationStorage: this.annotationStorage,
+        linkService: this.linkService,
         intent,
       };
 
@@ -80,6 +84,7 @@ class XfaLayerBuilder {
           xfa,
           page: this.pdfPage,
           annotationStorage: this.annotationStorage,
+          linkService: this.linkService,
           intent,
         };
 
@@ -129,6 +134,7 @@ class DefaultXfaLayerFactory {
       pageDiv,
       pdfPage,
       annotationStorage,
+      linkService: new SimpleLinkService(),
       xfaHtml,
     });
   }

Original file line number	Diff line number	Diff line change
`@@ -1366,6 +1366,7 @@ class BaseViewer {`
`1366`	`1366`	`pdfPage,`
`1367`	`1367`	`annotationStorage:`
`1368`	`1368`	`annotationStorage \|\| this.pdfDocument?.annotationStorage,`
	`1369`	`+ linkService: this.linkService,`
`1369`	`1370`	`});`
`1370`	`1371`	`}`
`1371`	`1372`