Native messaging optional permission (#1348)

* Refactor PermissionsToggleController to not require a setting * Update nativeMessaging to be optional on Chrome * Update parsing.enableMecabParser setting to request permissions * Update permissions page to use PermissionsToggleController * Update permissions documentation * Disable toggle for permissions which are not optional
2021-02-08 17:52:41 -05:00 · 2021-02-08 17:52:41 -05:00 · 849e4fabe1
commit 849e4fabe1
parent 008809e0e7
8 changed files with 99 additions and 74 deletions
--- a/dev/data/manifest-variants.json
+++ b/dev/data/manifest-variants.json
@ -79,12 +79,12 @@
            "storage",
            "clipboardWrite",
            "unlimitedStorage",
-            "nativeMessaging",
            "webRequest",
            "webRequestBlocking"
        ],
        "optional_permissions": [
-            "clipboardRead"
+            "clipboardRead",
+            "nativeMessaging"
        ],
        "commands": {
            "toggleTextScanning": {
@ -204,6 +204,16 @@
                            "strict_min_version": "57.0"
                        }
                    }
+                },
+                {
+                    "action": "remove",
+                    "path": ["optional_permissions"],
+                    "item": "nativeMessaging"
+                },
+                {
+                    "action": "add",
+                    "path": ["permissions"],
+                    "items": ["nativeMessaging"]
                }
            ],
            "excludeFiles": [
--- a/docs/permissions.md
+++ b/docs/permissions.md
@ -23,12 +23,6 @@
  Yomichan will sometimes need to inject stylesheets into webpages in order to
  properly display the search popup.

-* `nativeMessaging` <br>
-  Yomichan has the ability to communicate with an optional native messaging component in order to support
-  parsing large blocks of Japanese text using
-  [MeCab](https://en.wikipedia.org/wiki/MeCab).
-  The installation of this component is optional and is not included by default.
-
 * `clipboardWrite` <br>
  Yomichan supports simulating the `Ctrl+C` (copy to clipboard) keyboard shortcut
  when a definitions popup is open and focused.
@ -38,3 +32,10 @@
  while the browser is running, depending on how certain settings are configured.
  This allows Yomichan to support scanning text from external applications, provided there is a way
  to copy text from those applications to the clipboard.
+
+* `nativeMessaging` (optional on Chrome) <br>
+  Yomichan has the ability to communicate with an optional native messaging component in order to support
+  parsing large blocks of Japanese text using
+  [MeCab](https://en.wikipedia.org/wiki/MeCab).
+  The installation of this component is optional and is not included by default.
+  This permission is optional on Chrome, but required on Firefox, because Firefox does not permit it to be optional.
--- a/ext/bg/js/permissions-main.js
+++ b/ext/bg/js/permissions-main.js
@ -17,6 +17,8 @@

 /* global
 * DocumentFocusController
+ * PermissionsToggleController
+ * SettingsController
 * api
 */

@ -36,45 +38,24 @@ async function isAllowedFileSchemeAccess() {
    return await new Promise((resolve) => chrome.extension.isAllowedFileSchemeAccess(resolve));
 }

-function hasPermissions(permissions) {
-    return new Promise((resolve) => chrome.permissions.contains({permissions}, (result) => {
-        const e = chrome.runtime.lastError;
-        resolve(!e && result);
-    }));
-}
+function setupPermissionsToggles() {
+    const manifest = chrome.runtime.getManifest();
+    let optionalPermissions = manifest.optional_permissions;
+    if (!Array.isArray(optionalPermissions)) { optionalPermissions = []; }
+    optionalPermissions = new Set(optionalPermissions);

-function 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);
+    const hasAllPermisions = (set, values) => {
+        for (const value of values) {
+            if (!set.has(value)) { return false; }
        }
-        })) :
-        new Promise((resolve, reject) => chrome.permissions.remove({permissions}, (result) => {
-            const e = chrome.runtime.lastError;
-            if (e) {
-                reject(new Error(e.message));
-            } else {
-                resolve(!result);
+        return true;
+    };
+
+    for (const toggle of document.querySelectorAll('.permissions-toggle')) {
+        let permissions = toggle.dataset.requiredPermissions;
+        permissions = (typeof permissions === 'string' && permissions.length > 0 ? permissions.split(' ') : []);
+        toggle.disabled = !hasAllPermisions(optionalPermissions, permissions);
    }
-        }))
-    );
-}
-
-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 () => {
@ -82,6 +63,8 @@ async function updatePermissionCheckbox(checkbox, permissions, value) {
        const documentFocusController = new DocumentFocusController();
        documentFocusController.prepare();

+        setupPermissionsToggles();
+
        for (const node of document.querySelectorAll('.extension-id-example')) {
            node.textContent = chrome.runtime.getURL('/');
        }
@ -92,13 +75,11 @@ async function updatePermissionCheckbox(checkbox, permissions, value) {
        setupEnvironmentInfo();

        const permissionsCheckboxes = [
-            document.querySelector('#permission-checkbox-clipboard-read'),
            document.querySelector('#permission-checkbox-allow-in-private-windows'),
            document.querySelector('#permission-checkbox-allow-file-url-access')
        ];

        const permissions = await Promise.all([
-            hasPermissions(['clipboardRead']),
            isAllowedIncognitoAccess(),
            isAllowedFileSchemeAccess()
        ]);
@ -107,7 +88,11 @@ async function updatePermissionCheckbox(checkbox, permissions, value) {
            permissionsCheckboxes[i].checked = permissions[i];
        }

-        setupPermissionCheckbox(permissionsCheckboxes[0], ['clipboardRead']);
+        const settingsController = new SettingsController(0);
+        settingsController.prepare();
+
+        const permissionsToggleController = new PermissionsToggleController(settingsController);
+        permissionsToggleController.prepare();

        await promiseTimeout(100);

--- a/ext/bg/js/settings/permissions-toggle-controller.js
+++ b/ext/bg/js/settings/permissions-toggle-controller.js
@ -41,9 +41,16 @@ class PermissionsToggleController {
    // Private

    _onOptionsChanged({options}) {
-        const accessor = new ObjectPropertyAccessor(options);
+        let accessor = null;
        for (const toggle of this._toggles) {
-            const path = ObjectPropertyAccessor.getPathArray(toggle.dataset.permissionsSetting);
+            const {permissionsSetting} = toggle.dataset;
+            if (typeof permissionsSetting !== 'string') { continue; }
+
+            if (accessor === null) {
+                accessor = new ObjectPropertyAccessor(options);
+            }
+
+            const path = ObjectPropertyAccessor.getPathArray(permissionsSetting);
            let value;
            try {
                value = accessor.get(path, path.length);
@ -58,23 +65,38 @@ class PermissionsToggleController {
    async _onPermissionsToggleChange(e) {
        const toggle = e.currentTarget;
        let value = toggle.checked;
+        const valuePre = !value;
+        const {permissionsSetting} = toggle.dataset;
+        const hasPermissionsSetting = typeof permissionsSetting === 'string';

-        if (value) {
-            toggle.checked = false;
-            value = await this._settingsController.setPermissionsGranted(this._getRequiredPermissions(toggle), true);
+        if (value || !hasPermissionsSetting) {
+            toggle.checked = valuePre;
+            try {
+                value = await this._settingsController.setPermissionsGranted(this._getRequiredPermissions(toggle), value);
+            } catch (error) {
+                value = valuePre;
+            }
            toggle.checked = value;
        }

+        if (hasPermissionsSetting) {
            this._setToggleValid(toggle, true);
-
-        await this._settingsController.setProfileSetting(toggle.dataset.permissionsSetting, value);
+            await this._settingsController.setProfileSetting(permissionsSetting, value);
+        }
    }

    _onPermissionsChanged({permissions: {permissions}}) {
        const permissionsSet = new Set(permissions);
        for (const toggle of this._toggles) {
-            const valid = !toggle.checked || this._hasAll(permissionsSet, this._getRequiredPermissions(toggle));
+            const {permissionsSetting} = toggle.dataset;
+            const hasPermissions = this._hasAll(permissionsSet, this._getRequiredPermissions(toggle));
+
+            if (typeof permissionsSetting === 'string') {
+                const valid = !toggle.checked || hasPermissions;
                this._setToggleValid(toggle, valid);
+            } else {
+                toggle.checked = hasPermissions;
+            }
        }
    }

--- a/ext/bg/permissions.html
+++ b/ext/bg/permissions.html
@ -84,17 +84,6 @@
                </div>
            </div>
        </div></div>
-        <div class="settings-item"><div class="settings-item-inner">
-            <div class="settings-item-left">
-                <div class="settings-item-label"><code>nativeMessaging</code></div>
-                <div class="settings-item-description">
-                    Yomichan has the ability to communicate with an optional native messaging component in order to support
-                    parsing large blocks of Japanese text using
-                    <a href="https://en.wikipedia.org/wiki/MeCab" target="_blank" rel="noopener noreferrer">MeCab</a>.
-                    The installation of this component is optional and is not included by default.
-                </div>
-            </div>
-        </div></div>
        <div class="settings-item"><div class="settings-item-inner">
            <div class="settings-item-left">
                <div class="settings-item-label"><code>clipboardWrite</code></div>
@ -115,7 +104,21 @@
                </div>
            </div>
            <div class="settings-item-right">
-                <label class="toggle"><input type="checkbox" id="permission-checkbox-clipboard-read"><span class="toggle-body"><span class="toggle-track"></span><span class="toggle-knob"></span></span></label>
+                <label class="toggle"><input type="checkbox" class="permissions-toggle" data-required-permissions="clipboardRead"><span class="toggle-body"><span class="toggle-track"></span><span class="toggle-knob"></span></span></label>
+            </div>
+        </div></div>
+        <div class="settings-item"><div class="settings-item-inner">
+            <div class="settings-item-left">
+                <div class="settings-item-label"><code>nativeMessaging</code> <span class="light" data-show-for-browser="chrome edge">(optional)</span></div>
+                <div class="settings-item-description">
+                    Yomichan has the ability to communicate with an optional native messaging component in order to support
+                    parsing large blocks of Japanese text using
+                    <a href="https://en.wikipedia.org/wiki/MeCab" target="_blank" rel="noopener noreferrer">MeCab</a>.
+                    The installation of this component is optional and is not included by default.
+                </div>
+            </div>
+            <div class="settings-item-right">
+                <label class="toggle"><input type="checkbox" class="permissions-toggle" data-required-permissions="nativeMessaging"><span class="toggle-body"><span class="toggle-track"></span><span class="toggle-knob"></span></span></label>
            </div>
        </div></div>
        <div class="settings-item"><div class="settings-item-inner">
@ -164,6 +167,10 @@
 <script src="/mixed/js/api.js"></script>

 <script src="/mixed/js/document-focus-controller.js"></script>
+<script src="/mixed/js/html-template-collection.js"></script>
+
+<script src="/bg/js/settings/permissions-toggle-controller.js"></script>
+<script src="/bg/js/settings/settings-controller.js"></script>

 <script src="/bg/js/permissions-main.js"></script>

--- a/ext/bg/settings.html
+++ b/ext/bg/settings.html
@ -682,7 +682,7 @@
                </div>

                <div class="checkbox">
-                    <label><input type="checkbox" id="parsing-mecab-enable" data-setting="parsing.enableMecabParser"> Enable text parsing using MeCab</label>
+                    <label><input type="checkbox" id="parsing-mecab-enable" class="permissions-toggle" data-permissions-setting="parsing.enableMecabParser" data-required-permissions="nativeMessaging"> Enable text parsing using MeCab</label>
                </div>

                <div class="checkbox">
--- a/ext/bg/settings2.html
+++ b/ext/bg/settings2.html
@ -1151,7 +1151,7 @@
                    </div>
                </div>
                <div class="settings-item-right">
-                    <label class="toggle"><input type="checkbox" data-setting="parsing.enableMecabParser"><span class="toggle-body"><span class="toggle-track"></span><span class="toggle-knob"></span></span></label>
+                    <label class="toggle"><input type="checkbox" class="permissions-toggle" data-permissions-setting="parsing.enableMecabParser" data-required-permissions="nativeMessaging"><span class="toggle-body"><span class="toggle-track"></span><span class="toggle-knob"></span></span></label>
                </div>
            </div>
            <div class="settings-item-children more" hidden>
--- a/ext/manifest.json
+++ b/ext/manifest.json
@ -78,12 +78,12 @@
        "storage",
        "clipboardWrite",
        "unlimitedStorage",
-        "nativeMessaging",
        "webRequest",
        "webRequestBlocking"
    ],
    "optional_permissions": [
-        "clipboardRead"
+        "clipboardRead",
+        "nativeMessaging"
    ],
    "commands": {
        "toggleTextScanning": {