From 1d05a374df843f97408d76c8f9fb4dc5a29a7259 Mon Sep 17 00:00:00 2001 From: Dima Gerasimov Date: Sat, 8 Feb 2020 14:45:00 +0000 Subject: [PATCH] Ask for permissions during options saving Seems like a more reliable way --- src/js/background.js | 4 +-- src/js/common.js | 62 ---------------------------------- src/js/options_page.js | 8 ++++- src/js/permissions.js | 76 ++++++++++++++++++++++++++++++++++++++++++ src/js/popup.js | 18 +++++----- 5 files changed, 93 insertions(+), 75 deletions(-) create mode 100644 src/js/permissions.js diff --git a/src/js/background.js b/src/js/background.js index 41310ac..4555b38 100644 --- a/src/js/background.js +++ b/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); } }); diff --git a/src/js/common.js b/src/js/common.js index e3a3ce7..b1010a4 100644 --- a/src/js/common.js +++ b/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): Promise { 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??? -} diff --git a/src/js/options_page.js b/src/js/options_page.js index a244ccf..63392b7 100644 --- a/src/js/options_page.js +++ b/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, }; diff --git a/src/js/permissions.js b/src/js/permissions.js new file mode 100644 index 0000000..c616499 --- /dev/null +++ b/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??? +} diff --git a/src/js/popup.js b/src/js/popup.js index 87e58e2..fd3466c 100644 --- a/src/js/popup.js +++ b/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({ - 'method': METHOD_CAPTURE_WITH_EXTRAS, - ...state, - }, () => { - console.log("[popup] captured!"); - }); - window.submitted = true; - window.close(); + chrome.runtime.sendMessage({ + 'method': METHOD_CAPTURE_WITH_EXTRAS, + ...state, + }, () => { + console.log("[popup] captured!"); }); + window.submitted = true; + window.close(); } // $FlowFixMe