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

@@ -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;
-
+        this._frontend.popup.on('customOuterCssChanged', this._onCustomOuterCssChanged.bind(this));
-        const popup = this._frontend.popup;
-        popup.setChildrenSupported(false);
-
-        this._popupSetCustomOuterCssOld = popup.setCustomOuterCss.bind(popup);
-        popup.setCustomOuterCss = this._popupSetCustomOuterCss.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);
+        node2.parentNode.insertBefore(node, node2);
-
-        const node = document.querySelector('#client-css');
-        if (node !== null && result !== null) {
-            node.parentNode.insertBefore(result, node);
-        }
-
-        return result;
     }
 
     _onMessage(e) {

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

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)}],
+            ['setCustomOuterCss',    {async: false, handler: this._onApiSetCustomOuterCss.bind(this)}]
-            ['setChildrenSupported', {async: false, handler: this._onApiSetChildrenSupported.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}));
+            ({id, depth, frameId} = await api.crossFrame.invoke(frameId, 'getOrCreatePopup', {
-            const popup = new PopupProxy(id, depth, frameId, ownerFrameId, useFrameOffsetForwarder ? this._frameOffsetForwarder : null);
+                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}) {
+    async _onApiGetOrCreatePopup(details) {
-        const popup = await this.getOrCreatePopup({id, parentPopupId, frameId, ownerFrameId});
+        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) {

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

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

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);
+        const node = await dynamicLoader.loadStyle('yomichan-popup-outer-user-stylesheet', 'code', css, useWebExtensionApi, parentNode);
-    }
+        this.trigger('customOuterCssChanged', {node, useWebExtensionApi});
-
-    setChildrenSupported(value) {
-        this._childrenSupported = value;
     }
 
     getFrameRect() {

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