From b6038c87b66630b341e431a4722856c9a3a282ed Mon Sep 17 00:00:00 2001 From: toasted-nutbread Date: Mon, 28 Dec 2020 17:41:59 -0500 Subject: [PATCH] Improve document focus control (#1167) * Improve styles for #content-scroll-focus * Create new class to manage and control document focus * Use new focus class * Add a check to prevent redundant .blur calls --- ext/bg/css/settings2.css | 8 ++ ext/bg/info.html | 1 + ext/bg/js/generic-page-main.js | 7 +- ext/bg/js/info-main.js | 4 +- ext/bg/js/permissions-main.js | 5 +- ext/bg/js/search-main.js | 6 +- ext/bg/js/search.js | 4 +- ext/bg/js/settings2/settings-main.js | 4 +- ext/bg/js/welcome-main.js | 4 +- ext/bg/legal.html | 2 + ext/bg/permissions.html | 2 + ext/bg/search.html | 1 + ext/bg/settings2.html | 1 + ext/bg/welcome.html | 1 + ext/fg/float.html | 1 + ext/fg/js/float-main.js | 6 +- ext/mixed/css/display.css | 18 ++++ ext/mixed/js/display.js | 28 +----- ext/mixed/js/document-focus-controller.js | 110 ++++++++++++++++++++++ 19 files changed, 179 insertions(+), 34 deletions(-) create mode 100644 ext/mixed/js/document-focus-controller.js diff --git a/ext/bg/css/settings2.css b/ext/bg/css/settings2.css index 80217d9b..0933019f 100644 --- a/ext/bg/css/settings2.css +++ b/ext/bg/css/settings2.css @@ -361,8 +361,16 @@ h3 { opacity: 0; margin: 0; padding: 0; + outline: none; background-color: transparent; display: inline; + width: 0; + height: 0; + line-height: 0; + user-select: none; +} +#content-scroll-focus::-moz-focus-inner { + border: 0; } .content-dimmer { display: block; diff --git a/ext/bg/info.html b/ext/bg/info.html index 99f595d1..cc69ed1c 100644 --- a/ext/bg/info.html +++ b/ext/bg/info.html @@ -62,6 +62,7 @@ + diff --git a/ext/bg/js/generic-page-main.js b/ext/bg/js/generic-page-main.js index 8e1b3c76..29fcf63d 100644 --- a/ext/bg/js/generic-page-main.js +++ b/ext/bg/js/generic-page-main.js @@ -15,13 +15,18 @@ * along with this program. If not, see . */ +/* global + * DocumentFocusController + */ + function setupEnvironmentInfo() { const {manifest_version: manifestVersion} = chrome.runtime.getManifest(); document.documentElement.dataset.manifestVersion = `${manifestVersion}`; } (() => { - document.querySelector('#content-scroll-focus').focus(); + const documentFocusController = new DocumentFocusController(); + documentFocusController.prepare(); document.documentElement.dataset.loaded = 'true'; setupEnvironmentInfo(); })(); diff --git a/ext/bg/js/info-main.js b/ext/bg/js/info-main.js index 58e9d3b7..71cfdbd8 100644 --- a/ext/bg/js/info-main.js +++ b/ext/bg/js/info-main.js @@ -17,6 +17,7 @@ /* global * BackupController + * DocumentFocusController * SettingsController * api */ @@ -47,7 +48,8 @@ function getOperatingSystemDisplayName(os) { (async () => { try { - document.querySelector('#content-scroll-focus').focus(); + const documentFocusController = new DocumentFocusController(); + documentFocusController.prepare(); const manifest = chrome.runtime.getManifest(); const language = chrome.i18n.getUILanguage(); diff --git a/ext/bg/js/permissions-main.js b/ext/bg/js/permissions-main.js index a464f40e..b1c0430e 100644 --- a/ext/bg/js/permissions-main.js +++ b/ext/bg/js/permissions-main.js @@ -16,6 +16,7 @@ */ /* global + * DocumentFocusController * api */ @@ -52,7 +53,9 @@ async function setPermissionsGranted(permissions, shouldHave) { (async () => { try { - document.querySelector('#content-scroll-focus').focus(); + const documentFocusController = new DocumentFocusController(); + documentFocusController.prepare(); + document.querySelector('#extension-id-example').textContent = chrome.runtime.getURL('/'); api.forwardLogsToBackend(); diff --git a/ext/bg/js/search-main.js b/ext/bg/js/search-main.js index f98028b3..4bcf14e8 100644 --- a/ext/bg/js/search-main.js +++ b/ext/bg/js/search-main.js @@ -17,6 +17,7 @@ /* global * DisplaySearch + * DocumentFocusController * JapaneseUtil * api * wanakana @@ -24,11 +25,14 @@ (async () => { try { + const documentFocusController = new DocumentFocusController(); + documentFocusController.prepare(); + api.forwardLogsToBackend(); await yomichan.backendReady(); const japaneseUtil = new JapaneseUtil(wanakana); - const displaySearch = new DisplaySearch(japaneseUtil); + const displaySearch = new DisplaySearch(japaneseUtil, documentFocusController); await displaySearch.prepare(); document.documentElement.dataset.loaded = 'true'; diff --git a/ext/bg/js/search.js b/ext/bg/js/search.js index 884ab33c..fb8757eb 100644 --- a/ext/bg/js/search.js +++ b/ext/bg/js/search.js @@ -24,8 +24,8 @@ */ class DisplaySearch extends Display { - constructor(japaneseUtil) { - super('search', japaneseUtil); + constructor(japaneseUtil, documentFocusController) { + super('search', japaneseUtil, documentFocusController); this._searchButton = document.querySelector('#search-button'); this._queryInput = document.querySelector('#search-textbox'); this._introElement = document.querySelector('#intro'); diff --git a/ext/bg/js/settings2/settings-main.js b/ext/bg/js/settings2/settings-main.js index 6e6186e5..162ed3cd 100644 --- a/ext/bg/js/settings2/settings-main.js +++ b/ext/bg/js/settings2/settings-main.js @@ -23,6 +23,7 @@ * ClipboardPopupsController * DictionaryController * DictionaryImportController + * DocumentFocusController * GenericSettingController * ModalController * NestedPopupsController @@ -53,7 +54,8 @@ async function setupGenericSettingsController(genericSettingController) { (async () => { try { - document.querySelector('#content-scroll-focus').focus(); + const documentFocusController = new DocumentFocusController(); + documentFocusController.prepare(); const statusFooter = new StatusFooter(document.querySelector('.status-footer-container')); statusFooter.prepare(); diff --git a/ext/bg/js/welcome-main.js b/ext/bg/js/welcome-main.js index 8d43fdb0..cbc7e2f8 100644 --- a/ext/bg/js/welcome-main.js +++ b/ext/bg/js/welcome-main.js @@ -18,6 +18,7 @@ /* global * DictionaryController * DictionaryImportController + * DocumentFocusController * GenericSettingController * ModalController * ScanInputsSimpleController @@ -42,7 +43,8 @@ async function setupGenericSettingsController(genericSettingController) { (async () => { try { - document.querySelector('#content-scroll-focus').focus(); + const documentFocusController = new DocumentFocusController(); + documentFocusController.prepare(); const statusFooter = new StatusFooter(document.querySelector('.status-footer-container')); statusFooter.prepare(); diff --git a/ext/bg/legal.html b/ext/bg/legal.html index cda3dcc7..7aca19bc 100644 --- a/ext/bg/legal.html +++ b/ext/bg/legal.html @@ -224,6 +224,8 @@ THE SOFTWARE. + + diff --git a/ext/bg/permissions.html b/ext/bg/permissions.html index a1138aee..596f038a 100644 --- a/ext/bg/permissions.html +++ b/ext/bg/permissions.html @@ -138,6 +138,8 @@ + + diff --git a/ext/bg/search.html b/ext/bg/search.html index bcc02578..f0333ab2 100644 --- a/ext/bg/search.html +++ b/ext/bg/search.html @@ -79,6 +79,7 @@ + diff --git a/ext/bg/settings2.html b/ext/bg/settings2.html index 850b6794..120f4ba3 100644 --- a/ext/bg/settings2.html +++ b/ext/bg/settings2.html @@ -2555,6 +2555,7 @@ + diff --git a/ext/bg/welcome.html b/ext/bg/welcome.html index f3a8c63f..6aebb356 100644 --- a/ext/bg/welcome.html +++ b/ext/bg/welcome.html @@ -334,6 +334,7 @@ + diff --git a/ext/fg/float.html b/ext/fg/float.html index 4dc0696e..2064e8ed 100644 --- a/ext/fg/float.html +++ b/ext/fg/float.html @@ -95,6 +95,7 @@ + diff --git a/ext/fg/js/float-main.js b/ext/fg/js/float-main.js index a8554867..aee736a2 100644 --- a/ext/fg/js/float-main.js +++ b/ext/fg/js/float-main.js @@ -17,17 +17,21 @@ /* global * Display + * DocumentFocusController * JapaneseUtil * api */ (async () => { try { + const documentFocusController = new DocumentFocusController(); + documentFocusController.prepare(); + api.forwardLogsToBackend(); await yomichan.backendReady(); const japaneseUtil = new JapaneseUtil(null); - const display = new Display('popup', japaneseUtil); + const display = new Display('popup', japaneseUtil, documentFocusController); await display.prepare(); display.initializeState(); diff --git a/ext/mixed/css/display.css b/ext/mixed/css/display.css index a6ca2810..f0f39d07 100644 --- a/ext/mixed/css/display.css +++ b/ext/mixed/css/display.css @@ -244,6 +244,24 @@ a { } +/* Selection */ +#content-scroll-focus { + opacity: 0; + margin: 0; + padding: 0; + outline: none; + background-color: transparent; + display: inline; + width: 0; + height: 0; + line-height: 0; + user-select: none; +} +#content-scroll-focus::-moz-focus-inner { + border: 0; +} + + /* Scrollbars */ :root:not([data-theme=default]) .scrollbar { scrollbar-color: var(--scrollbar-thumb-color) var(--scrollbar-track-color); diff --git a/ext/mixed/js/display.js b/ext/mixed/js/display.js index 7491cd60..b9ea3802 100644 --- a/ext/mixed/js/display.js +++ b/ext/mixed/js/display.js @@ -35,10 +35,11 @@ */ class Display extends EventDispatcher { - constructor(pageType, japaneseUtil) { + constructor(pageType, japaneseUtil, documentFocusController) { super(); this._pageType = pageType; this._japaneseUtil = japaneseUtil; + this._documentFocusController = documentFocusController; this._container = document.querySelector('#definitions'); this._definitions = []; this._optionsContext = {depth: 0, url: window.location.href}; @@ -95,7 +96,6 @@ class Display extends EventDispatcher { this._updateAdderButtonsPromise = Promise.resolve(); this._contentScrollElement = document.querySelector('#content-scroll'); this._contentScrollBodyElement = document.querySelector('#content-body'); - this._contentScrollFocusElement = document.querySelector('#content-scroll-focus'); this._windowScroll = new WindowScroll(this._contentScrollElement); this._contentSidebar = document.querySelector('#content-sidebar'); this._closeButton = document.querySelector('#close-button'); @@ -218,7 +218,6 @@ class Display extends EventDispatcher { ['popupMessage', {async: 'dynamic', handler: this._onDirectMessage.bind(this)}] ]); window.addEventListener('message', this._onWindowMessage.bind(this), false); - window.addEventListener('focus', this._onWindowFocus.bind(this), false); if (this._pageType === 'popup' && documentElement !== null) { documentElement.addEventListener('mouseup', this._onDocumentElementMouseUp.bind(this), false); @@ -241,9 +240,6 @@ class Display extends EventDispatcher { if (this._frameResizeHandle !== null) { this._frameResizeHandle.addEventListener('mousedown', this._onFrameResizerMouseDown.bind(this), false); } - - // Final preparation - this._updateFocusedElement(); } initializeState() { @@ -457,8 +453,7 @@ class Display extends EventDispatcher { } blurElement(element) { - element.blur(); - this._updateFocusedElement(); + this._documentFocusController.blurElement(element); } searchLast() { @@ -711,10 +706,6 @@ class Display extends EventDispatcher { } } - _onWindowFocus() { - this._updateFocusedElement(); - } - async _onKanjiLookup(e) { try { e.preventDefault(); @@ -1633,19 +1624,6 @@ class Display extends EventDispatcher { await this.setOptionsContext(optionsContext); } - _updateFocusedElement() { - const target = this._contentScrollFocusElement; - if (target === null) { return; } - const {activeElement} = document; - if ( - activeElement === null || - activeElement === document.documentElement || - activeElement === document.body - ) { - target.focus({preventScroll: true}); - } - } - _setContentScale(scale) { const body = document.body; if (body === null) { return; } diff --git a/ext/mixed/js/document-focus-controller.js b/ext/mixed/js/document-focus-controller.js new file mode 100644 index 00000000..985e9f5a --- /dev/null +++ b/ext/mixed/js/document-focus-controller.js @@ -0,0 +1,110 @@ +/* + * Copyright (C) 2020 Yomichan Authors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +/** + * This class is used to control the document focus when a non-body element contains the main scrollbar. + * Web browsers will not automatically focus a custom element with the scrollbar on load, which results in + * keyboard shortcuts (e.g. arrow keys) not controlling page scroll. Instead, this class will manually + * focus a dummy element inside the main content, which gives keyboard scroll focus to that element. + */ +class DocumentFocusController { + constructor() { + this._contentScrollFocusElement = document.querySelector('#content-scroll-focus'); + } + + prepare() { + window.addEventListener('focus', this._onWindowFocus.bind(this), false); + this._updateFocusedElement(false); + } + + blurElement(element) { + if (document.activeElement !== element) { return; } + element.blur(); + this._updateFocusedElement(false); + } + + // Private + + _onWindowFocus() { + this._updateFocusedElement(false); + } + + _updateFocusedElement(force) { + const target = this._contentScrollFocusElement; + if (target === null) { return; } + + const {activeElement} = document; + if ( + force || + activeElement === null || + activeElement === document.documentElement || + activeElement === document.body + ) { + // Get selection + const selection = window.getSelection(); + const selectionRanges1 = this._getSelectionRanges(selection); + + // Note: This function will cause any selected text to be deselected on Firefox. + target.focus({preventScroll: true}); + + // Restore selection + const selectionRanges2 = this._getSelectionRanges(selection); + if (!this._areRangesSame(selectionRanges1, selectionRanges2)) { + this._setSelectionRanges(selection, selectionRanges1); + } + } + } + + _getSelectionRanges(selection) { + const ranges = []; + for (let i = 0, ii = selection.rangeCount; i < ii; ++i) { + ranges.push(selection.getRangeAt(i)); + } + return ranges; + } + + _setSelectionRanges(selection, ranges) { + selection.removeAllRanges(); + for (const range of ranges) { + selection.addRange(range); + } + } + + _areRangesSame(ranges1, ranges2) { + const ii = ranges1.length; + if (ii !== ranges2.length) { + return false; + } + + for (let i = 0; i < ii; ++i) { + const range1 = ranges1[i]; + const range2 = ranges2[i]; + try { + if ( + range1.compareBoundaryPoints(Range.START_TO_START, range2) !== 0 || + range1.compareBoundaryPoints(Range.END_TO_END, range2) !== 0 + ) { + return false; + } + } catch (e) { + return false; + } + } + + return true; + } +}