From f2ad94e54f2a110bf93aebfae33c808c497005be Mon Sep 17 00:00:00 2001 From: toasted-nutbread Date: Thu, 12 Nov 2020 20:32:46 -0500 Subject: [PATCH] Text scanning options propagation (#1020) * Update Display.setOptionsContext to update options * Update how options context is updated in Popup * Omit optionsContext for some _showPopupContent calls * Remove extension unload * Disable modifier keys in frontend's options context * Update how text scanner passes modifiers to options context * Update how options context is passed to display * Update how display uses options context --- ext/fg/js/float.js | 9 +------ ext/fg/js/frontend.js | 46 ++++++------------------------------ ext/fg/js/popup.js | 18 +++++++------- ext/mixed/js/display.js | 38 +++++++++++++++++++---------- ext/mixed/js/text-scanner.js | 11 +++++++-- 5 files changed, 52 insertions(+), 70 deletions(-) diff --git a/ext/fg/js/float.js b/ext/fg/js/float.js index 7a6cae64..3b752df5 100644 --- a/ext/fg/js/float.js +++ b/ext/fg/js/float.js @@ -61,11 +61,6 @@ class DisplayFloat extends Display { this._invokeOwner('closePopup'); } - async setOptionsContext(optionsContext) { - super.setOptionsContext(optionsContext); - await this.updateOptions(); - } - async getDocumentTitle() { try { const targetFrameId = 0; @@ -99,9 +94,7 @@ class DisplayFloat extends Display { async _onMessageConfigure({frameId, ownerFrameId, popupId, optionsContext, childrenSupported, scale}) { this.ownerFrameId = ownerFrameId; - this.setOptionsContext(optionsContext); - - await this.updateOptions(); + await this.setOptionsContext(optionsContext); if (childrenSupported && !this._nestedPopupsPrepared) { const {depth} = optionsContext; diff --git a/ext/fg/js/frontend.js b/ext/fg/js/frontend.js index 53b19d89..82a56f1e 100644 --- a/ext/fg/js/frontend.js +++ b/ext/fg/js/frontend.js @@ -33,21 +33,18 @@ class Frontend { pageType, allowRootFramePopupProxy }) { - this._id = generateId(16); this._popup = null; this._disabledOverride = false; this._options = null; this._pageZoomFactor = 1.0; this._contentScale = 1.0; this._lastShowPromise = Promise.resolve(); - this._activeModifiers = new Set(); - this._optionsUpdatePending = false; this._documentUtil = new DocumentUtil(); this._textScanner = new TextScanner({ node: window, ignoreElements: this._ignoreElements.bind(this), ignorePoint: this._ignorePoint.bind(this), - getOptionsContext: this._getUpToDateOptionsContext.bind(this), + getOptionsContext: this._getOptionsContext.bind(this), documentUtil: this._documentUtil, searchTerms: true, searchKanji: true @@ -112,7 +109,6 @@ class Frontend { chrome.runtime.onMessage.addListener(this._onRuntimeMessage.bind(this)); this._textScanner.on('clearSelection', this._onClearSelection.bind(this)); - this._textScanner.on('activeModifiersChanged', this._onActiveModifiersChanged.bind(this)); this._textScanner.on('searched', this._onSearched.bind(this)); api.crossFrame.registerHandlers([ @@ -148,7 +144,6 @@ class Frontend { if (!yomichan.isExtensionUnloaded) { throw e; } - this._showExtensionUnloaded(null); } } @@ -236,18 +231,6 @@ class Frontend { this._popup.clearAutoPlayTimer(); this._isPointerOverPopup = false; } - this._updatePendingOptions(); - } - - async _onActiveModifiersChanged({modifiers}) { - modifiers = new Set(modifiers); - if (areSetsEqual(modifiers, this._activeModifiers)) { return; } - this._activeModifiers = modifiers; - if (this._popup !== null && await this._popup.isVisible()) { - this._optionsUpdatePending = true; - return; - } - await this.updateOptions(); } _onSearched({type, definitions, sentence, inputInfo: {cause, empty}, textSource, optionsContext, error}) { @@ -386,7 +369,7 @@ class Frontend { const optionsContext = await this._getOptionsContext(); if (this._updatePopupToken !== token) { return; } if (popup !== null) { - await popup.setOptionsContext(optionsContext, this._id); + await popup.setOptionsContext(optionsContext); } if (this._updatePopupToken !== token) { return; } @@ -481,16 +464,15 @@ class Frontend { } } - async _showExtensionUnloaded(textSource) { + _showExtensionUnloaded(textSource) { if (textSource === null) { textSource = this._textScanner.getCurrentTextSource(); if (textSource === null) { return; } } - this._showPopupContent(textSource, await this._getOptionsContext()); + this._showPopupContent(textSource, null); } _showContent(textSource, focus, definitions, type, sentence, optionsContext) { - const {url} = optionsContext; const query = textSource.text(); const details = { focus, @@ -503,7 +485,7 @@ class Frontend { state: { focusEntry: 0, sentence, - url + optionsContext }, content: { definitions @@ -521,7 +503,6 @@ class Frontend { this._popup !== null ? this._popup.showContent( { - source: this._id, optionsContext, elementRect: textSource.getRect(), writingMode: textSource.getWritingMode() @@ -537,13 +518,6 @@ class Frontend { return this._lastShowPromise; } - async _updatePendingOptions() { - if (this._optionsUpdatePending) { - this._optionsUpdatePending = false; - await this.updateOptions(); - } - } - _updateTextScannerEnabled() { const enabled = ( this._options.general.enable && @@ -580,7 +554,7 @@ class Frontend { this._popup !== null && await this._popup.isVisible() ) { - this._showPopupContent(textSource, await this._getOptionsContext()); + this._showPopupContent(textSource, null); } } @@ -610,11 +584,6 @@ class Frontend { await promise; } - async _getUpToDateOptionsContext() { - await this._updatePendingOptions(); - return await this._getOptionsContext(); - } - _getPreventMiddleMouseValueForPageType(preventMiddleMouseOptions) { switch (this._pageType) { case 'web': return preventMiddleMouseOptions.onWebPages; @@ -639,7 +608,6 @@ class Frontend { } const depth = this._depth; - const modifierKeys = [...this._activeModifiers]; - return {depth, url, modifierKeys}; + return {depth, url}; } } diff --git a/ext/fg/js/popup.js b/ext/fg/js/popup.js index ee3bf646..df177289 100644 --- a/ext/fg/js/popup.js +++ b/ext/fg/js/popup.js @@ -39,7 +39,6 @@ class Popup extends EventDispatcher { this._optionsContext = null; this._contentScale = 1.0; this._targetOrigin = chrome.runtime.getURL('/').replace(/\/$/, ''); - this._previousOptionsContextSource = null; this._frameSizeContentScale = null; this._frameClient = null; @@ -105,14 +104,10 @@ class Popup extends EventDispatcher { this._onVisibleChange({value: this.isVisibleSync()}); } - async setOptionsContext(optionsContext, source) { + async setOptionsContext(optionsContext) { this._optionsContext = optionsContext; - this._previousOptionsContextSource = source; - this._options = await api.optionsGet(optionsContext); this.updateTheme(); - - this._invokeSafe('setOptionsContext', {optionsContext}); } hide(changeFocus) { @@ -154,9 +149,9 @@ class Popup extends EventDispatcher { async showContent(details, displayDetails) { if (this._options === null) { throw new Error('Options not assigned'); } - const {source, optionsContext, elementRect, writingMode} = details; - if (typeof source !== 'undefined' && source !== this._previousOptionsContextSource) { - await this.setOptionsContext(optionsContext, source); + const {optionsContext, elementRect, writingMode} = details; + if (optionsContext !== null) { + await this._setOptionsContextIfDifferent(optionsContext); } if (typeof elementRect !== 'undefined' && typeof writingMode !== 'undefined') { @@ -659,4 +654,9 @@ class Popup extends EventDispatcher { bottom: window.innerHeight }; } + + async _setOptionsContextIfDifferent(optionsContext) { + if (deepEqual(this._optionsContext, optionsContext)) { return; } + await this.setOptionsContext(optionsContext); + } } diff --git a/ext/mixed/js/display.js b/ext/mixed/js/display.js index d061859e..0d3ee69d 100644 --- a/ext/mixed/js/display.js +++ b/ext/mixed/js/display.js @@ -227,8 +227,9 @@ class Display extends EventDispatcher { return this._optionsContext; } - setOptionsContext(optionsContext) { + async setOptionsContext(optionsContext) { this._optionsContext = optionsContext; + await this.updateOptions(); } async updateOptions() { @@ -507,7 +508,7 @@ class Display extends EventDispatcher { } } - _onQueryParserSearch({type, definitions, sentence, inputInfo: {cause}, textSource}) { + _onQueryParserSearch({type, definitions, sentence, inputInfo: {cause}, textSource, optionsContext}) { const query = textSource.text(); const history = (cause === 'click'); const details = { @@ -516,7 +517,7 @@ class Display extends EventDispatcher { params: this._createSearchParams(type, query, false), state: { sentence, - url: window.location.href + optionsContext }, content: { definitions @@ -570,7 +571,7 @@ class Display extends EventDispatcher { state: { focusEntry: 0, sentence: state.sentence, - url: state.url + optionsContext: state.optionsContext }, content: { definitions @@ -629,7 +630,7 @@ class Display extends EventDispatcher { state: { focusEntry: 0, sentence, - url: state.url + optionsContext: state.optionsContext }, content: { definitions @@ -779,8 +780,7 @@ class Display extends EventDispatcher { } } - async _findDefinitions(isTerms, source, urlSearchParams) { - const optionsContext = this.getOptionsContext(); + async _findDefinitions(isTerms, source, urlSearchParams, optionsContext) { if (isTerms) { const findDetails = {}; if (urlSearchParams.get('wildcards') !== 'off') { @@ -821,6 +821,16 @@ class Display extends EventDispatcher { changeHistory = true; } + let {sentence=null, optionsContext=null, focusEntry=null, scrollX=null, scrollY=null} = state; + if (!(typeof optionsContext === 'object' && optionsContext !== null)) { + optionsContext = this.getOptionsContext(); + state.optionsContext = optionsContext; + changeHistory = true; + } + let {url} = optionsContext; + if (typeof url !== 'string') { url = window.location.href; } + sentence = this._getValidSentenceData(sentence); + source = this.postProcessQuery(source); let full = urlSearchParams.get('full'); full = (full === null ? source : this.postProcessQuery(full)); @@ -829,12 +839,15 @@ class Display extends EventDispatcher { let {definitions} = content; if (!Array.isArray(definitions)) { - definitions = await this._findDefinitions(isTerms, source, urlSearchParams); + definitions = await this._findDefinitions(isTerms, source, urlSearchParams, optionsContext); if (this._setContentToken !== token) { return true; } content.definitions = definitions; changeHistory = true; } + await this._setOptionsContextIfDifferent(optionsContext); + if (this._setContentToken !== token) { return true; } + if (changeHistory) { this._historyStateUpdate(state, content); } @@ -843,10 +856,6 @@ class Display extends EventDispatcher { eventArgs.content = content; this.trigger('contentUpdating', eventArgs); - let {sentence=null, url=null, focusEntry=null, scrollX=null, scrollY=null} = state; - if (typeof url !== 'string') { url = window.location.href; } - sentence = this._getValidSentenceData(sentence); - this._definitions = definitions; for (const definition of definitions) { @@ -1486,4 +1495,9 @@ class Display extends EventDispatcher { } return true; } + + async _setOptionsContextIfDifferent(optionsContext) { + if (deepEqual(this._optionsContext, optionsContext)) { return; } + await this.setOptionsContext(optionsContext); + } } diff --git a/ext/mixed/js/text-scanner.js b/ext/mixed/js/text-scanner.js index 5ab90de8..66d37c93 100644 --- a/ext/mixed/js/text-scanner.js +++ b/ext/mixed/js/text-scanner.js @@ -238,6 +238,14 @@ class TextScanner extends EventDispatcher { // Private + async _getOptionsContextForInput(inputInfo) { + const optionsContext = clone(await this._getOptionsContext()); + const {modifiers, modifierKeys} = inputInfo; + optionsContext.modifiers = [...modifiers]; + optionsContext.modifierKeys = [...modifierKeys]; + return optionsContext; + } + async _search(textSource, searchTerms, searchKanji, inputInfo) { let definitions = null; let sentence = null; @@ -251,7 +259,7 @@ class TextScanner extends EventDispatcher { return; } - optionsContext = await this._getOptionsContext(); + optionsContext = await this._getOptionsContextForInput(inputInfo); searched = true; const result = await this._findDefinitions(textSource, searchTerms, searchKanji, optionsContext); @@ -810,7 +818,6 @@ class TextScanner extends EventDispatcher { _getMatchingInputGroupFromEvent(type, cause, event) { const modifiers = DocumentUtil.getActiveModifiersAndButtons(event); const modifierKeys = DocumentUtil.getActiveModifiers(event); - this.trigger('activeModifiersChanged', {modifiers, modifierKeys}); return this._getMatchingInputGroup(type, cause, modifiers, modifierKeys); }