From de299c64ae0d32f316d1679b79dd1ad72e1c0ed9 Mon Sep 17 00:00:00 2001 From: toasted-nutbread Date: Tue, 24 Nov 2020 11:54:08 -0500 Subject: [PATCH] Display updates (#1057) * Fix history assignment on the search page * Use clear instead of assigned * Simplify definitions assignment * Organize * Fix query not being cleared * Fix media loading * Fix potential issue with options not being assigned * Catch error when frameId is null, causing infinite loop * Fix frontend construction parameters --- ext/bg/js/search.js | 14 ++--- ext/fg/js/popup-factory.js | 5 +- ext/mixed/js/display.js | 101 +++++++++++++++++++---------------- ext/mixed/js/media-loader.js | 4 +- 4 files changed, 67 insertions(+), 57 deletions(-) diff --git a/ext/bg/js/search.js b/ext/bg/js/search.js index 476370bf..effa31bc 100644 --- a/ext/bg/js/search.js +++ b/ext/bg/js/search.js @@ -143,7 +143,7 @@ class DisplaySearch extends Display { await this.updateOptions(); const query = this._queryInput.value; if (query) { - this._search(false); + this._search(false, false); } } @@ -153,7 +153,7 @@ class DisplaySearch extends Display { switch (type) { case 'terms': case 'kanji': - animate = content.animate; + animate = !!content.animate; valid = content.definitions.length > 0; this.blurElement(this._queryInput); break; @@ -182,12 +182,12 @@ class DisplaySearch extends Display { e.preventDefault(); e.stopImmediatePropagation(); this.blurElement(e.currentTarget); - this._search(true); + this._search(true, true); } _onSearch(e) { e.preventDefault(); - this._search(true); + this._search(true, true); } _onCopy() { @@ -197,7 +197,7 @@ class DisplaySearch extends Display { _onExternalSearchUpdate({text, animate=true}) { this._queryInput.value = text; - this._search(animate); + this._search(animate, false); } _onWanakanaEnableChange(e) { @@ -342,11 +342,11 @@ class DisplaySearch extends Display { }); } - _search(animate) { + _search(animate, history) { const query = this._queryInput.value; const details = { focus: false, - history: false, + history, params: { query }, diff --git a/ext/fg/js/popup-factory.js b/ext/fg/js/popup-factory.js index d63ed17a..252bcdf4 100644 --- a/ext/fg/js/popup-factory.js +++ b/ext/fg/js/popup-factory.js @@ -114,7 +114,7 @@ class PopupFactory { const popup = new Popup({ id, depth, - frameId, + frameId: this._frameId, ownerFrameId, childrenSupported }); @@ -129,6 +129,9 @@ class PopupFactory { popup.prepare(); return popup; } else { + if (frameId === null) { + throw new Error('Invalid frameId'); + } const useFrameOffsetForwarder = (parentPopupId === null); ({id, depth, frameId} = await api.crossFrame.invoke(frameId, 'getOrCreatePopup', { id, diff --git a/ext/mixed/js/display.js b/ext/mixed/js/display.js index acab9345..2f24d7bd 100644 --- a/ext/mixed/js/display.js +++ b/ext/mixed/js/display.js @@ -522,6 +522,12 @@ class Display extends EventDispatcher { const token = {}; // Unique identifier token this._setContentToken = token; try { + // Clear + this._closePopups(); + this._eventListeners.removeAllEventListeners(); + this._mediaLoader.unloadAll(); + + // Prepare const urlSearchParams = new URLSearchParams(location.search); let type = urlSearchParams.get('type'); if (type === null) { type = 'terms'; } @@ -530,46 +536,51 @@ class Display extends EventDispatcher { this._queryParserVisibleOverride = (fullVisible === null ? null : (fullVisible !== 'false')); this._updateQueryParser(); - this._closePopups(); - this._eventListeners.removeAllEventListeners(); - - let assigned = false; - const eventArgs = {type, urlSearchParams, token}; + let clear = true; this._historyHasChanged = true; this._contentType = type; - this._mediaLoader.unloadAll(); + const eventArgs = {type, urlSearchParams, token}; + + // Set content switch (type) { case 'terms': case 'kanji': { + let query = urlSearchParams.get('query'); + if (!query) { break; } + + clear = false; const isTerms = (type === 'terms'); - assigned = await this._setContentTermsOrKanji(token, isTerms, urlSearchParams, eventArgs); + query = this.postProcessQuery(query); + let queryFull = urlSearchParams.get('full'); + queryFull = (queryFull !== null ? this.postProcessQuery(queryFull) : query); + const wildcardsEnabled = (urlSearchParams.get('wildcards') !== 'off'); + await this._setContentTermsOrKanji(token, isTerms, query, queryFull, wildcardsEnabled, eventArgs); } break; case 'unloaded': { + clear = false; const {content} = this._history; eventArgs.content = content; this.trigger('contentUpdating', eventArgs); this._setContentExtensionUnloaded(); - assigned = true; } break; } - const stale = (this._setContentToken !== token); - if (!stale) { - if (!assigned) { - type = 'clear'; - this._contentType = type; - const {content} = this._history; - eventArgs.type = type; - eventArgs.content = content; - this.trigger('contentUpdating', eventArgs); - this._clearContent(); - } + // Clear + if (clear) { + type = 'clear'; + this._contentType = type; + const {content} = this._history; + eventArgs.type = type; + eventArgs.content = content; + this.trigger('contentUpdating', eventArgs); + this._clearContent(); } + const stale = (this._setContentToken !== token); eventArgs.stale = stale; this.trigger('contentUpdated', eventArgs); } catch (e) { @@ -788,10 +799,10 @@ class Display extends EventDispatcher { document.documentElement.dataset.yomichanTheme = themeName; } - async _findDefinitions(isTerms, source, urlSearchParams, optionsContext) { + async _findDefinitions(isTerms, source, wildcardsEnabled, optionsContext) { if (isTerms) { const findDetails = {}; - if (urlSearchParams.get('wildcards') !== 'off') { + if (wildcardsEnabled) { const match = /^([*\uff0a]*)([\w\W]*?)([*\uff0a]*)$/.exec(source); if (match !== null) { if (match[1]) { @@ -811,13 +822,7 @@ class Display extends EventDispatcher { } } - async _setContentTermsOrKanji(token, isTerms, urlSearchParams, eventArgs) { - let source = urlSearchParams.get('query'); - if (!source) { - this._setFullQuery(''); - return false; - } - + async _setContentTermsOrKanji(token, isTerms, query, queryFull, wildcardsEnabled, eventArgs) { let {state, content} = this._history; let changeHistory = false; if (!isObject(content)) { @@ -839,28 +844,30 @@ class Display extends EventDispatcher { 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)); - this._setFullQuery(full); - this._setTitleText(source); + this._setFullQuery(queryFull); + this._setTitleText(query); let {definitions} = content; if (!Array.isArray(definitions)) { - definitions = await this._findDefinitions(isTerms, source, urlSearchParams, optionsContext); - if (this._setContentToken !== token) { return true; } + definitions = await this._findDefinitions(isTerms, query, wildcardsEnabled, optionsContext); + if (this._setContentToken !== token) { return; } content.definitions = definitions; changeHistory = true; } await this._setOptionsContextIfDifferent(optionsContext); - if (this._setContentToken !== token) { return true; } + if (this._setContentToken !== token) { return; } + + if (this._options === null) { + await this.updateOptions(); + if (this._setContentToken !== token) { return; } + } if (changeHistory) { this._replaceHistoryStateNoNavigate(state, content); } - eventArgs.source = source; + eventArgs.source = query; eventArgs.content = content; this.trigger('contentUpdating', eventArgs); @@ -880,7 +887,7 @@ class Display extends EventDispatcher { for (let i = 0, ii = definitions.length; i < ii; ++i) { if (i > 0) { await promiseTimeout(1); - if (this._setContentToken !== token) { return true; } + if (this._setContentToken !== token) { return; } } const definition = definitions[i]; @@ -912,8 +919,6 @@ class Display extends EventDispatcher { } this._updateAdderButtons(token, isTerms, definitions); - - return true; } _setContentExtensionUnloaded() { @@ -930,11 +935,13 @@ class Display extends EventDispatcher { this._updateNavigation(false, false); this._setNoContentVisible(false); this._setTitleText(''); + this._setFullQuery(''); } _clearContent() { this._container.textContent = ''; this._setTitleText(''); + this._setFullQuery(''); } _setNoContentVisible(visible) { @@ -1566,7 +1573,7 @@ class Display extends EventDispatcher { try { if (this._frontendSetupPromise === null) { - this._frontendSetupPromise = this._setupNestedFrontend(isSearchPage); + this._frontendSetupPromise = this._setupNestedFrontend(); } await this._frontendSetupPromise; } catch (e) { @@ -1580,12 +1587,12 @@ class Display extends EventDispatcher { this._frontend.setDisabledOverride(!isEnabled); } - async _setupNestedFrontend(isSearchPage) { - const setupNestedPopupsOptions = ( - (isSearchPage) ? - {useProxyPopup: false} : - {useProxyPopup: true, parentPopupId: this._parentPopupId, parentFrameId: this._parentFrameId} - ); + async _setupNestedFrontend() { + const setupNestedPopupsOptions = { + useProxyPopup: this._parentFrameId !== null, + parentPopupId: this._parentPopupId, + parentFrameId: this._parentFrameId + }; const {frameId} = await api.frameInformationGet(); diff --git a/ext/mixed/js/media-loader.js b/ext/mixed/js/media-loader.js index fc6e93d1..70d7bad9 100644 --- a/ext/mixed/js/media-loader.js +++ b/ext/mixed/js/media-loader.js @@ -27,13 +27,13 @@ class MediaLoader { } async loadMedia(path, dictionaryName, onLoad, onUnload) { - const token = this.token; + const token = this._token; const data = {onUnload, loaded: false}; this._loadMediaData.push(data); const media = await this.getMedia(path, dictionaryName); - if (token !== this.token) { return; } + if (token !== this._token) { return; } onLoad(media.url); data.loaded = true;