Fix screenshot popup hide (#753)

* Refactor Popup.setVisibleOverride

* Use event to observe visibility changes

* Add setAllVisibleOverride/clearAllVisibleOverride

* Add setAllVisibleOverride/clearAllVisibleOverride cross frame handlers

* Update how visibility is changed

* Wait for next frame to ensure visibility has been updated
This commit is contained in:
toasted-nutbread 2020-08-23 15:18:41 -04:00 committed by GitHub
parent 934355dd09
commit 773e28aa3c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 104 additions and 33 deletions

View File

@ -62,7 +62,6 @@ class Frontend {
this._updatePopupToken = null; this._updatePopupToken = null;
this._runtimeMessageHandlers = new Map([ this._runtimeMessageHandlers = new Map([
['popupSetVisibleOverride', {async: false, handler: this._onMessagePopupSetVisibleOverride.bind(this)}],
['requestFrontendReadyBroadcast', {async: false, handler: this._onMessageRequestFrontendReadyBroadcast.bind(this)}] ['requestFrontendReadyBroadcast', {async: false, handler: this._onMessageRequestFrontendReadyBroadcast.bind(this)}]
]); ]);
} }
@ -112,7 +111,9 @@ class Frontend {
['closePopup', {async: false, handler: this._onApiClosePopup.bind(this)}], ['closePopup', {async: false, handler: this._onApiClosePopup.bind(this)}],
['copySelection', {async: false, handler: this._onApiCopySelection.bind(this)}], ['copySelection', {async: false, handler: this._onApiCopySelection.bind(this)}],
['getPopupInfo', {async: false, handler: this._onApiGetPopupInfo.bind(this)}], ['getPopupInfo', {async: false, handler: this._onApiGetPopupInfo.bind(this)}],
['getDocumentInformation', {async: false, handler: this._onApiGetDocumentInformation.bind(this)}] ['getDocumentInformation', {async: false, handler: this._onApiGetDocumentInformation.bind(this)}],
['setAllVisibleOverride', {async: true, handler: this._onApiSetAllVisibleOverride.bind(this)}],
['clearAllVisibleOverride', {async: true, handler: this._onApiClearAllVisibleOverride.bind(this)}]
]); ]);
this._updateContentScale(); this._updateContentScale();
@ -161,11 +162,6 @@ class Frontend {
// Message handlers // Message handlers
_onMessagePopupSetVisibleOverride({visible}) {
if (this._popup === null) { return; }
this._popup.setVisibleOverride(visible);
}
_onMessageRequestFrontendReadyBroadcast({frameId}) { _onMessageRequestFrontendReadyBroadcast({frameId}) {
this._signalFrontendReady(frameId); this._signalFrontendReady(frameId);
} }
@ -196,6 +192,18 @@ class Frontend {
}; };
} }
async _onApiSetAllVisibleOverride({value, priority, awaitFrame}) {
const result = await this._popupFactory.setAllVisibleOverride(value, priority);
if (awaitFrame) {
await promiseAnimationFrame(100);
}
return result;
}
async _onApiClearAllVisibleOverride({token}) {
return await this._popupFactory.clearAllVisibleOverride(token);
}
// Private // Private
_onResize() { _onResize() {

View File

@ -27,6 +27,7 @@ class PopupFactory {
this._frameId = frameId; this._frameId = frameId;
this._frameOffsetForwarder = new FrameOffsetForwarder(frameId); this._frameOffsetForwarder = new FrameOffsetForwarder(frameId);
this._popups = new Map(); this._popups = new Map();
this._allPopupVisibilityTokenMap = new Map();
} }
// Public functions // Public functions
@ -39,6 +40,7 @@ class PopupFactory {
['hide', {async: false, handler: this._onApiHide.bind(this)}], ['hide', {async: false, handler: this._onApiHide.bind(this)}],
['isVisible', {async: true, handler: this._onApiIsVisibleAsync.bind(this)}], ['isVisible', {async: true, handler: this._onApiIsVisibleAsync.bind(this)}],
['setVisibleOverride', {async: true, handler: this._onApiSetVisibleOverride.bind(this)}], ['setVisibleOverride', {async: true, handler: this._onApiSetVisibleOverride.bind(this)}],
['clearVisibleOverride', {async: true, handler: this._onApiClearVisibleOverride.bind(this)}],
['containsPoint', {async: true, handler: this._onApiContainsPoint.bind(this)}], ['containsPoint', {async: true, handler: this._onApiContainsPoint.bind(this)}],
['showContent', {async: true, handler: this._onApiShowContent.bind(this)}], ['showContent', {async: true, handler: this._onApiShowContent.bind(this)}],
['setCustomCss', {async: false, handler: this._onApiSetCustomCss.bind(this)}], ['setCustomCss', {async: false, handler: this._onApiSetCustomCss.bind(this)}],
@ -108,6 +110,40 @@ class PopupFactory {
} }
} }
async setAllVisibleOverride(value, priority) {
const promises = [];
const errors = [];
for (const popup of this._popups.values()) {
const promise = popup.setVisibleOverride(value, priority)
.then(
(token) => ({popup, token}),
(error) => { errors.push(error); return null; }
);
promises.push(promise);
}
const results = await Promise.all(promises);
if (errors.length === 0) {
const token = generateId(16);
this._allPopupVisibilityTokenMap.set(token, results);
return token;
}
// Revert on error
await this._revertPopupVisibilityOverrides(results);
throw errors[0];
}
async clearAllVisibleOverride(token) {
const results = this._allPopupVisibilityTokenMap.get(token);
if (typeof results === 'undefined') { return false; }
this._allPopupVisibilityTokenMap.delete(token);
await this._revertPopupVisibilityOverrides(results);
return true;
}
// API message handlers // API message handlers
async _onApiGetOrCreatePopup({id, parentPopupId, frameId, ownerFrameId}) { async _onApiGetOrCreatePopup({id, parentPopupId, frameId, ownerFrameId}) {
@ -134,9 +170,14 @@ class PopupFactory {
return await popup.isVisible(); return await popup.isVisible();
} }
async _onApiSetVisibleOverride({id, visible}) { async _onApiSetVisibleOverride({id, value, priority}) {
const popup = this._getPopup(id); const popup = this._getPopup(id);
return await popup.setVisibleOverride(visible); return await popup.setVisibleOverride(value, priority);
}
async _onApiClearVisibleOverride({id, token}) {
const popup = this._getPopup(id);
return await popup.clearVisibleOverride(token);
} }
async _onApiContainsPoint({id, x, y}) { async _onApiContainsPoint({id, x, y}) {
@ -216,4 +257,19 @@ class PopupFactory {
const parent = popup.parent; const parent = popup.parent;
return parent === null || parent.isVisibleSync(); return parent === null || parent.isVisibleSync();
} }
async _revertPopupVisibilityOverrides(overrides) {
const promises = [];
for (const value of overrides) {
if (value === null) { continue; }
const {popup, token} = value;
const promise = popup.clearVisibleOverride(token)
.then(
(v) => v,
() => false
);
promises.push(promise);
}
return await Promise.all(promises);
}
} }

View File

@ -86,8 +86,12 @@ class PopupProxy extends EventDispatcher {
return this._invokeSafe('isVisible', {id: this._id}, false); return this._invokeSafe('isVisible', {id: this._id}, false);
} }
setVisibleOverride(visible) { async setVisibleOverride(value, priority) {
return this._invokeSafe('setVisibleOverride', {id: this._id, visible}); return this._invokeSafe('setVisibleOverride', {id: this._id, value, priority});
}
async clearVisibleOverride(token) {
return this._invokeSafe('clearVisibleOverride', {id: this._id, token});
} }
async containsPoint(x, y) { async containsPoint(x, y) {

View File

@ -34,8 +34,7 @@ class Popup extends EventDispatcher {
this._childrenSupported = true; this._childrenSupported = true;
this._injectPromise = null; this._injectPromise = null;
this._injectPromiseComplete = false; this._injectPromiseComplete = false;
this._visible = false; this._visible = new DynamicProperty(false);
this._visibleOverride = null;
this._options = null; this._options = null;
this._optionsContext = null; this._optionsContext = null;
this._contentScale = 1.0; this._contentScale = 1.0;
@ -96,11 +95,12 @@ class Popup extends EventDispatcher {
// Public functions // Public functions
prepare() { prepare() {
this._updateVisibility();
this._frame.addEventListener('mousedown', (e) => e.stopPropagation()); this._frame.addEventListener('mousedown', (e) => e.stopPropagation());
this._frame.addEventListener('scroll', (e) => e.stopPropagation()); this._frame.addEventListener('scroll', (e) => e.stopPropagation());
this._frame.addEventListener('load', this._onFrameLoad.bind(this)); this._frame.addEventListener('load', this._onFrameLoad.bind(this));
this._visible.on('change', this._onVisibleChange.bind(this));
yomichan.on('extensionUnloaded', this._onExtensionUnloaded.bind(this)); yomichan.on('extensionUnloaded', this._onExtensionUnloaded.bind(this));
this._onVisibleChange({value: this.isVisibleSync()});
} }
async setOptionsContext(optionsContext, source) { async setOptionsContext(optionsContext, source) {
@ -131,9 +131,12 @@ class Popup extends EventDispatcher {
return this.isVisibleSync(); return this.isVisibleSync();
} }
setVisibleOverride(visible) { async setVisibleOverride(value, priority) {
this._visibleOverride = visible; return this._visible.setOverride(value, priority);
this._updateVisibility(); }
async clearVisibleOverride(token) {
return this._visible.clearOverride(token);
} }
async containsPoint(x, y) { async containsPoint(x, y) {
@ -177,7 +180,7 @@ class Popup extends EventDispatcher {
} }
isVisibleSync() { isVisibleSync() {
return (this._visibleOverride !== null ? this._visibleOverride : this._visible); return this._visible.value;
} }
updateTheme() { updateTheme() {
@ -392,12 +395,11 @@ class Popup extends EventDispatcher {
} }
_setVisible(visible) { _setVisible(visible) {
this._visible = visible; this._visible.defaultValue = visible;
this._updateVisibility();
} }
_updateVisibility() { _onVisibleChange({value}) {
this._frame.style.setProperty('visibility', this.isVisibleSync() ? 'visible' : 'hidden', 'important'); this._frame.style.setProperty('visibility', value ? 'visible' : 'hidden', 'important');
} }
_focusParent() { _focusParent() {

View File

@ -1145,9 +1145,12 @@ class Display extends EventDispatcher {
} }
async _getScreenshot() { async _getScreenshot() {
const ownerFrameId = this._ownerFrameId;
let token = null;
try { try {
await this._setPopupVisibleOverride(false); if (ownerFrameId !== null) {
await promiseTimeout(1); // Wait for popup to be hidden. token = await api.crossFrame.invoke(ownerFrameId, 'setAllVisibleOverride', {value: false, priority: 0, awaitFrame: true});
}
const {format, quality} = this._options.anki.screenshot; const {format, quality} = this._options.anki.screenshot;
const dataUrl = await api.screenshotGet({format, quality}); const dataUrl = await api.screenshotGet({format, quality});
@ -1155,7 +1158,9 @@ class Display extends EventDispatcher {
return {dataUrl, format}; return {dataUrl, format};
} finally { } finally {
await this._setPopupVisibleOverride(null); if (token !== null) {
await api.crossFrame.invoke(ownerFrameId, 'clearAllVisibleOverride', {token});
}
} }
} }
@ -1163,10 +1168,6 @@ class Display extends EventDispatcher {
return this._options.general.resultOutputMode === 'merge' ? 0 : -1; return this._options.general.resultOutputMode === 'merge' ? 0 : -1;
} }
_setPopupVisibleOverride(visible) {
return api.broadcastTab('popupSetVisibleOverride', {visible});
}
_getEntry(index) { _getEntry(index) {
const entries = this._container.querySelectorAll('.entry'); const entries = this._container.querySelectorAll('.entry');
return index >= 0 && index < entries.length ? entries[index] : null; return index >= 0 && index < entries.length ? entries[index] : null;