Unify handling of various cursor tools, e.g. the current Hand Tool and a possible future Zoom Tool, in a new PDFCursorTools module

With the current way that the `HandTool` is implemented, if someone would try to also add a Zoom tool (as issue 1260 asks for) that probably wouldn't work very well given that you'd then have two cursor tools which may not play nice together.
Hence this patch, which attempts to refactor things so that it should be simpler to add e.g. a Zoom tool as well (given that that issue is marked as "good-beginner-bug", and I'm not sure if that really applies considering the current state of the code).

Note that I personally have no interest in implementing a Zoom tool (similar to Adobe Reader) since I wouldn't use it, but I figured that it can't hurt to make this code a bit more future proof.

extensions/chromium/preferences_schema.json

@@ -27,11 +27,19 @@
       "default": 0
     },
     "enableHandToolOnLoad": {
-      "title": "Activate Hand tool by default",
-      "description": "Whether to activate the hand tool by default.",
       "type": "boolean",
       "default": false
     },
+    "cursorToolOnLoad": {
+      "title": "Cursor tool on load",
+      "description": "The cursor tool that is enabled upon load.\n 0 = Text selection tool.\n 1 = Hand tool.",
+      "type": "integer",
+      "enum": [
+        0,
+        1
+      ],
+      "default": 0
+    },
     "enableWebGL": {
       "title": "Enable WebGL",
       "description": "Whether to enable WebGL.",

l10n/en-US/viewer.properties

@@ -60,10 +60,10 @@ page_rotate_ccw.title=Rotate Counterclockwise
 page_rotate_ccw.label=Rotate Counterclockwise
 page_rotate_ccw_label=Rotate Counterclockwise
 
-hand_tool_enable.title=Enable hand tool
+cursor_text_select_tool.title=Enable Text Selection Tool
-hand_tool_enable_label=Enable hand tool
+cursor_text_select_tool_label=Text Selection Tool
-hand_tool_disable.title=Disable hand tool
+cursor_hand_tool.title=Enable Hand Tool
-hand_tool_disable_label=Disable hand tool
+cursor_hand_tool_label=Hand Tool
 
 # Document properties dialog box
 document_properties.title=Document Properties…

l10n/sv-SE/viewer.properties

@@ -60,10 +60,10 @@ page_rotate_ccw.title=Rotera moturs
 page_rotate_ccw.label=Rotera moturs
 page_rotate_ccw_label=Rotera moturs
 
-hand_tool_enable.title=Aktivera handverktyg
+cursor_text_select_tool.title=Aktivera Textmarkeringsverktyg
-hand_tool_enable_label=Aktivera handverktyg
+cursor_text_select_tool_label=Textmarkeringsverktyg
-hand_tool_disable.title=Inaktivera handverktyg
+cursor_hand_tool.title=Aktivera handverktyg
-hand_tool_disable_label=Inaktivera handverktyg
+cursor_hand_tool_label=Handverktyg
 
 # Document properties dialog box
 document_properties.title=Dokumentegenskaper…

web/app.js

@@ -24,13 +24,11 @@ import {
   MissingPDFException, OPS, PDFJS, shadow, UnexpectedResponseException,
   UNSUPPORTED_FEATURES, version,
 } from './pdfjs';
-import {
+import { CursorTool, PDFCursorTools } from './pdf_cursor_tools';
-  PDFRenderingQueue, RenderingStates
+import { PDFRenderingQueue, RenderingStates } from './pdf_rendering_queue';
-} from './pdf_rendering_queue';
 import { PDFSidebar, SidebarView } from './pdf_sidebar';
 import { PDFViewer, PresentationModeState } from './pdf_viewer';
 import { getGlobalEventBus } from './dom_events';
-import { HandTool } from './hand_tool';
 import { OverlayManager } from './overlay_manager';
 import { PasswordPrompt } from './password_prompt';
 import { PDFAttachmentViewer } from './pdf_attachment_viewer';
@@ -115,6 +113,8 @@ var PDFViewerApplication = {
   pdfOutlineViewer: null,
   /** @type {PDFAttachmentViewer} */
   pdfAttachmentViewer: null,
+  /** @type {PDFCursorTools} */
+  pdfCursorTools: null,
   /** @type {ViewHistory} */
   store: null,
   /** @type {DownloadManager} */
@@ -337,15 +337,15 @@ var PDFViewerApplication = {
 
       this.overlayManager = OverlayManager;
 
-      this.handTool = new HandTool({
+      this.pdfDocumentProperties =
+        new PDFDocumentProperties(appConfig.documentProperties);
+
+      this.pdfCursorTools = new PDFCursorTools({
         container,
         eventBus,
         preferences: this.preferences,
       });
 
-      this.pdfDocumentProperties =
-        new PDFDocumentProperties(appConfig.documentProperties);
-
       this.toolbar = new Toolbar(appConfig.toolbar, container, eventBus);
 
       this.secondaryToolbar =
@@ -2119,11 +2119,13 @@ function webViewerKeyDown(evt) {
         }
         break;
 
-      case 72: // 'h'
+      case 83: // 's'
-        if (!isViewerInPresentationMode) {
+        PDFViewerApplication.pdfCursorTools.switchTool(CursorTool.SELECT);
-          PDFViewerApplication.handTool.toggle();
-        }
         break;
+      case 72: // 'h'
+        PDFViewerApplication.pdfCursorTools.switchTool(CursorTool.HAND);
+        break;
+
       case 82: // 'r'
         PDFViewerApplication.rotatePages(90);
         break;

web/default_preferences.json

@@ -3,6 +3,7 @@
   "defaultZoomValue": "",
   "sidebarViewOnLoad": 0,
   "enableHandToolOnLoad": false,
+  "cursorToolOnLoad": 0,
   "enableWebGL": false,
   "pdfBugEnabled": false,
   "disableRange": false,

web/hand_tool.js

@@ -1,93 +0,0 @@
-/* Copyright 2013 Mozilla Foundation
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import { GrabToPan } from './grab_to_pan';
-import { localized } from './ui_utils';
-
-/**
- * @typedef {Object} HandToolOptions
- * @property {HTMLDivElement} container - The document container.
- * @property {EventBus} eventBus - The application event bus.
- * @property {BasePreferences} preferences - Object for reading/writing
- *                                           persistent settings.
- */
-
-class HandTool {
-  /**
-   * @param {HandToolOptions} options
-   */
-  constructor({ container, eventBus, preferences, }) {
-    this.container = container;
-    this.eventBus = eventBus;
-
-    this.wasActive = false;
-
-    this.handTool = new GrabToPan({
-      element: this.container,
-      onActiveChanged: (isActive) => {
-        this.eventBus.dispatch('handtoolchanged', { isActive, });
-      },
-    });
-
-    this.eventBus.on('togglehandtool', this.toggle.bind(this));
-
-    let enableOnLoad = preferences.get('enableHandToolOnLoad');
-    Promise.all([localized, enableOnLoad]).then((values) => {
-      if (values[1] === true) {
-        this.handTool.activate();
-      }
-    }).catch(function(reason) {});
-
-    this.eventBus.on('presentationmodechanged', (evt) => {
-      if (evt.switchInProgress) {
-        return;
-      }
-      if (evt.active) {
-        this.enterPresentationMode();
-      } else {
-        this.exitPresentationMode();
-      }
-    });
-  }
-
-  /**
-   * @return {boolean}
-   */
-  get isActive() {
-    return !!this.handTool.active;
-  }
-
-  toggle() {
-    this.handTool.toggle();
-  }
-
-  enterPresentationMode() {
-    if (this.isActive) {
-      this.wasActive = true;
-      this.handTool.deactivate();
-    }
-  }
-
-  exitPresentationMode() {
-    if (this.wasActive) {
-      this.wasActive = false;
-      this.handTool.activate();
-    }
-  }
-}
-
-export {
-  HandTool,
-};

web/pdf_cursor_tools.js

@@ -0,0 +1,163 @@
+/* Copyright 2017 Mozilla Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { GrabToPan } from './grab_to_pan';
+
+const CursorTool = {
+  SELECT: 0, // The default value.
+  HAND: 1,
+  ZOOM: 2,
+};
+
+/**
+ * @typedef {Object} PDFCursorToolsOptions
+ * @property {HTMLDivElement} container - The document container.
+ * @property {EventBus} eventBus - The application event bus.
+ * @property {BasePreferences} preferences - Object for reading/writing
+ *                                           persistent settings.
+ */
+
+class PDFCursorTools {
+  /**
+   * @param {PDFCursorToolsOptions} options
+   */
+  constructor({ container, eventBus, preferences, }) {
+    this.container = container;
+    this.eventBus = eventBus;
+
+    this.active = CursorTool.SELECT;
+    this.activeBeforePresentationMode = null;
+
+    this.handTool = new GrabToPan({
+      element: this.container,
+    });
+
+    this._addEventListeners();
+
+    Promise.all([
+      preferences.get('cursorToolOnLoad'),
+      preferences.get('enableHandToolOnLoad')
+    ]).then(([cursorToolPref, handToolPref]) => {
+      // If the 'cursorToolOnLoad' preference has not been set to a non-default
+      // value, attempt to convert the old 'enableHandToolOnLoad' preference.
+      // TODO: Remove this conversion after a suitable number of releases.
+      if (handToolPref === true) {
+        preferences.set('enableHandToolOnLoad', false);
+
+        if (cursorToolPref === CursorTool.SELECT) {
+          cursorToolPref = CursorTool.HAND;
+          preferences.set('cursorToolOnLoad', cursorToolPref).catch(() => { });
+        }
+      }
+      this.switchTool(cursorToolPref);
+    }).catch(() => { });
+  }
+
+  /**
+   * @returns {number} One of the values in {CursorTool}.
+   */
+  get activeTool() {
+    return this.active;
+  }
+
+  /**
+   * NOTE: This method is ignored while Presentation Mode is active.
+   * @param {number} tool - The cursor mode that should be switched to,
+   *                        must be one of the values in {CursorTool}.
+   */
+  switchTool(tool) {
+    if (this.activeBeforePresentationMode !== null) {
+      return; // Cursor tools cannot be used in Presentation Mode.
+    }
+    if (tool === this.active) {
+      return; // The requested tool is already active.
+    }
+
+    let disableActiveTool = () => {
+      switch (this.active) {
+        case CursorTool.SELECT:
+          break;
+        case CursorTool.HAND:
+          this.handTool.deactivate();
+          break;
+        case CursorTool.ZOOM:
+          /* falls through */
+      }
+    };
+
+    switch (tool) { // Enable the new cursor tool.
+      case CursorTool.SELECT:
+        disableActiveTool();
+        break;
+      case CursorTool.HAND:
+        disableActiveTool();
+        this.handTool.activate();
+        break;
+      case CursorTool.ZOOM:
+        /* falls through */
+      default:
+        console.error(`switchTool: "${tool}" is an unsupported value.`);
+        return;
+    }
+    // Update the active tool *after* it has been validated above,
+    // in order to prevent setting it to an invalid state.
+    this.active = tool;
+
+    this._dispatchEvent();
+  }
+
+  /**
+   * @private
+   */
+  _dispatchEvent() {
+    this.eventBus.dispatch('cursortoolchanged', {
+      source: this,
+      tool: this.active,
+    });
+  }
+
+  /**
+   * @private
+   */
+  _addEventListeners() {
+    this.eventBus.on('switchcursortool', (evt) => {
+      this.switchTool(evt.tool);
+    });
+
+    this.eventBus.on('presentationmodechanged', (evt) => {
+      if (evt.switchInProgress) {
+        return;
+      }
+      let previouslyActive;
+
+      if (evt.active) {
+        previouslyActive = this.active;
+
+        this.switchTool(CursorTool.SELECT);
+        this.activeBeforePresentationMode = previouslyActive;
+      } else {
+        previouslyActive = this.activeBeforePresentationMode;
+
+        this.activeBeforePresentationMode = null;
+        this.switchTool(previouslyActive);
+      }
+    });
+  }
+}
+
+export {
+  CursorTool,
+  PDFCursorTools,
+};

web/secondary_toolbar.js

@@ -13,7 +13,8 @@
  * limitations under the License.
  */
 
-import { mozL10n, SCROLLBAR_PADDING } from './ui_utils';
+import { CursorTool } from './pdf_cursor_tools';
+import { SCROLLBAR_PADDING } from './ui_utils';
 
 /**
  * @typedef {Object} SecondaryToolbarOptions
@@ -39,7 +40,9 @@ import { mozL10n, SCROLLBAR_PADDING } from './ui_utils';
  *   clockwise.
  * @property {HTMLButtonElement} pageRotateCcwButton - Button to rotate the
  *   pages counterclockwise.
- * @property {HTMLButtonElement} toggleHandToolButton - Button to toggle the
+ * @property {HTMLButtonElement} cursorSelectToolButton - Button to enable the
+ *   select tool.
+ * @property {HTMLButtonElement} cursorHandToolButton - Button to enable the
  *   hand tool.
  * @property {HTMLButtonElement} documentPropertiesButton - Button for opening
  *   the document properties dialog.
@@ -68,8 +71,10 @@ class SecondaryToolbar {
         close: false },
       { element: options.pageRotateCcwButton, eventName: 'rotateccw',
         close: false },
-      { element: options.toggleHandToolButton, eventName: 'togglehandtool',
+      { element: options.cursorSelectToolButton, eventName: 'switchcursortool',
-        close: true },
+        eventDetails: { tool: CursorTool.SELECT, }, close: true },
+      { element: options.cursorHandToolButton, eventName: 'switchcursortool',
+        eventDetails: { tool: CursorTool.HAND, }, close: true },
       { element: options.documentPropertiesButton,
         eventName: 'documentproperties', close: true }
     ];
@@ -89,9 +94,9 @@ class SecondaryToolbar {
 
     this.reset();
 
-    // Bind the event listeners for click and hand tool actions.
+    // Bind the event listeners for click and cursor tool actions.
     this._bindClickListeners();
-    this._bindHandToolListener(options.toggleHandToolButton);
+    this._bindCursorToolsListener(options);
 
     // Bind the event listener for adjusting the 'max-height' of the toolbar.
     this.eventBus.on('resize', this._setMaxHeight.bind(this));
@@ -133,11 +138,15 @@ class SecondaryToolbar {
 
     // All items within the secondary toolbar.
     for (let button in this.buttons) {
-      let { element, eventName, close, } = this.buttons[button];
+      let { element, eventName, close, eventDetails, } = this.buttons[button];
 
       element.addEventListener('click', (evt) => {
         if (eventName !== null) {
-          this.eventBus.dispatch(eventName, { source: this, });
+          let details = { source: this, };
+          for (let property in eventDetails) {
+            details[property] = eventDetails[property];
+          }
+          this.eventBus.dispatch(eventName, details);
         }
         if (close) {
           this.close();
@@ -146,25 +155,18 @@ class SecondaryToolbar {
     }
   }
 
-  _bindHandToolListener(toggleHandToolButton) {
+  _bindCursorToolsListener(buttons) {
-    let isHandToolActive = false;
+    this.eventBus.on('cursortoolchanged', function(evt) {
+      buttons.cursorSelectToolButton.classList.remove('toggled');
+      buttons.cursorHandToolButton.classList.remove('toggled');
 
-    this.eventBus.on('handtoolchanged', function(evt) {
+      switch (evt.tool) {
-      if (isHandToolActive === evt.isActive) {
+        case CursorTool.SELECT:
-        return;
+          buttons.cursorSelectToolButton.classList.add('toggled');
-      }
+          break;
-      isHandToolActive = evt.isActive;
+        case CursorTool.HAND:
-
+          buttons.cursorHandToolButton.classList.add('toggled');
-      if (isHandToolActive) {
+          break;
-        toggleHandToolButton.title =
-          mozL10n.get('hand_tool_disable.title', null, 'Disable hand tool');
-        toggleHandToolButton.firstElementChild.textContent =
-          mozL10n.get('hand_tool_disable_label', null, 'Disable hand tool');
-      } else {
-        toggleHandToolButton.title =
-          mozL10n.get('hand_tool_enable.title', null, 'Enable hand tool');
-        toggleHandToolButton.firstElementChild.textContent =
-          mozL10n.get('hand_tool_enable_label', null, 'Enable hand tool');
       }
     });
   }

web/viewer.css

@@ -1010,6 +1010,10 @@ html[dir="rtl"] .secondaryToolbarButton > span {
   content: url(images/secondaryToolbarButton-rotateCw.png);
 }
 
+.secondaryToolbarButton.selectTool::before {
+  content: url(images/secondaryToolbarButton-selectTool.png);
+}
+
 .secondaryToolbarButton.handTool::before {
   content: url(images/secondaryToolbarButton-handTool.png);
 }
@@ -1735,6 +1739,10 @@ html[dir='rtl'] #documentPropertiesOverlay .row > * {
     content: url(images/secondaryToolbarButton-rotateCw@2x.png);
   }
 
+  .secondaryToolbarButton.selectTool::before {
+    content: url(images/secondaryToolbarButton-selectTool@2x.png);
+  }
+
   .secondaryToolbarButton.handTool::before {
     content: url(images/secondaryToolbarButton-handTool@2x.png);
   }

web/viewer.html

@@ -166,13 +166,16 @@ See https://github.com/adobe-type-tools/cmap-resources
 
             <div class="horizontalToolbarSeparator"></div>
 
-            <button id="toggleHandTool" class="secondaryToolbarButton handTool" title="Enable hand tool" tabindex="60" data-l10n-id="hand_tool_enable">
+            <button id="cursorSelectTool" class="secondaryToolbarButton selectTool toggled" title="Enable Text Selection Tool" tabindex="60" data-l10n-id="cursor_text_select_tool">
-              <span data-l10n-id="hand_tool_enable_label">Enable hand tool</span>
+              <span data-l10n-id="cursor_text_select_tool_label">Text Selection Tool</span>
+            </button>
+            <button id="cursorHandTool" class="secondaryToolbarButton handTool" title="Enable Hand Tool" tabindex="61" data-l10n-id="cursor_hand_tool">
+              <span data-l10n-id="cursor_hand_tool_label">Hand Tool</span>
             </button>
 
             <div class="horizontalToolbarSeparator"></div>
 
-            <button id="documentProperties" class="secondaryToolbarButton documentProperties" title="Document Properties…" tabindex="61" data-l10n-id="document_properties">
+            <button id="documentProperties" class="secondaryToolbarButton documentProperties" title="Document Properties…" tabindex="62" data-l10n-id="document_properties">
               <span data-l10n-id="document_properties_label">Document Properties…</span>
             </button>
           </div>

web/viewer.js

@@ -93,7 +93,8 @@ function getViewerConfiguration() {
       lastPageButton: document.getElementById('lastPage'),
       pageRotateCwButton: document.getElementById('pageRotateCw'),
       pageRotateCcwButton: document.getElementById('pageRotateCcw'),
-      toggleHandToolButton: document.getElementById('toggleHandTool'),
+      cursorSelectToolButton: document.getElementById('cursorSelectTool'),
+      cursorHandToolButton: document.getElementById('cursorHandTool'),
       documentPropertiesButton: document.getElementById('documentProperties'),
     },
     fullscreen: {