From 36c2791296adc4831a200fde5eed819781fc8174 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 7 Sep 2016 13:30:26 +0200 Subject: [PATCH] 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 | 12 +- l10n/en-US/viewer.properties | 8 +- l10n/sv-SE/viewer.properties | 8 +- web/app.js | 24 +-- web/default_preferences.json | 1 + web/hand_tool.js | 93 ----------- web/pdf_cursor_tools.js | 163 ++++++++++++++++++++ web/secondary_toolbar.js | 56 +++---- web/viewer.css | 8 + web/viewer.html | 9 +- web/viewer.js | 3 +- 11 files changed, 240 insertions(+), 145 deletions(-) delete mode 100644 web/hand_tool.js create mode 100644 web/pdf_cursor_tools.js diff --git a/extensions/chromium/preferences_schema.json b/extensions/chromium/preferences_schema.json index 763387f0e..55a0df979 100644 --- a/extensions/chromium/preferences_schema.json +++ b/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.", diff --git a/l10n/en-US/viewer.properties b/l10n/en-US/viewer.properties index 27939c13b..f1c505478 100644 --- a/l10n/en-US/viewer.properties +++ b/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 -hand_tool_enable_label=Enable hand tool -hand_tool_disable.title=Disable hand tool -hand_tool_disable_label=Disable hand tool +cursor_text_select_tool.title=Enable Text Selection Tool +cursor_text_select_tool_label=Text Selection Tool +cursor_hand_tool.title=Enable Hand Tool +cursor_hand_tool_label=Hand Tool # Document properties dialog box document_properties.title=Document Properties… diff --git a/l10n/sv-SE/viewer.properties b/l10n/sv-SE/viewer.properties index 4df0af9e3..db52fc2a2 100644 --- a/l10n/sv-SE/viewer.properties +++ b/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 -hand_tool_enable_label=Aktivera handverktyg -hand_tool_disable.title=Inaktivera handverktyg -hand_tool_disable_label=Inaktivera handverktyg +cursor_text_select_tool.title=Aktivera Textmarkeringsverktyg +cursor_text_select_tool_label=Textmarkeringsverktyg +cursor_hand_tool.title=Aktivera handverktyg +cursor_hand_tool_label=Handverktyg # Document properties dialog box document_properties.title=Dokumentegenskaper… diff --git a/web/app.js b/web/app.js index f39679cdf..92629b5ea 100644 --- a/web/app.js +++ b/web/app.js @@ -24,13 +24,11 @@ import { MissingPDFException, OPS, PDFJS, shadow, UnexpectedResponseException, UNSUPPORTED_FEATURES, version, } from './pdfjs'; -import { - PDFRenderingQueue, RenderingStates -} from './pdf_rendering_queue'; +import { CursorTool, PDFCursorTools } from './pdf_cursor_tools'; +import { PDFRenderingQueue, RenderingStates } 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 83: // 's' + PDFViewerApplication.pdfCursorTools.switchTool(CursorTool.SELECT); + break; case 72: // 'h' - if (!isViewerInPresentationMode) { - PDFViewerApplication.handTool.toggle(); - } + PDFViewerApplication.pdfCursorTools.switchTool(CursorTool.HAND); break; + case 82: // 'r' PDFViewerApplication.rotatePages(90); break; diff --git a/web/default_preferences.json b/web/default_preferences.json index dbb9dd5a5..73b315bff 100644 --- a/web/default_preferences.json +++ b/web/default_preferences.json @@ -3,6 +3,7 @@ "defaultZoomValue": "", "sidebarViewOnLoad": 0, "enableHandToolOnLoad": false, + "cursorToolOnLoad": 0, "enableWebGL": false, "pdfBugEnabled": false, "disableRange": false, diff --git a/web/hand_tool.js b/web/hand_tool.js deleted file mode 100644 index 48e0c4ca0..000000000 --- a/web/hand_tool.js +++ /dev/null @@ -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, -}; diff --git a/web/pdf_cursor_tools.js b/web/pdf_cursor_tools.js new file mode 100644 index 000000000..6bc3aa850 --- /dev/null +++ b/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, +}; diff --git a/web/secondary_toolbar.js b/web/secondary_toolbar.js index c86e073d6..7215a23be 100644 --- a/web/secondary_toolbar.js +++ b/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', - close: true }, + { element: options.cursorSelectToolButton, eventName: 'switchcursortool', + 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) { - let isHandToolActive = false; - - this.eventBus.on('handtoolchanged', function(evt) { - if (isHandToolActive === evt.isActive) { - return; - } - isHandToolActive = evt.isActive; - - if (isHandToolActive) { - 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'); + _bindCursorToolsListener(buttons) { + this.eventBus.on('cursortoolchanged', function(evt) { + buttons.cursorSelectToolButton.classList.remove('toggled'); + buttons.cursorHandToolButton.classList.remove('toggled'); + + switch (evt.tool) { + case CursorTool.SELECT: + buttons.cursorSelectToolButton.classList.add('toggled'); + break; + case CursorTool.HAND: + buttons.cursorHandToolButton.classList.add('toggled'); + break; } }); } diff --git a/web/viewer.css b/web/viewer.css index 78ef01d55..b0e580beb 100644 --- a/web/viewer.css +++ b/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); } diff --git a/web/viewer.html b/web/viewer.html index d08f6392e..e2a506bec 100644 --- a/web/viewer.html +++ b/web/viewer.html @@ -166,13 +166,16 @@ See https://github.com/adobe-type-tools/cmap-resources
- +
- diff --git a/web/viewer.js b/web/viewer.js index 01f27980f..ca31a3057 100644 --- a/web/viewer.js +++ b/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: {