Permissions requesting fixes (#1334)

* Fix permissions requests not occurring during user interaction

* Update toggle after changing permissions

* Update functions

* Update clipboard requests

4 files changed, 71 insertions, 23 deletions

diff --git a/ext/bg/js/permissions-main.js b/ext/bg/js/permissions-main.js
index 0d45ace6..cd03dd72 100644
--- a/ext/bg/js/permissions-main.js
+++ b/ext/bg/js/permissions-main.js
@@ -36,21 +36,47 @@ async function isAllowedFileSchemeAccess() {
     return await new Promise((resolve) => chrome.extension.isAllowedFileSchemeAccess(resolve));
 }
 
-async function hasPermissions(permissions) {
-    return await new Promise((resolve) => chrome.permissions.contains({permissions}, resolve));
+function hasPermissions(permissions) {
+    return new Promise((resolve) => chrome.permissions.contains({permissions}, (result) => {
+        const e = chrome.runtime.lastError;
+        resolve(!e && result);
+    }));
 }
 
-async function setPermissionsGranted(permissions, shouldHave) {
-    const has = await hasPermissions(permissions);
-    if (shouldHave === has) { return has; }
-
-    return await (
+function setPermissionsGranted(permissions, shouldHave) {
+    return (
         shouldHave ?
-        new Promise((resolve) => chrome.permissions.request({permissions}, resolve)) :
-        new Promise((resolve) => chrome.permissions.remove({permissions}, (v) => resolve(!v)))
+        new Promise((resolve, reject) => chrome.permissions.request({permissions}, (result) => {
+            const e = chrome.runtime.lastError;
+            if (e) {
+                reject(new Error(e.message));
+            } else {
+                resolve(result);
+            }
+        })) :
+        new Promise((resolve, reject) => chrome.permissions.remove({permissions}, (result) => {
+            const e = chrome.runtime.lastError;
+            if (e) {
+                reject(new Error(e.message));
+            } else {
+                resolve(!result);
+            }
+        }))
     );
 }
 
+function setupPermissionCheckbox(checkbox, permissions) {
+    checkbox.addEventListener('change', (e) => {
+        updatePermissionCheckbox(checkbox, permissions, e.currentTarget.checked);
+    }, false);
+}
+
+async function updatePermissionCheckbox(checkbox, permissions, value) {
+    checkbox.checked = !value;
+    const hasPermission = await setPermissionsGranted(permissions, value);
+    checkbox.checked = hasPermission;
+}
+
 (async () => {
     try {
         const documentFocusController = new DocumentFocusController();
@@ -79,9 +105,7 @@ async function setPermissionsGranted(permissions, shouldHave) {
             permissionsCheckboxes[i].checked = permissions[i];
         }
 
-        permissionsCheckboxes[0].addEventListener('change', (e) => {
-            setPermissionsGranted(['clipboardRead'], e.currentTarget.checked);
-        });
+        setupPermissionCheckbox(permissionsCheckboxes[0], ['clipboardRead']);
 
         await promiseTimeout(100);
 
diff --git a/ext/bg/js/settings/anki-controller.js b/ext/bg/js/settings/anki-controller.js
index e7c8db15..26abebeb 100644
--- a/ext/bg/js/settings/anki-controller.js
+++ b/ext/bg/js/settings/anki-controller.js
@@ -339,14 +339,7 @@ class AnkiController {
     }
 
     async _requestClipboardReadPermission() {
-        const permissions = ['clipboardRead'];
-
-        if (await new Promise((resolve) => chrome.permissions.contains({permissions}, resolve))) {
-            // Already has permission
-            return;
-        }
-
-        return await new Promise((resolve) => chrome.permissions.request({permissions}, resolve));
+        return await this._settingsController.setPermissionsGranted(['clipboardRead'], true);
     }
 
     _getFieldMarkers(fieldValue) {
diff --git a/ext/bg/js/settings/clipboard-popups-controller.js b/ext/bg/js/settings/clipboard-popups-controller.js
index ab1a8288..ec1d20ec 100644
--- a/ext/bg/js/settings/clipboard-popups-controller.js
+++ b/ext/bg/js/settings/clipboard-popups-controller.js
@@ -58,9 +58,7 @@ class ClipboardPopupsController {
         let value = checkbox.checked;
 
         if (value) {
-            value = await new Promise((resolve) => {
-                chrome.permissions.request({permissions: ['clipboardRead']}, resolve);
-            });
+            value = await this._settingsController.setPermissionsGranted(['clipboardRead'], true);
             checkbox.checked = value;
         }
 
diff --git a/ext/bg/js/settings/settings-controller.js b/ext/bg/js/settings/settings-controller.js
index 107e8004..e59ab7db 100644
--- a/ext/bg/js/settings/settings-controller.js
+++ b/ext/bg/js/settings/settings-controller.js
@@ -132,6 +132,39 @@ class SettingsController extends EventDispatcher {
         return optionsFull;
     }
 
+    hasPermissions(permissions) {
+        return new Promise((resolve, reject) => chrome.permissions.contains({permissions}, (result) => {
+            const e = chrome.runtime.lastError;
+            if (e) {
+                reject(new Error(e.message));
+            } else {
+                resolve(result);
+            }
+        }));
+    }
+
+    setPermissionsGranted(permissions, shouldHave) {
+        return (
+            shouldHave ?
+            new Promise((resolve, reject) => chrome.permissions.request({permissions}, (result) => {
+                const e = chrome.runtime.lastError;
+                if (e) {
+                    reject(new Error(e.message));
+                } else {
+                    resolve(result);
+                }
+            })) :
+            new Promise((resolve, reject) => chrome.permissions.remove({permissions}, (result) => {
+                const e = chrome.runtime.lastError;
+                if (e) {
+                    reject(new Error(e.message));
+                } else {
+                    resolve(!result);
+                }
+            }))
+        );
+    }
+
     // Private
 
     _setProfileIndex(value) {