From 166451b8f76224542b49c13cb27a258eb291f05e Mon Sep 17 00:00:00 2001 From: toasted-nutbread Date: Tue, 9 Feb 2021 22:56:04 -0500 Subject: [PATCH] Improve popup window ownership (#1364) * Update frameInformationGet to also return the tab ID * Add tabId to Frontend * Pass tabId/frameId to Display * Pass ownership information using setContent * Remove ownerFrameId for Popup classes * Use frameId instead of ownerFrameId for screenshotting * Use contentOrigin instead of owner * Update _invokeContentOrigin implementation --- ext/bg/js/backend.js | 16 ++-- ext/bg/js/search-main.js | 4 +- ext/bg/js/search.js | 10 +- .../js/settings/popup-preview-frame-main.js | 4 +- ext/bg/js/settings/popup-preview-frame.js | 4 +- ext/fg/js/content-script-main.js | 3 +- ext/fg/js/float-main.js | 4 +- ext/fg/js/frontend.js | 12 ++- ext/fg/js/popup-factory.js | 7 +- ext/fg/js/popup-proxy.js | 2 - ext/fg/js/popup-window.js | 4 +- ext/fg/js/popup.js | 3 - ext/mixed/js/display.js | 96 +++++++++++++------ 13 files changed, 106 insertions(+), 63 deletions(-) diff --git a/ext/bg/js/backend.js b/ext/bg/js/backend.js index f1983cb3..fd90a220 100644 --- a/ext/bg/js/backend.js +++ b/ext/bg/js/backend.js @@ -525,8 +525,10 @@ class Backend { } _onApiFrameInformationGet(params, sender) { + const tab = sender.tab; + const tabId = tab ? tab.id : void 0; const frameId = sender.frameId; - return Promise.resolve({frameId}); + return Promise.resolve({tabId, frameId}); } _onApiInjectStylesheet({type, value}, sender) { @@ -1505,17 +1507,17 @@ class Backend { return isValidTab ? tab : null; } - async _getScreenshot(windowId, tabId, ownerFrameId, format, quality) { + async _getScreenshot(windowId, tabId, frameId, format, quality) { if (typeof windowId !== 'number') { throw new Error('Invalid window ID'); } let token = null; try { - if (typeof tabId === 'number' && typeof ownerFrameId === 'number') { + if (typeof tabId === 'number' && typeof frameId === 'number') { const action = 'setAllVisibleOverride'; const params = {value: false, priority: 0, awaitFrame: true}; - token = await this._sendMessageTabPromise(tabId, {action, params}, {frameId: ownerFrameId}); + token = await this._sendMessageTabPromise(tabId, {action, params}, {frameId}); } return await new Promise((resolve, reject) => { @@ -1533,7 +1535,7 @@ class Backend { const action = 'clearAllVisibleOverride'; const params = {token}; try { - await this._sendMessageTabPromise(tabId, {action, params}, {frameId: ownerFrameId}); + await this._sendMessageTabPromise(tabId, {action, params}, {frameId}); } catch (e) { // NOP } @@ -1634,8 +1636,8 @@ class Backend { } async _injectAnkNoteScreenshot(ankiConnect, timestamp, definitionDetails, details) { - const {windowId, tabId, ownerFrameId, format, quality} = details; - const dataUrl = await this._getScreenshot(windowId, tabId, ownerFrameId, format, quality); + const {windowId, tabId, frameId, format, quality} = details; + const dataUrl = await this._getScreenshot(windowId, tabId, frameId, format, quality); const {mediaType, data} = this._getDataUrlInfo(dataUrl); const extension = this._mediaUtility.getFileExtensionFromImageMediaType(mediaType); diff --git a/ext/bg/js/search-main.js b/ext/bg/js/search-main.js index 38c6f4dd..f6abdfc8 100644 --- a/ext/bg/js/search-main.js +++ b/ext/bg/js/search-main.js @@ -32,12 +32,14 @@ api.forwardLogsToBackend(); await yomichan.backendReady(); + const {tabId, frameId} = await api.frameInformationGet(); + const japaneseUtil = new JapaneseUtil(wanakana); const hotkeyHandler = new HotkeyHandler(); hotkeyHandler.prepare(); - const displaySearch = new DisplaySearch(japaneseUtil, documentFocusController, hotkeyHandler); + const displaySearch = new DisplaySearch(tabId, frameId, japaneseUtil, documentFocusController, hotkeyHandler); await displaySearch.prepare(); document.documentElement.dataset.loaded = 'true'; diff --git a/ext/bg/js/search.js b/ext/bg/js/search.js index 27cba50f..dbc679b1 100644 --- a/ext/bg/js/search.js +++ b/ext/bg/js/search.js @@ -23,8 +23,8 @@ */ class DisplaySearch extends Display { - constructor(japaneseUtil, documentFocusController, hotkeyHandler) { - super('search', japaneseUtil, documentFocusController, hotkeyHandler); + constructor(tabId, frameId, japaneseUtil, documentFocusController, hotkeyHandler) { + super('search', tabId, frameId, japaneseUtil, documentFocusController, hotkeyHandler); this._searchButton = document.querySelector('#search-button'); this._queryInput = document.querySelector('#search-textbox'); this._introElement = document.querySelector('#intro'); @@ -353,7 +353,11 @@ class DisplaySearch extends Display { }, content: { definitions: null, - animate + animate, + contentOrigin: { + tabId: this.tabId, + frameId: this.frameId + } } }; if (!lookup) { details.params.lookup = 'false'; } diff --git a/ext/bg/js/settings/popup-preview-frame-main.js b/ext/bg/js/settings/popup-preview-frame-main.js index 71454017..dcbc0d96 100644 --- a/ext/bg/js/settings/popup-preview-frame-main.js +++ b/ext/bg/js/settings/popup-preview-frame-main.js @@ -26,7 +26,7 @@ try { api.forwardLogsToBackend(); - const {frameId} = await api.frameInformationGet(); + const {tabId, frameId} = await api.frameInformationGet(); const hotkeyHandler = new HotkeyHandler(); hotkeyHandler.prepare(); @@ -34,7 +34,7 @@ const popupFactory = new PopupFactory(frameId); popupFactory.prepare(); - const preview = new PopupPreviewFrame(frameId, popupFactory, hotkeyHandler); + const preview = new PopupPreviewFrame(tabId, frameId, popupFactory, hotkeyHandler); await preview.prepare(); document.documentElement.dataset.loaded = 'true'; diff --git a/ext/bg/js/settings/popup-preview-frame.js b/ext/bg/js/settings/popup-preview-frame.js index 73ac6caf..56100fb3 100644 --- a/ext/bg/js/settings/popup-preview-frame.js +++ b/ext/bg/js/settings/popup-preview-frame.js @@ -23,7 +23,8 @@ */ class PopupPreviewFrame { - constructor(frameId, popupFactory, hotkeyHandler) { + constructor(tabId, frameId, popupFactory, hotkeyHandler) { + this._tabId = tabId; this._frameId = frameId; this._popupFactory = popupFactory; this._hotkeyHandler = hotkeyHandler; @@ -67,6 +68,7 @@ class PopupPreviewFrame { // Overwrite frontend this._frontend = new Frontend({ + tabId: this._tabId, frameId: this._frameId, popupFactory: this._popupFactory, depth: 0, diff --git a/ext/fg/js/content-script-main.js b/ext/fg/js/content-script-main.js index 42f95e24..5dee4c56 100644 --- a/ext/fg/js/content-script-main.js +++ b/ext/fg/js/content-script-main.js @@ -27,7 +27,7 @@ api.forwardLogsToBackend(); await yomichan.backendReady(); - const {frameId} = await api.frameInformationGet(); + const {tabId, frameId} = await api.frameInformationGet(); if (typeof frameId !== 'number') { throw new Error('Failed to get frameId'); } @@ -39,6 +39,7 @@ popupFactory.prepare(); const frontend = new Frontend({ + tabId, frameId, popupFactory, depth: 0, diff --git a/ext/fg/js/float-main.js b/ext/fg/js/float-main.js index 2b81a08a..efc012d7 100644 --- a/ext/fg/js/float-main.js +++ b/ext/fg/js/float-main.js @@ -32,12 +32,14 @@ api.forwardLogsToBackend(); await yomichan.backendReady(); + const {tabId, frameId} = await api.frameInformationGet(); + const japaneseUtil = new JapaneseUtil(null); const hotkeyHandler = new HotkeyHandler(); hotkeyHandler.prepare(); - const display = new Display('popup', japaneseUtil, documentFocusController, hotkeyHandler); + const display = new Display(tabId, frameId, 'popup', japaneseUtil, documentFocusController, hotkeyHandler); await display.prepare(); const displayProfileSelection = new DisplayProfileSelection(display); displayProfileSelection.prepare(); diff --git a/ext/fg/js/frontend.js b/ext/fg/js/frontend.js index f02d5609..a62b06bf 100644 --- a/ext/fg/js/frontend.js +++ b/ext/fg/js/frontend.js @@ -28,6 +28,7 @@ class Frontend { pageType, popupFactory, depth, + tabId, frameId, parentPopupId, parentFrameId, @@ -40,6 +41,7 @@ class Frontend { this._pageType = pageType; this._popupFactory = popupFactory; this._depth = depth; + this._tabId = tabId; this._frameId = frameId; this._parentPopupId = parentPopupId; this._parentFrameId = parentFrameId; @@ -437,7 +439,6 @@ class Frontend { return await this._popupFactory.getOrCreatePopup({ frameId: this._frameId, - ownerFrameId: this._frameId, depth: this._depth, childrenSupported: this._childrenSupported }); @@ -446,7 +447,6 @@ class Frontend { async _getProxyPopup() { return await this._popupFactory.getOrCreatePopup({ frameId: this._parentFrameId, - ownerFrameId: this._frameId, depth: this._depth, parentPopupId: this._parentPopupId, childrenSupported: this._childrenSupported @@ -469,7 +469,6 @@ class Frontend { const popup = await this._popupFactory.getOrCreatePopup({ frameId: targetFrameId, - ownerFrameId: this._frameId, id: popupId, childrenSupported: this._childrenSupported }); @@ -482,7 +481,6 @@ class Frontend { async _getPopupWindow() { return await this._popupFactory.getOrCreatePopup({ - ownerFrameId: this._frameId, depth: this._depth, popupWindow: true, childrenSupported: this._childrenSupported @@ -537,7 +535,11 @@ class Frontend { documentTitle }, content: { - definitions + definitions, + contentOrigin: { + tabId: this._tabId, + frameId: this._frameId + } } }; if (textSource instanceof TextSourceElement && textSource.fullContent !== query) { diff --git a/ext/fg/js/popup-factory.js b/ext/fg/js/popup-factory.js index 6960b254..7571d7ab 100644 --- a/ext/fg/js/popup-factory.js +++ b/ext/fg/js/popup-factory.js @@ -56,7 +56,6 @@ class PopupFactory { async getOrCreatePopup({ frameId=null, - ownerFrameId=null, id=null, parentPopupId=null, depth=null, @@ -103,8 +102,7 @@ class PopupFactory { const popup = new PopupWindow({ id, depth, - frameId: this._frameId, - ownerFrameId + frameId: this._frameId }); this._popups.set(id, popup); return popup; @@ -117,7 +115,6 @@ class PopupFactory { id, depth, frameId: this._frameId, - ownerFrameId, childrenSupported }); if (parent !== null) { @@ -139,14 +136,12 @@ class PopupFactory { id, parentPopupId, frameId, - ownerFrameId, childrenSupported })); const popup = new PopupProxy({ id, depth, frameId, - ownerFrameId, frameOffsetForwarder: useFrameOffsetForwarder ? this._frameOffsetForwarder : null }); this._popups.set(id, popup); diff --git a/ext/fg/js/popup-proxy.js b/ext/fg/js/popup-proxy.js index bb037705..b2e81824 100644 --- a/ext/fg/js/popup-proxy.js +++ b/ext/fg/js/popup-proxy.js @@ -24,14 +24,12 @@ class PopupProxy extends EventDispatcher { id, depth, frameId, - ownerFrameId, frameOffsetForwarder }) { super(); this._id = id; this._depth = depth; this._frameId = frameId; - this._ownerFrameId = ownerFrameId; this._frameOffsetForwarder = frameOffsetForwarder; this._frameOffset = [0, 0]; diff --git a/ext/fg/js/popup-window.js b/ext/fg/js/popup-window.js index a6011874..9398c287 100644 --- a/ext/fg/js/popup-window.js +++ b/ext/fg/js/popup-window.js @@ -23,14 +23,12 @@ class PopupWindow extends EventDispatcher { constructor({ id, depth, - frameId, - ownerFrameId + frameId }) { super(); this._id = id; this._depth = depth; this._frameId = frameId; - this._ownerFrameId = ownerFrameId; this._popupTabId = null; } diff --git a/ext/fg/js/popup.js b/ext/fg/js/popup.js index 5c2e57e9..47330090 100644 --- a/ext/fg/js/popup.js +++ b/ext/fg/js/popup.js @@ -27,14 +27,12 @@ class Popup extends EventDispatcher { 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; @@ -275,7 +273,6 @@ class Popup extends EventDispatcher { depth: this._depth, parentPopupId: this._id, parentFrameId: this._frameId, - ownerFrameId: this._ownerFrameId, childrenSupported: this._childrenSupported, scale: this._contentScale, optionsContext: this._optionsContext diff --git a/ext/mixed/js/display.js b/ext/mixed/js/display.js index 99ccfa85..69cc3d42 100644 --- a/ext/mixed/js/display.js +++ b/ext/mixed/js/display.js @@ -36,8 +36,10 @@ */ class Display extends EventDispatcher { - constructor(pageType, japaneseUtil, documentFocusController, hotkeyHandler) { + constructor(tabId, frameId, pageType, japaneseUtil, documentFocusController, hotkeyHandler) { super(); + this._tabId = tabId; + this._frameId = frameId; this._pageType = pageType; this._japaneseUtil = japaneseUtil; this._documentFocusController = documentFocusController; @@ -98,7 +100,8 @@ class Display extends EventDispatcher { this._depth = 0; this._parentPopupId = null; this._parentFrameId = null; - this._ownerFrameId = null; + this._contentOriginTabId = tabId; + this._contentOriginFrameId = frameId; this._childrenSupported = true; this._frameEndpoint = (pageType === 'popup' ? new FrameEndpoint() : null); this._browser = null; @@ -199,6 +202,14 @@ class Display extends EventDispatcher { return this._progressIndicatorVisible; } + get tabId() { + return this._tabId; + } + + get frameId() { + return this._frameId; + } + async prepare() { // State setup const {documentElement} = document; @@ -246,6 +257,13 @@ class Display extends EventDispatcher { } } + getContentOrigin() { + return { + tabId: this._contentOriginTabId, + frameId: this._contentOriginFrameId + }; + } + initializeState() { this._onStateChanged(); if (this._frameEndpoint !== null) { @@ -394,7 +412,7 @@ class Display extends EventDispatcher { close() { switch (this._pageType) { case 'popup': - this._invokeOwner('closePopup'); + this._invokeContentOrigin('closePopup'); break; case 'search': this._closeTab(); @@ -427,7 +445,8 @@ class Display extends EventDispatcher { params: this._createSearchParams(type, query, false), state, content: { - definitions: null + definitions: null, + contentOrigin: this.getContentOrigin() } }; this.setContent(details); @@ -494,14 +513,10 @@ class Display extends EventDispatcher { this._setContentScale(scale); } - async _onMessageConfigure({depth, parentPopupId, parentFrameId, ownerFrameId, childrenSupported, scale, optionsContext}) { + async _onMessageConfigure({depth, parentPopupId, parentFrameId, childrenSupported, scale, optionsContext}) { this._depth = depth; this._parentPopupId = parentPopupId; this._parentFrameId = parentFrameId; - this._ownerFrameId = ownerFrameId; - if (this._pageType === 'popup') { - this._hotkeyHandler.forwardFrameId = ownerFrameId; - } this._childrenSupported = childrenSupported; this._setContentScale(scale); await this.setOptionsContext(optionsContext); @@ -615,7 +630,8 @@ class Display extends EventDispatcher { cause: 'queryParser' }, content: { - definitions + definitions, + contentOrigin: this.getContentOrigin() } }; this.setContent(details); @@ -629,7 +645,12 @@ class Display extends EventDispatcher { history: false, params: {type}, state: {}, - content: {} + content: { + contentOrigin: { + tabId: this._tabId, + frameId: this._frameId + } + } }; this.setContent(details); } @@ -691,7 +712,8 @@ class Display extends EventDispatcher { documentTitle }, content: { - definitions + definitions, + contentOrigin: this.getContentOrigin() } }; this.setContent(details); @@ -886,6 +908,24 @@ class Display extends EventDispatcher { changeHistory = true; } + let contentOriginValid = false; + const {contentOrigin} = content; + if (typeof contentOrigin === 'object' && contentOrigin !== null) { + const {tabId, frameId} = contentOrigin; + if (typeof tabId === 'number' && typeof frameId === 'number') { + this._contentOriginTabId = tabId; + this._contentOriginFrameId = frameId; + if (this._pageType === 'popup') { + this._hotkeyHandler.forwardFrameId = (tabId === this._tabId ? frameId : null); + } + contentOriginValid = true; + } + } + if (!contentOriginValid) { + content.contentOrigin = this.getContentOrigin(); + changeHistory = true; + } + await this._setOptionsContextIfDifferent(optionsContext); if (this._setContentToken !== token) { return; } @@ -1499,10 +1539,10 @@ class Display extends EventDispatcher { } = options; const timestamp = Date.now(); - const ownerFrameId = this._ownerFrameId; + const screenshotFrameId = this._contentOriginFrameId; const definitionDetails = this._getDefinitionDetailsForNote(definition); const audioDetails = (mode !== 'kanji' && this._ankiNoteBuilder.containsMarker(fields, 'audio') ? {sources, customSourceUrl, customSourceType} : null); - const screenshotDetails = (this._ankiNoteBuilder.containsMarker(fields, 'screenshot') ? {ownerFrameId, format, quality} : null); + const screenshotDetails = (this._ankiNoteBuilder.containsMarker(fields, 'screenshot') ? {frameId: screenshotFrameId, format, quality} : null); const clipboardDetails = { image: this._ankiNoteBuilder.containsMarker(fields, 'clipboard-image'), text: this._ankiNoteBuilder.containsMarker(fields, 'clipboard-text') @@ -1583,8 +1623,6 @@ class Display extends EventDispatcher { parentFrameId: this._parentFrameId }; - const {frameId} = await api.frameInformationGet(); - await dynamicLoader.loadScripts([ '/mixed/js/text-scanner.js', '/mixed/js/frame-client.js', @@ -1597,12 +1635,13 @@ class Display extends EventDispatcher { '/fg/js/frontend.js' ]); - const popupFactory = new PopupFactory(frameId); + const popupFactory = new PopupFactory(this._frameId); popupFactory.prepare(); Object.assign(setupNestedPopupsOptions, { depth: this._depth + 1, - frameId, + tabId: this._tabId, + frameId: this._frameId, popupFactory, pageType: this._pageType, allowRootFramePopupProxy: true, @@ -1615,15 +1654,15 @@ class Display extends EventDispatcher { await frontend.prepare(); } - async _invokeOwner(action, params={}) { - if (this._ownerFrameId === null) { - throw new Error('No owner frame'); + async _invokeContentOrigin(action, params={}) { + if (this._contentOriginTabId === this._tabId && this._contentOriginFrameId === this._frameId) { + throw new Error('Content origin is same page'); } - return await api.crossFrame.invoke(this._ownerFrameId, action, params); + return await api.crossFrame.invokeTab(this._contentOriginTabId, this._contentOriginFrameId, action, params); } _copyHostSelection() { - if (this._ownerFrameId === null || window.getSelection().toString()) { return false; } + if (this._contentOriginFrameId === null || window.getSelection().toString()) { return false; } this._copyHostSelectionInner(); return true; } @@ -1635,7 +1674,7 @@ class Display extends EventDispatcher { { let text; try { - text = await this._invokeOwner('getSelectionText'); + text = await this._invokeContentOrigin('getSelectionText'); } catch (e) { break; } @@ -1643,7 +1682,7 @@ class Display extends EventDispatcher { } break; default: - await this._invokeOwner('copySelection'); + await this._invokeContentOrigin('copySelection'); break; } } @@ -1760,7 +1799,8 @@ class Display extends EventDispatcher { documentTitle }, content: { - definitions + definitions, + contentOrigin: this.getContentOrigin() } }; this._definitionTextScanner.clearSelection(true); @@ -1816,7 +1856,7 @@ class Display extends EventDispatcher { } async _initializeFrameResize(token) { - const size = await this._invokeOwner('getFrameSize'); + const size = await this._invokeContentOrigin('getFrameSize'); if (this._frameResizeToken !== token) { return; } this._frameResizeStartSize = size; } @@ -1842,7 +1882,7 @@ class Display extends EventDispatcher { height += y - this._frameResizeStartOffset.y; width = Math.max(Math.max(0, handleSize.width), width); height = Math.max(Math.max(0, handleSize.height), height); - await this._invokeOwner('setFrameSize', {width, height}); + await this._invokeContentOrigin('setFrameSize', {width, height}); } _updateHotkeys(options) {