From 3a6f6d23d6190483ba7c8b4ca298e2bd209c6809 Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Tue, 13 Feb 2018 14:03:52 +0100
Subject: [PATCH] Move the `externalLinkTarget` and `externalLinkRel` options
 to `PDFLinkService` options

This removes the `PDFJS.externalLinkTarget`/`PDFJS.externalLinkRel` dependency from the viewer components, but please note that as a *temporary* solution the default viewer still uses it.
---
 src/display/annotation_layer.js | 16 +++++----
 src/display/dom_utils.js        | 61 +++++++--------------------------
 src/display/global.js           | 11 ++----
 src/pdf.js                      |  1 +
 test/unit/dom_utils_spec.js     | 38 +-------------------
 web/app.js                      | 11 +++---
 web/pdf_link_service.js         | 16 ++++++++-
 web/pdf_outline_viewer.js       | 28 +++++++--------
 8 files changed, 62 insertions(+), 120 deletions(-)

diff --git a/src/display/annotation_layer.js b/src/display/annotation_layer.js
index edb500d75..048bfb562 100644
--- a/src/display/annotation_layer.js
+++ b/src/display/annotation_layer.js
@@ -281,17 +281,21 @@ class LinkAnnotationElement extends AnnotationElement {
   render() {
     this.container.className = 'linkAnnotation';
 
+    let { data, linkService, } = this;
     let link = document.createElement('a');
+
     addLinkAttributes(link, {
-      url: this.data.url,
-      target: (this.data.newWindow ? LinkTarget.BLANK : undefined),
+      url: data.url,
+      target: (data.newWindow ?
+               LinkTarget.BLANK : linkService.externalLinkTarget),
+      rel: linkService.externalLinkRel,
     });
 
-    if (!this.data.url) {
-      if (this.data.action) {
-        this._bindNamedAction(link, this.data.action);
+    if (!data.url) {
+      if (data.action) {
+        this._bindNamedAction(link, data.action);
       } else {
-        this._bindLink(link, this.data.dest);
+        this._bindLink(link, data.dest);
       }
     }
 
diff --git a/src/display/dom_utils.js b/src/display/dom_utils.js
index 0551c90ce..52bcf4895 100644
--- a/src/display/dom_utils.js
+++ b/src/display/dom_utils.js
@@ -274,7 +274,7 @@ var RenderingCancelledException = (function RenderingCancelledException() {
   return RenderingCancelledException;
 })();
 
-var LinkTarget = {
+const LinkTarget = {
   NONE: 0, // Default value.
   SELF: 1,
   BLANK: 2,
@@ -282,7 +282,7 @@ var LinkTarget = {
   TOP: 4,
 };
 
-var LinkTargetStringMap = [
+const LinkTargetStringMap = [
   '',
   '_self',
   '_blank',
@@ -294,8 +294,10 @@ var LinkTargetStringMap = [
  * @typedef ExternalLinkParameters
  * @typedef {Object} ExternalLinkParameters
  * @property {string} url - An absolute URL.
- * @property {LinkTarget} target - The link target.
- * @property {string} rel - The link relationship.
+ * @property {LinkTarget} target - (optional) The link target.
+ *   The default value is `LinkTarget.NONE`.
+ * @property {string} rel - (optional) The link relationship.
+ *   The default value is `DEFAULT_LINK_REL`.
  */
 
 /**
@@ -303,22 +305,16 @@ var LinkTargetStringMap = [
  * @param {HTMLLinkElement} link - The link element.
  * @param {ExternalLinkParameters} params
  */
-function addLinkAttributes(link, params) {
-  var url = params && params.url;
+function addLinkAttributes(link, { url, target, rel, } = {}) {
   link.href = link.title = (url ? removeNullCharacters(url) : '');
 
   if (url) {
-    var target = params.target;
-    if (typeof target === 'undefined') {
-      target = getDefaultSetting('externalLinkTarget');
-    }
-    link.target = LinkTargetStringMap[target];
+    const LinkTargetValues = Object.values(LinkTarget);
+    let targetIndex =
+      LinkTargetValues.includes(target) ? target : LinkTarget.NONE;
+    link.target = LinkTargetStringMap[targetIndex];
 
-    var rel = params.rel;
-    if (typeof rel === 'undefined') {
-      rel = getDefaultSetting('externalLinkRel');
-    }
-    link.rel = rel;
+    link.rel = (typeof rel === 'string' ? rel : DEFAULT_LINK_REL);
   }
 }
 
@@ -365,25 +361,6 @@ function getDefaultSetting(id) {
       return globalSettings ? globalSettings.maxImageSize : -1;
     case 'isEvalSupported':
       return globalSettings ? globalSettings.isEvalSupported : true;
-    case 'externalLinkTarget':
-      if (!globalSettings) {
-        return LinkTarget.NONE;
-      }
-      switch (globalSettings.externalLinkTarget) {
-        case LinkTarget.NONE:
-        case LinkTarget.SELF:
-        case LinkTarget.BLANK:
-        case LinkTarget.PARENT:
-        case LinkTarget.TOP:
-          return globalSettings.externalLinkTarget;
-      }
-      warn('PDFJS.externalLinkTarget is invalid: ' +
-           globalSettings.externalLinkTarget);
-      // Reset the external link target, to suppress further warnings.
-      globalSettings.externalLinkTarget = LinkTarget.NONE;
-      return LinkTarget.NONE;
-    case 'externalLinkRel':
-      return globalSettings ? globalSettings.externalLinkRel : DEFAULT_LINK_REL;
     case 'enableStats':
       return !!(globalSettings && globalSettings.enableStats);
     default:
@@ -391,19 +368,6 @@ function getDefaultSetting(id) {
   }
 }
 
-function isExternalLinkTargetSet() {
-  var externalLinkTarget = getDefaultSetting('externalLinkTarget');
-  switch (externalLinkTarget) {
-    case LinkTarget.NONE:
-      return false;
-    case LinkTarget.SELF:
-    case LinkTarget.BLANK:
-    case LinkTarget.PARENT:
-    case LinkTarget.TOP:
-      return true;
-  }
-}
-
 class StatTimer {
   constructor(enable = true) {
     this.enabled = !!enable;
@@ -481,7 +445,6 @@ class DummyStatTimer {
 export {
   RenderingCancelledException,
   addLinkAttributes,
-  isExternalLinkTargetSet,
   getFilenameFromUrl,
   LinkTarget,
   getDefaultSetting,
diff --git a/src/display/global.js b/src/display/global.js
index 199b87831..e764752a8 100644
--- a/src/display/global.js
+++ b/src/display/global.js
@@ -13,10 +13,6 @@
  * limitations under the License.
  */
 
-import {
-  addLinkAttributes, DEFAULT_LINK_REL, getFilenameFromUrl,
-  isExternalLinkTargetSet, isValidUrl, LinkTarget
-} from './dom_utils';
 import {
   createBlob, createObjectURL, createPromiseCapability, getVerbosityLevel,
   InvalidPDFException, isLittleEndian, MissingPDFException, OPS, PageViewport,
@@ -24,6 +20,7 @@ import {
   shadow, UnexpectedResponseException, UnknownErrorException,
   UNSUPPORTED_FEATURES, Util, VERBOSITY_LEVELS
 } from '../shared/util';
+import { DEFAULT_LINK_REL, getFilenameFromUrl, LinkTarget } from './dom_utils';
 import {
   getDocument, LoopbackPort, PDFDataRangeTransport, PDFWorker
 } from './api';
@@ -67,7 +64,6 @@ Object.defineProperty(PDFJS, 'verbosity', {
 PDFJS.VERBOSITY_LEVELS = VERBOSITY_LEVELS;
 PDFJS.OPS = OPS;
 PDFJS.UNSUPPORTED_FEATURES = UNSUPPORTED_FEATURES;
-PDFJS.isValidUrl = isValidUrl;
 PDFJS.shadow = shadow;
 PDFJS.createBlob = createBlob;
 PDFJS.createObjectURL = function PDFJS_createObjectURL(data, contentType) {
@@ -193,7 +189,7 @@ PDFJS.disableWebGL = (PDFJS.disableWebGL === undefined ?
 
 /**
  * Specifies the |target| attribute for external links.
- * The constants from PDFJS.LinkTarget should be used:
+ * The constants from {LinkTarget} should be used:
  *  - NONE [default]
  *  - SELF
  *  - BLANK
@@ -225,10 +221,7 @@ PDFJS.LoopbackPort = LoopbackPort;
 PDFJS.PDFDataRangeTransport = PDFDataRangeTransport;
 PDFJS.PDFWorker = PDFWorker;
 
-PDFJS.LinkTarget = LinkTarget;
-PDFJS.addLinkAttributes = addLinkAttributes;
 PDFJS.getFilenameFromUrl = getFilenameFromUrl;
-PDFJS.isExternalLinkTargetSet = isExternalLinkTargetSet;
 
 PDFJS.AnnotationLayer = AnnotationLayer;
 
diff --git a/src/pdf.js b/src/pdf.js
index 38e5e458c..644ff8a37 100644
--- a/src/pdf.js
+++ b/src/pdf.js
@@ -91,4 +91,5 @@ exports.createBlob = pdfjsSharedUtil.createBlob;
 exports.RenderingCancelledException =
   pdfjsDisplayDOMUtils.RenderingCancelledException;
 exports.getFilenameFromUrl = pdfjsDisplayDOMUtils.getFilenameFromUrl;
+exports.LinkTarget = pdfjsDisplayDOMUtils.LinkTarget;
 exports.addLinkAttributes = pdfjsDisplayDOMUtils.addLinkAttributes;
diff --git a/test/unit/dom_utils_spec.js b/test/unit/dom_utils_spec.js
index 314459450..938aafd83 100644
--- a/test/unit/dom_utils_spec.js
+++ b/test/unit/dom_utils_spec.js
@@ -13,11 +13,8 @@
  * limitations under the License.
  */
 
-import {
-  DOMSVGFactory, getFilenameFromUrl, isExternalLinkTargetSet, LinkTarget
-} from '../../src/display/dom_utils';
+import { DOMSVGFactory, getFilenameFromUrl } from '../../src/display/dom_utils';
 import isNodeJS from '../../src/shared/is_node';
-import { PDFJS } from '../../src/display/global';
 
 describe('dom_utils', function() {
   describe('DOMSVGFactory', function() {
@@ -95,37 +92,4 @@ describe('dom_utils', function() {
       expect(result).toEqual(expected);
     });
   });
-
-  describe('isExternalLinkTargetSet', function() {
-    var savedExternalLinkTarget;
-
-    beforeAll(function (done) {
-      savedExternalLinkTarget = PDFJS.externalLinkTarget;
-      done();
-    });
-
-    afterAll(function () {
-      PDFJS.externalLinkTarget = savedExternalLinkTarget;
-    });
-
-    it('handles the predefined LinkTargets', function() {
-      for (var key in LinkTarget) {
-        var linkTarget = LinkTarget[key];
-        PDFJS.externalLinkTarget = linkTarget;
-
-        expect(isExternalLinkTargetSet()).toEqual(!!linkTarget);
-      }
-    });
-
-    it('handles incorrect LinkTargets', function() {
-      var targets = [true, '', false, -1, '_blank', null];
-
-      for (var i = 0, ii = targets.length; i < ii; i++) {
-        var linkTarget = targets[i];
-        PDFJS.externalLinkTarget = linkTarget;
-
-        expect(isExternalLinkTargetSet()).toEqual(false);
-      }
-    });
-  });
 });
diff --git a/web/app.js b/web/app.js
index 35a1da2db..df983f181 100644
--- a/web/app.js
+++ b/web/app.js
@@ -22,7 +22,7 @@ import {
 } from './ui_utils';
 import {
   build, createBlob, getDocument, getFilenameFromUrl, InvalidPDFException,
-  MissingPDFException, OPS, PDFJS, PDFWorker, shadow,
+  LinkTarget, MissingPDFException, OPS, PDFJS, PDFWorker, shadow,
   UnexpectedResponseException, UNSUPPORTED_FEATURES, version
 } from 'pdfjs-lib';
 import { CursorTool, PDFCursorTools } from './pdf_cursor_tools';
@@ -184,10 +184,11 @@ let PDFViewerApplication = {
         this.eventBus.dispatch('localized');
       });
 
-      if (this.isViewerEmbedded && !PDFJS.isExternalLinkTargetSet()) {
+      if (this.isViewerEmbedded &&
+          PDFJS.externalLinkTarget === LinkTarget.NONE) {
         // Prevent external links from "replacing" the viewer,
         // when it's embedded in e.g. an iframe or an object.
-        PDFJS.externalLinkTarget = PDFJS.LinkTarget.TOP;
+        PDFJS.externalLinkTarget = LinkTarget.TOP;
       }
 
       this.initialized = true;
@@ -250,7 +251,7 @@ let PDFViewerApplication = {
         PDFJS.useOnlyCssZoom = value;
       }),
       preferences.get('externalLinkTarget').then(function resolved(value) {
-        if (PDFJS.isExternalLinkTargetSet()) {
+        if (PDFJS.externalLinkTarget !== LinkTarget.NONE) {
           return;
         }
         PDFJS.externalLinkTarget = value;
@@ -378,6 +379,8 @@ let PDFViewerApplication = {
 
       let pdfLinkService = new PDFLinkService({
         eventBus,
+        externalLinkTarget: PDFJS.externalLinkTarget,
+        externalLinkRel: PDFJS.externalLinkRel,
       });
       this.pdfLinkService = pdfLinkService;
 
diff --git a/web/pdf_link_service.js b/web/pdf_link_service.js
index 7062a69ae..91df46320 100644
--- a/web/pdf_link_service.js
+++ b/web/pdf_link_service.js
@@ -19,6 +19,11 @@ import { parseQueryString } from './ui_utils';
 /**
  * @typedef {Object} PDFLinkServiceOptions
  * @property {EventBus} eventBus - The application event bus.
+ * @property {number} externalLinkTarget - (optional) Specifies the `target`
+ *   attribute for external links. Must use one of the values from {LinkTarget}.
+ *   Defaults to using no target.
+ * @property {string} externalLinkRel - (optional) Specifies the `rel` attribute
+ *   for external links. Defaults to stripping the referrer.
  */
 
 /**
@@ -30,8 +35,12 @@ class PDFLinkService {
   /**
    * @param {PDFLinkServiceOptions} options
    */
-  constructor({ eventBus, } = {}) {
+  constructor({ eventBus, externalLinkTarget = null,
+                externalLinkRel = null, } = {}) {
     this.eventBus = eventBus || getGlobalEventBus();
+    this.externalLinkTarget = externalLinkTarget;
+    this.externalLinkRel = externalLinkRel;
+
     this.baseUrl = null;
     this.pdfDocument = null;
     this.pdfViewer = null;
@@ -409,6 +418,11 @@ function isValidExplicitDestination(dest) {
 }
 
 class SimpleLinkService {
+  constructor() {
+    this.externalLinkTarget = null;
+    this.externalLinkRel = null;
+  }
+
   /**
    * @returns {number}
    */
diff --git a/web/pdf_outline_viewer.js b/web/pdf_outline_viewer.js
index 7aee5b626..05b94016c 100644
--- a/web/pdf_outline_viewer.js
+++ b/web/pdf_outline_viewer.js
@@ -13,9 +13,7 @@
  * limitations under the License.
  */
 
-import {
-  addLinkAttributes, PDFJS, removeNullCharacters
-} from 'pdfjs-lib';
+import { addLinkAttributes, LinkTarget, removeNullCharacters } from 'pdfjs-lib';
 
 const DEFAULT_TITLE = '\u2013';
 
@@ -68,20 +66,22 @@ class PDFOutlineViewer {
   /**
    * @private
    */
-  _bindLink(element, item) {
-    if (item.url) {
+  _bindLink(element, { url, newWindow, dest, }) {
+    let { linkService, } = this;
+
+    if (url) {
       addLinkAttributes(element, {
-        url: item.url,
-        target: (item.newWindow ? PDFJS.LinkTarget.BLANK : undefined),
+        url,
+        target: (newWindow ? LinkTarget.BLANK : linkService.externalLinkTarget),
+        rel: linkService.externalLinkRel,
       });
       return;
     }
-    let destination = item.dest;
 
-    element.href = this.linkService.getDestinationHash(destination);
+    element.href = this.linkService.getDestinationHash(dest);
     element.onclick = () => {
-      if (destination) {
-        this.linkService.navigateTo(destination);
+      if (dest) {
+        this.linkService.navigateTo(dest);
       }
       return false;
     };
@@ -90,12 +90,12 @@ class PDFOutlineViewer {
   /**
    * @private
    */
-  _setStyles(element, item) {
+  _setStyles(element, { bold, italic, }) {
     let styleStr = '';
-    if (item.bold) {
+    if (bold) {
       styleStr += 'font-weight: bold;';
     }
-    if (item.italic) {
+    if (italic) {
       styleStr += 'font-style: italic;';
     }