more work towards manifest v3 compatiblity + proper end2end tests

extension/src/background.ts

@@ -33,6 +33,7 @@ import browser from "webextension-polyfill"
 import {COMMAND_CAPTURE_SIMPLE, METHOD_CAPTURE_WITH_EXTRAS, showNotification} from './common'
 import {getOptions} from './options'
 import type {Options} from './options'
+import { hasPermissions } from './permissions'
 
 
 type Params = {
@@ -54,6 +55,16 @@ async function makeCaptureRequest(
 
     const endpoint = options.endpoint
 
+    // Ugh. this is kinda idiotic.
+    // We can't call ensurePermissions here, getting "permissions.request may only be called from a user input handler" error
+    // even though it's called from the keyboard shortcut???
+    // so the best we can do is try to check and at least show a more helful error
+
+    const has = await hasPermissions(endpoint)
+    if (!has) {
+        throw new Error(`${endpoint}: no permissions detected! Go to extension settings and approve them.`)
+    }
+
     const data = JSON.stringify(params)
     console.log(`capturing ${data} via ${endpoint}`)
 
@@ -69,7 +80,7 @@ async function makeCaptureRequest(
     } catch (err) {
         console.error(err)
         // fetch only throws when host is unavailable
-        throw new Error(`${endpoint} unavailable, check if server is running`)
+        throw new Error(`${endpoint} unavailable, check if server is running: ${(err as Error).toString()}`)
     }
 
     if (!response.ok) { // http error

extension/src/options_page.ts

@@ -61,8 +61,18 @@ async function saveOptions() {
     // TODO could also check for permissions and display message?
 
     const endpoint = getEndpoint().value
-    await ensurePermissions(endpoint)
-    refreshPermissionValidation(endpoint)
+    try {
+        // note: this might fail if we messed up manifest for instance, and didn't add optional hosts
+        const got = await ensurePermissions(endpoint)
+        if (!got) {
+            throw Error("Permissions weren't granted!")
+        }
+    } catch (error) {
+        // todo show notification instead?
+        alert(`${(error as Error).toString()}`)
+        throw error
+    }
+    refreshPermissionValidation(endpoint)  // TODO need to call it after every typed character
 
     const opts = {
         endpoint: endpoint,

extension/webpack.config.js

@@ -45,8 +45,8 @@ const commands = {
     "capture-simple": {
         "description": "Quick capture: url, title and selection",
         "suggested_key": {
-            "default": `Ctrl+${modifier}+C`,
-            "mac":  `Command+${modifier}+C`,
+            "default": `Ctrl+${modifier}+H`,
+            "mac":  `Command+${modifier}+H`,
         },
     }
 }
@@ -147,20 +147,27 @@ const manifestExtra = {
 manifestExtra[action_name] = action
 
 if (v3) {
-  manifestExtra['host_permissions'] = host_permissions
+  if (target === T.FIREFOX) {
+    // firefox doesn't support optional host permissions
+    // note that these will still have to be granted by user (unlike in chrome)
+    manifestExtra['host_permissions'] = [...host_permissions, ...optional_host_permissions]
+  } else {
+    manifestExtra['host_permissions'] = host_permissions
+    manifestExtra['optional_host_permissions'] = optional_host_permissions
+  }
+} else {
+  manifestExtra.permissions.push(...host_permissions)
+  manifestExtra.optional_permissions.push(...optional_host_permissions)
+}
 
-  // FIXME not sure if firefox supports this??
-  // https://bugzilla.mozilla.org/show_bug.cgi?id=1766026
-  manifestExtra['optional_host_permissions'] = optional_host_permissions
+if (v3) {
+  // this isn't really required in chrome, but without it, webext lint fails for chrome addon
   const gecko_id = target === T.FIREFOX ? ext_id : "{00000000-0000-0000-0000-000000000000}"
   manifestExtra['browser_specific_settings'] = {
     "gecko": {
       "id": gecko_id,
     },
   }
-} else {
-  manifestExtra.permissions.push(...host_permissions)
-  manifestExtra.optional_permissions.push(...optional_host_permissions)
 }
 
 

setup.py

@@ -37,7 +37,15 @@ def main() -> None:
         ],
         extras_require={
             'testing': ['pytest', 'requests'],
-            'linting': ['pytest', 'mypy', 'lxml', 'ruff'], # lxml for mypy coverage report
+            'linting': [
+                'pytest',
+                'mypy', 'lxml',  # lxml for mypy coverage report
+                'ruff',
+
+                'selenium',
+                'click',
+                'loguru',
+            ],
         },
 
 

tests/addon_helper.py

@@ -0,0 +1,119 @@
+from dataclasses import dataclass
+from functools import cached_property
+import json
+from pathlib import Path
+import subprocess
+import re
+
+
+import psutil
+from selenium import webdriver
+
+
+@dataclass
+class AddonHelper:
+    driver: webdriver.Remote
+
+    @cached_property
+    def addon_id(self) -> str:
+        return get_addon_id(driver=self.driver)
+
+    @property
+    def extension_prefix(self) -> str:
+        protocol = {
+            'chrome': 'chrome-extension',
+            'firefox': 'moz-extension',
+        }[self.driver.name]
+        return f'{protocol}://{self.addon_id}'
+
+    def open_page(self, path: str) -> None:
+        self.driver.get(self.extension_prefix + '/' + path)
+
+
+# NOTE looks like it used to be posssible in webdriver api?
+# at least as of 2011 https://github.com/gavinp/chromium/blob/681563ea0f892a051f4ef3d5e53438e0bb7d2261/chrome/test/webdriver/test/chromedriver.py#L35-L40
+# but here https://github.com/SeleniumHQ/selenium/blob/master/cpp/webdriver-server/command_types.h there are no Extension commands
+# also see driver.command_executor._commands
+def _get_chrome_addon_id(driver: webdriver.Remote) -> str:
+    """
+    For a temporary addon extension id is autogenerated, so we need to extract it every time
+    """
+    user_data_dir = Path(driver.capabilities['chrome']['userDataDir'])
+    prefs_file = user_data_dir / 'Default/Preferences'
+    assert prefs_file.exists(), prefs_file
+
+    # for some idiotic reason, after chrome launches, extension settings aren't immediately available
+    # this can take up to 30 secons in this loop until they are populated
+    while True:
+        prefs = json.loads(prefs_file.read_text())
+        extension_settings = prefs.get('extensions', {}).get('settings', None)
+        if extension_settings is not None:
+            # there are some other weird builtin extension as well
+            # this seems like the easiest way to filter them out extracing by extension name or path
+            [addon_id] = [k for k, v in extension_settings.items() if v['creation_flags'] != 1]
+            return addon_id
+
+
+def _get_firefox_addon_id(driver: webdriver.Remote) -> str:
+    moz_profile = Path(driver.capabilities['moz:profile'])
+    prefs_file = moz_profile / 'prefs.js'
+    assert prefs_file.exists(), prefs_file
+
+    while True:
+        for line in prefs_file.read_text().splitlines():
+            m = re.fullmatch(r'user_pref\("extensions.webextensions.uuids", "(.*)"\);', line)
+            if m is None:
+                continue
+            # this contains a json with escaped quotes
+            user_prefs_s = m.group(1).replace('\\', '')
+            user_prefs = json.loads(user_prefs_s)
+            addon_ids = [v for k, v in user_prefs.items() if 'mozilla.' not in k]
+            if len(addon_ids) == 0:
+                # for some stupid reason it doesn't appear immediately in the file
+                continue
+            [addon_id] = addon_ids
+            return addon_id
+
+
+def get_addon_id(driver: webdriver.Remote) -> str:
+    extractor = {
+        'firefox': _get_firefox_addon_id,
+        'chrome': _get_chrome_addon_id,
+    }[driver.name]
+    return extractor(driver)
+
+
+def get_window_id(driver: webdriver.Remote) -> str:
+    if driver.name == 'firefox':
+        pid = str(driver.capabilities['moz:processID'])
+    elif driver.name == 'chrome':
+        # ugh no pid in capabilities...
+        driver_pid = driver.service.process.pid  # type: ignore[attr-defined]
+        process = psutil.Process(driver_pid)
+        [chrome_process] = process.children()
+        cmdline = chrome_process.cmdline()
+        assert '--enable-automation' in cmdline, cmdline
+        pid = str(chrome_process.pid)
+    else:
+        raise RuntimeError(driver.name)
+    return get_wid_by_pid(pid)
+
+
+def get_wid_by_pid(pid: str) -> str:
+    # https://askubuntu.com/a/385037/427470
+    wids = subprocess.check_output(['xdotool', 'search', '--pid', pid]).decode('utf8').splitlines()
+    wids = [w.strip() for w in wids if len(w.strip()) > 0]
+
+    def has_wm_desktop(wid: str) -> bool:
+        # TODO no idea why is that important. found out experimentally
+        out = subprocess.check_output(['xprop', '-id', wid, '_NET_WM_DESKTOP']).decode('utf8')
+        return 'not found' not in out
+
+    [wid] = filter(has_wm_desktop, wids)
+    return wid
+
+
+def focus_browser_window(driver: webdriver.Remote) -> None:
+    # FIXME assert not is_headless(driver)  # just in case
+    wid = get_window_id(driver)
+    subprocess.check_call(['xdotool', 'windowactivate', '--sync', wid])

tests/conftest.py

@@ -0,0 +1,14 @@
+import pytest
+
+
+def pytest_addoption(parser):
+    parser.addoption(
+        '--browser',
+        choices=['firefox', 'chrome'],
+        default='firefox',  # FIXME add --all or something
+    )
+
+
+@pytest.fixture
+def browser(request) -> str:
+    return request.config.getoption("--browser")