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

@@ -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.setOptionsContext(optionsContext);
-
-        await this.updateOptions();
 
         if (childrenSupported && !this._nestedPopupsPrepared) {
             const {depth} = optionsContext;

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};
-        return {depth, url, modifierKeys};
     }
 }

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;
+        const {optionsContext, elementRect, writingMode} = details;
-        if (typeof source !== 'undefined' && source !== this._previousOptionsContextSource) {
+        if (optionsContext !== null) {
-            await this.setOptionsContext(optionsContext, source);
+            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);
+    }
 }

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) {
+    async _findDefinitions(isTerms, source, urlSearchParams, optionsContext) {
-        const optionsContext = this.getOptionsContext();
         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);
+    }
 }

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);
     }