Ask for permissions during options saving

Seems like a more reliable way

src/js/background.js

@@ -1,7 +1,7 @@
 /* @flow */
 // TODO do I really need to annotate all files with @flow??
 
-import {COMMAND_CAPTURE_SIMPLE, METHOD_CAPTURE_WITH_EXTRAS, showNotification, ensurePermissions} from './common';
+import {COMMAND_CAPTURE_SIMPLE, METHOD_CAPTURE_WITH_EXTRAS, showNotification} from './common';
 import {get_options} from './options';
 
 type Params = {
@@ -108,7 +108,7 @@ function capture(comment: ?string = null, tag_str: ?string = null) {
 
 chrome.commands.onCommand.addListener(command => {
     if (command === COMMAND_CAPTURE_SIMPLE) {
-        ensurePermissions(() => capture(null, null));
+        capture(null, null);
     }
 });
 

src/js/common.js

@@ -1,6 +1,4 @@
 /* @flow */
-import {getOptions} from './options';
-
 export const COMMAND_CAPTURE_SIMPLE = 'capture-simple';
 
 export const METHOD_CAPTURE_WITH_EXTRAS = 'captureWithExtras';
@@ -30,63 +28,3 @@ export function awrap(fn, ...args: Array<any>): Promise<any> {
         fn(...args, cbb);
     });
 }
-
-export function chromePermissionsRequest(params: any) {
-    return awrap(chrome.permissions.request, params);
-}
-
-export function chromePermissionsContains(params: any) {
-    return awrap(chrome.permissions.contains, params);
-}
-
-
-function urlForPermissionsCheck(url: string): string {
-    var u = new URL(url);
-
-    u.port = '';
-    // firefox doesn't like port numbers
-    // https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Match_patterns#Invalid_match_patterns
-
-    // also toString adds a trailing slash, otherwise Firefox is also confused
-    return u.toString();
-}
-
-// TODO fuck. doesn't seem to work for Firefox at all...
-// just getting  Error: "An unexpected error occurred" background.bundle.js
-// seems like this bug https://bugzilla.mozilla.org/show_bug.cgi?id=1382953
-export function ensurePermissions(cb: () => void) {
-    const back = chrome.extension.getBackgroundPage().console;
-
-    getOptions().then(opts => {
-        const endpoint = opts.endpoint;
-        // that's capable of handling default (localhost), so shouldn't prompt
-        const perms = {
-            // TODO shit ok, apparently needs to end with / ??
-            origins: [
-                urlForPermissionsCheck(endpoint),
-            ],
-        };
-        return chromePermissionsContains(perms).then(has => {
-            if (has) {
-                // no need to do anything
-                return true;
-            } else {
-                return chromePermissionsRequest(perms);
-            }
-        });
-    }).then(granted => {
-        if (granted) {
-            back.info("Permission was granted");
-            cb();
-        } else {
-            const err = chrome.runtime.lastError;
-            const msg = err ? String(err) : 'Permission was refused';
-            showNotification(`ERROR: ${msg}`);
-        }
-    }).catch(err => {
-        // TODO eh, use generic defensify instead...
-        console.error(err);
-        showNotification(`ERROR: ${err}`);
-    });
-    // TODO improve message somehow???
-}

src/js/options_page.js

@@ -1,5 +1,6 @@
 /* @flow */
 import {get_options, set_options} from './options';
+import {ensurePermissions} from './permissions';
 
 const ENDPOINT_ID = 'endpoint_id';
 const NOTIFICATION_ID = 'notification_id';
@@ -34,8 +35,13 @@ function restoreOptions() {
 }
 
 function saveOptions() {
+    // TODO could also check for permissions and display message?
+
+    const endpoint = getEndpoint().value;
+    ensurePermissions(endpoint);
+
     const opts = {
-        endpoint: getEndpoint().value, // TODO validate??
+        endpoint: endpoint,
         default_tags: getDefaultTags().value,
         notification: getEnableNotification().checked,
     };

src/js/permissions.js

@@ -0,0 +1,76 @@
+/* @flow */
+import {showNotification, awrap} from './common';
+import {getOptions} from './options';
+
+
+export function chromePermissionsRequest(params: any) {
+    return awrap(chrome.permissions.request, params);
+}
+
+export function chromePermissionsContains(params: any) {
+    return awrap(chrome.permissions.contains, params);
+}
+
+
+function urlForPermissionsCheck(url: string): string {
+    var u = new URL(url);
+
+    u.port = '';
+    // firefox doesn't like port numbers
+    // https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Match_patterns#Invalid_match_patterns
+
+    // also toString adds a trailing slash, otherwise Firefox is also confused
+    return u.toString();
+}
+
+
+export function ensurePermissions(endpoint: string) {
+    const perms = {
+        origins: [
+            urlForPermissionsCheck(endpoint),
+        ],
+    };
+    // shouldn't prompt if we already have the permission
+    chromePermissionsRequest(perms);
+}
+
+// just keeping the old one for the reference...
+
+// TODO fuck. doesn't seem to work for Firefox at all...
+// just getting  Error: "An unexpected error occurred" background.bundle.js
+// seems like this bug https://bugzilla.mozilla.org/show_bug.cgi?id=1382953
+function ensurePermissions_old(cb: () => void) { // eslint-disable-line no-unused-vars
+    const back = chrome.extension.getBackgroundPage().console;
+
+    getOptions().then(opts => {
+        const endpoint = opts.endpoint;
+        // that's capable of handling default (localhost), so shouldn't prompt
+        const perms = {
+            origins: [
+                urlForPermissionsCheck(endpoint),
+            ],
+        };
+        return chromePermissionsContains(perms).then(has => {
+            if (has) {
+                // no need to do anything
+                return true;
+            } else {
+                return chromePermissionsRequest(perms);
+            }
+        });
+    }).then(granted => {
+        if (granted) {
+            back.info("Permission was granted");
+            cb();
+        } else {
+            const err = chrome.runtime.lastError;
+            const msg = err ? String(err) : 'Permission was refused';
+            showNotification(`ERROR: ${msg}`);
+        }
+    }).catch(err => {
+        // TODO eh, use generic defensify instead...
+        console.error(err);
+        showNotification(`ERROR: ${err}`);
+    });
+    // TODO improve message somehow???
+}

src/js/popup.js

@@ -1,6 +1,6 @@
 /* @flow */
 
-import {METHOD_CAPTURE_WITH_EXTRAS, ensurePermissions} from './common';
+import {METHOD_CAPTURE_WITH_EXTRAS} from './common';
 import {get_options} from './options';
 
 // TODO template it in html too?
@@ -62,16 +62,14 @@ function submitComment () {
     // TODO focus
     const state = getState();
 
-    ensurePermissions(() => {
+    chrome.runtime.sendMessage({
-        chrome.runtime.sendMessage({
+        'method': METHOD_CAPTURE_WITH_EXTRAS,
-            'method': METHOD_CAPTURE_WITH_EXTRAS,
+        ...state,
-            ...state,
+    }, () => {
-        }, () => {
+        console.log("[popup] captured!");
-            console.log("[popup] captured!");
-        });
-        window.submitted = true;
-        window.close();
     });
+    window.submitted = true;
+    window.close();
 }
 
 // $FlowFixMe