From 01ff7436ee381c4414e95572cff08aac999b7721 Mon Sep 17 00:00:00 2001 From: toasted-nutbread Date: Mon, 23 Nov 2020 15:23:47 -0500 Subject: [PATCH] Popup setup refactoring (#1054) * Pass childrenSupported as a parameter to Frontend/Popup constructors * Remove setChildrenSupported * Use event listener instead of function override * Update options order * Expand options and use object for clarity * Fix childrenSupported not being fully propagated --- ext/bg/js/settings/popup-preview-frame.js | 27 +++++------- ext/fg/js/frontend.js | 36 ++++++++------- ext/fg/js/popup-factory.js | 53 +++++++++++++++++------ ext/fg/js/popup-proxy.js | 12 ++--- ext/fg/js/popup-window.js | 11 ++--- ext/fg/js/popup.js | 17 +++++--- ext/mixed/js/display.js | 3 +- 7 files changed, 95 insertions(+), 64 deletions(-) diff --git a/ext/bg/js/settings/popup-preview-frame.js b/ext/bg/js/settings/popup-preview-frame.js index d9b05368..5a402a71 100644 --- a/ext/bg/js/settings/popup-preview-frame.js +++ b/ext/bg/js/settings/popup-preview-frame.js @@ -29,7 +29,6 @@ class PopupPreviewFrame { this._popupFactory = popupFactory; this._frontend = null; this._apiOptionsGetOld = null; - this._popupSetCustomOuterCssOld = null; this._popupShown = false; this._themeChangeTimeout = null; this._textSource = null; @@ -75,18 +74,14 @@ class PopupPreviewFrame { parentFrameId: null, useProxyPopup: false, pageType: 'web', - allowRootFramePopupProxy: false + allowRootFramePopupProxy: false, + childrenSupported: false }); this._frontend.setOptionsContextOverride(this._optionsContext); await this._frontend.prepare(); this._frontend.setDisabledOverride(true); this._frontend.canClearSelection = false; - - const popup = this._frontend.popup; - popup.setChildrenSupported(false); - - this._popupSetCustomOuterCssOld = popup.setCustomOuterCss.bind(popup); - popup.setCustomOuterCss = this._popupSetCustomOuterCss.bind(this); + this._frontend.popup.on('customOuterCssChanged', this._onCustomOuterCssChanged.bind(this)); // Update search this._updateSearch(); @@ -110,16 +105,14 @@ class PopupPreviewFrame { return options; } - async _popupSetCustomOuterCss(...args) { + _onCustomOuterCssChanged({node}) { + if (node === null) { return; } + + const node2 = document.querySelector('#client-css'); + if (node2 === null) { return; } + // This simulates the stylesheet priorities when injecting using the web extension API. - const result = await this._popupSetCustomOuterCssOld(...args); - - const node = document.querySelector('#client-css'); - if (node !== null && result !== null) { - node.parentNode.insertBefore(result, node); - } - - return result; + node2.parentNode.insertBefore(node, node2); } _onMessage(e) { diff --git a/ext/fg/js/frontend.js b/ext/fg/js/frontend.js index e4c6342e..6ae3b06d 100644 --- a/ext/fg/js/frontend.js +++ b/ext/fg/js/frontend.js @@ -24,15 +24,25 @@ class Frontend { constructor({ - frameId, + pageType, popupFactory, depth, + frameId, parentPopupId, parentFrameId, useProxyPopup, - pageType, - allowRootFramePopupProxy + allowRootFramePopupProxy, + childrenSupported=true }) { + this._pageType = pageType; + this._popupFactory = popupFactory; + this._depth = depth; + this._frameId = frameId; + this._parentPopupId = parentPopupId; + this._parentFrameId = parentFrameId; + this._useProxyPopup = useProxyPopup; + this._allowRootFramePopupProxy = allowRootFramePopupProxy; + this._childrenSupported = childrenSupported; this._popup = null; this._disabledOverride = false; this._options = null; @@ -49,14 +59,6 @@ class Frontend { searchTerms: true, searchKanji: true }); - this._parentPopupId = parentPopupId; - this._parentFrameId = parentFrameId; - this._useProxyPopup = useProxyPopup; - this._pageType = pageType; - this._depth = depth; - this._frameId = frameId; - this._popupFactory = popupFactory; - this._allowRootFramePopupProxy = allowRootFramePopupProxy; this._popupCache = new Map(); this._popupEventListeners = new EventListenerCollection(); this._updatePopupToken = null; @@ -398,7 +400,8 @@ class Frontend { return await this._popupFactory.getOrCreatePopup({ frameId: this._frameId, ownerFrameId: this._frameId, - depth: this._depth + depth: this._depth, + childrenSupported: this._childrenSupported }); } @@ -407,7 +410,8 @@ class Frontend { frameId: this._parentFrameId, ownerFrameId: this._frameId, depth: this._depth, - parentPopupId: this._parentPopupId + parentPopupId: this._parentPopupId, + childrenSupported: this._childrenSupported }); } @@ -428,7 +432,8 @@ class Frontend { const popup = await this._popupFactory.getOrCreatePopup({ frameId: targetFrameId, ownerFrameId: this._frameId, - id: popupId + id: popupId, + childrenSupported: this._childrenSupported }); popup.on('offsetNotFound', () => { this._allowRootFramePopupProxy = false; @@ -441,7 +446,8 @@ class Frontend { return await this._popupFactory.getOrCreatePopup({ ownerFrameId: this._frameId, depth: this._depth, - popupWindow: true + popupWindow: true, + childrenSupported: this._childrenSupported }); } diff --git a/ext/fg/js/popup-factory.js b/ext/fg/js/popup-factory.js index 3e817247..d63ed17a 100644 --- a/ext/fg/js/popup-factory.js +++ b/ext/fg/js/popup-factory.js @@ -48,12 +48,19 @@ class PopupFactory { ['clearAutoPlayTimer', {async: false, handler: this._onApiClearAutoPlayTimer.bind(this)}], ['setContentScale', {async: false, handler: this._onApiSetContentScale.bind(this)}], ['updateTheme', {async: false, handler: this._onApiUpdateTheme.bind(this)}], - ['setCustomOuterCss', {async: false, handler: this._onApiSetCustomOuterCss.bind(this)}], - ['setChildrenSupported', {async: false, handler: this._onApiSetChildrenSupported.bind(this)}] + ['setCustomOuterCss', {async: false, handler: this._onApiSetCustomOuterCss.bind(this)}] ]); } - async getOrCreatePopup({frameId=null, ownerFrameId=null, id=null, parentPopupId=null, depth=null, popupWindow=false}) { + async getOrCreatePopup({ + frameId=null, + ownerFrameId=null, + id=null, + parentPopupId=null, + depth=null, + popupWindow=false, + childrenSupported=false + }) { // Find by existing id if (id !== null) { const popup = this._popups.get(id); @@ -91,7 +98,12 @@ class PopupFactory { if (id === null) { id = generateId(16); } - const popup = new PopupWindow(id, depth, this._frameId, ownerFrameId); + const popup = new PopupWindow({ + id, + depth, + frameId: this._frameId, + ownerFrameId + }); this._popups.set(id, popup); return popup; } else if (frameId === this._frameId) { @@ -99,7 +111,13 @@ class PopupFactory { if (id === null) { id = generateId(16); } - const popup = new Popup(id, depth, frameId, ownerFrameId); + const popup = new Popup({ + id, + depth, + frameId, + ownerFrameId, + childrenSupported + }); if (parent !== null) { if (parent.child !== null) { throw new Error('Parent popup already has a child'); @@ -112,8 +130,20 @@ class PopupFactory { return popup; } else { const useFrameOffsetForwarder = (parentPopupId === null); - ({id, depth, frameId} = await api.crossFrame.invoke(frameId, 'getOrCreatePopup', {id, parentPopupId, frameId, ownerFrameId})); - const popup = new PopupProxy(id, depth, frameId, ownerFrameId, useFrameOffsetForwarder ? this._frameOffsetForwarder : null); + ({id, depth, frameId} = await api.crossFrame.invoke(frameId, 'getOrCreatePopup', { + id, + parentPopupId, + frameId, + ownerFrameId, + childrenSupported + })); + const popup = new PopupProxy({ + id, + depth, + frameId, + ownerFrameId, + frameOffsetForwarder: useFrameOffsetForwarder ? this._frameOffsetForwarder : null + }); this._popups.set(id, popup); return popup; } @@ -155,8 +185,8 @@ class PopupFactory { // API message handlers - async _onApiGetOrCreatePopup({id, parentPopupId, frameId, ownerFrameId}) { - const popup = await this.getOrCreatePopup({id, parentPopupId, frameId, ownerFrameId}); + async _onApiGetOrCreatePopup(details) { + const popup = await this.getOrCreatePopup(details); return { id: popup.id, depth: popup.depth, @@ -232,11 +262,6 @@ class PopupFactory { return popup.setCustomOuterCss(css, useWebExtensionApi); } - _onApiSetChildrenSupported({id, value}) { - const popup = this._getPopup(id); - return popup.setChildrenSupported(value); - } - // Private functions _getPopup(id) { diff --git a/ext/fg/js/popup-proxy.js b/ext/fg/js/popup-proxy.js index 26659672..26dce0d1 100644 --- a/ext/fg/js/popup-proxy.js +++ b/ext/fg/js/popup-proxy.js @@ -20,7 +20,13 @@ */ class PopupProxy extends EventDispatcher { - constructor(id, depth, frameId, ownerFrameId, frameOffsetForwarder=null) { + constructor({ + id, + depth, + frameId, + ownerFrameId, + frameOffsetForwarder + }) { super(); this._id = id; this._depth = depth; @@ -139,10 +145,6 @@ class PopupProxy extends EventDispatcher { return this._invokeSafe('setCustomOuterCss', {id: this._id, css, useWebExtensionApi}); } - setChildrenSupported(value) { - return this._invokeSafe('setChildrenSupported', {id: this._id, value}); - } - getFrameRect() { return new DOMRect(0, 0, 0, 0); } diff --git a/ext/fg/js/popup-window.js b/ext/fg/js/popup-window.js index 1365211c..3d707a9e 100644 --- a/ext/fg/js/popup-window.js +++ b/ext/fg/js/popup-window.js @@ -20,7 +20,12 @@ */ class PopupWindow extends EventDispatcher { - constructor(id, depth, frameId, ownerFrameId) { + constructor({ + id, + depth, + frameId, + ownerFrameId + }) { super(); this._id = id; this._depth = depth; @@ -123,10 +128,6 @@ class PopupWindow extends EventDispatcher { // NOP } - async setChildrenSupported(_value) { - // NOP - } - getFrameRect() { return new DOMRect(0, 0, 0, 0); } diff --git a/ext/fg/js/popup.js b/ext/fg/js/popup.js index 2feb220d..00afb773 100644 --- a/ext/fg/js/popup.js +++ b/ext/fg/js/popup.js @@ -23,15 +23,21 @@ */ class Popup extends EventDispatcher { - constructor(id, depth, frameId, ownerFrameId) { + constructor({ + id, + depth, + frameId, + ownerFrameId, + childrenSupported + }) { super(); this._id = id; this._depth = depth; this._frameId = frameId; this._ownerFrameId = ownerFrameId; + this._childrenSupported = childrenSupported; this._parent = null; this._child = null; - this._childrenSupported = true; this._injectPromise = null; this._injectPromiseComplete = false; this._visible = new DynamicProperty(false); @@ -190,11 +196,8 @@ class Popup extends EventDispatcher { useWebExtensionApi = false; parentNode = this._shadow; } - return await dynamicLoader.loadStyle('yomichan-popup-outer-user-stylesheet', 'code', css, useWebExtensionApi, parentNode); - } - - setChildrenSupported(value) { - this._childrenSupported = value; + const node = await dynamicLoader.loadStyle('yomichan-popup-outer-user-stylesheet', 'code', css, useWebExtensionApi, parentNode); + this.trigger('customOuterCssChanged', {node, useWebExtensionApi}); } getFrameRect() { diff --git a/ext/mixed/js/display.js b/ext/mixed/js/display.js index 8b755b83..0bbdf730 100644 --- a/ext/mixed/js/display.js +++ b/ext/mixed/js/display.js @@ -1704,7 +1704,8 @@ class Display extends EventDispatcher { frameId, popupFactory, pageType: this._pageType, - allowRootFramePopupProxy: true + allowRootFramePopupProxy: true, + childrenSupported: this._childrenSupported }); const frontend = new Frontend(setupNestedPopupsOptions);