From e3e7337841d07d86f3aa58f93d560eb296ea34cb Mon Sep 17 00:00:00 2001 From: rugk Date: Fri, 13 Sep 2019 19:09:14 +0200 Subject: [PATCH 1/2] Finally fix retry logic and corner cases --- src/common/modules/AutomaticSettings | 2 +- src/options/modules/CustomOptionTriggers.js | 39 ++++++++++++++------- src/options/modules/PermissionRequest.js | 20 ++++++++++- src/popup/lib/emoji-mart-embed | 1 + 4 files changed, 48 insertions(+), 14 deletions(-) create mode 160000 src/popup/lib/emoji-mart-embed diff --git a/src/common/modules/AutomaticSettings b/src/common/modules/AutomaticSettings index fac4b54..2bc739f 160000 --- a/src/common/modules/AutomaticSettings +++ b/src/common/modules/AutomaticSettings @@ -1 +1 @@ -Subproject commit fac4b54b3ebb430ed10982be77e529371c4aff1b +Subproject commit 2bc739feb39cf5ae17db1e508c93a293c9850885 diff --git a/src/options/modules/CustomOptionTriggers.js b/src/options/modules/CustomOptionTriggers.js index 8a0198b..388ef3e 100644 --- a/src/options/modules/CustomOptionTriggers.js +++ b/src/options/modules/CustomOptionTriggers.js @@ -273,9 +273,10 @@ function updateEmojiPerLineMaxViaEmojiSize(optionValue, option, event) { * @private * @param {Object} optionValue * @param {string} [option] + * @param {Object} [event] * @returns {Promise} */ -function applyEmojiSearch(optionValue) { +function applyEmojiSearch(optionValue, option, event = {}) { // switch status of dependent settings if (optionValue.enabled) { document.getElementById("searchAction").disabled = false; @@ -293,6 +294,25 @@ function applyEmojiSearch(optionValue) { toEnable: optionValue.enabled }); + const reloadEmojiSearchStatus = () => { + // get new settings, because they could have been changed + // TODO: generalize in AutomaticSettings + const isEnabled = document.getElementById("omnibarIntegration").checked; + + const newOptionValue = { + enabled: isEnabled + }; + + if (document.getElementById("searchAction").checked) { + newOptionValue.action = document.getElementById("searchAction").value; + } else if (document.getElementById("emojipediaAction").checked) { + newOptionValue.action = document.getElementById("emojipediaAction").value; + } + + // we can only all hope, this won't end in an inifnitive loop + applyEmojiSearch(newOptionValue); + } + // request permission from user if (optionValue.enabled && // only if actually enabled optionValue.action === "copy" && // if we require a permission for copying @@ -303,19 +323,14 @@ function applyEmojiSearch(optionValue) { MESSAGE_EMOJI_COPY_PERMISSION_SEARCH, event, {retry: true} - ).then(() => { - // also trigger update when permission is granted - browser.runtime.sendMessage({ - type: COMMUNICATION_MESSAGE_TYPE.OMNIBAR_TOGGLE, - toEnable: optionValue.enabled - }); - }).catch(() => { - // only rejects in case of fatal error - debugger; - // TODO: revert setting to previous state + ).finally(() => { + // Note: Error (rejection) will never happen, because we have infinite retries enabled + // So this is equivalent to a "then". + reloadEmojiSearchStatus(); }); } else { - debugger; // TODO. cancel permission prompt already shown + // debugger; + PermissionRequest.cancelPermissionPrompt(CLIPBOARD_WRITE_PERMISSION); } return Promise.resolve(); diff --git a/src/options/modules/PermissionRequest.js b/src/options/modules/PermissionRequest.js index b485e0a..6c0783b 100644 --- a/src/options/modules/PermissionRequest.js +++ b/src/options/modules/PermissionRequest.js @@ -285,7 +285,7 @@ export function requestPermission(permissions, messageId, event, options = {}) { } // validate parameters - if (options.retry < 1) { + if (options.retry !== true && options.retry !== false && options.retry < 1) { throw new TypeError(`invalid options.retry value of ${options.retry} passed.`); } @@ -371,6 +371,24 @@ export function requestPermission(permissions, messageId, event, options = {}) { return returnPermission; } +/** + * Cancels the permission prompt. + * + * Due to technical limitations, it cannot actually close the permission prompt. It can just hide the own + * Thus, if a permission is currently being requested, this may lead to strange side-effects if the permission + * is granted anyway, because the old Promise will still be fullfilled then. + * + * @public + * @param {browser.permissions.Permissions} permissions the permission request to close, + * see https://developer.mozilla.org/docs/Mozilla/Add-ons/WebExtensions/API/permissions/Permissions + * @returns {void} + */ +export function cancelPermissionPrompt(permissions) { + const thisPermission = getInternalPermissionData(permissions); + + thisPermission.messageBoxes.forEach(hideMessageBox); +} + /** * **Syncronously** checks whether the permission has is granted. * diff --git a/src/popup/lib/emoji-mart-embed b/src/popup/lib/emoji-mart-embed new file mode 160000 index 0000000..2b78524 --- /dev/null +++ b/src/popup/lib/emoji-mart-embed @@ -0,0 +1 @@ +Subproject commit 2b78524314ae8ee3807be555452877624c181fdd From 5b21c0ff893f04f95474f91d7a95722c03771e0c Mon Sep 17 00:00:00 2001 From: rugk Date: Fri, 13 Sep 2019 19:11:51 +0200 Subject: [PATCH 2/2] Add explanation link --- src/options/modules/PermissionRequest.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/options/modules/PermissionRequest.js b/src/options/modules/PermissionRequest.js index 6c0783b..56ab096 100644 --- a/src/options/modules/PermissionRequest.js +++ b/src/options/modules/PermissionRequest.js @@ -384,6 +384,9 @@ export function requestPermission(permissions, messageId, event, options = {}) { * @returns {void} */ export function cancelPermissionPrompt(permissions) { + // we cannot actually really close the permission prompt, see: + // https://discourse.mozilla.org/t/can-browser-extension-permission-requests-be-cancelled/44734?u=rugkx + const thisPermission = getInternalPermissionData(permissions); thisPermission.messageBoxes.forEach(hideMessageBox);