Ignore AltGr keystrokes in Ctrl+Alt keyboard shortcuts (#825)

* Ignore AltGr keystrokes in Ctrl+Alt keyboard shortcuts

Browsers report AltGr (right Alt on AZERTY/QWERTZ and most non-US
layouts, used to type @ # { } [ ] | \ and the euro sign) as
ctrlKey+altKey. The default keybinds map destructive actions to
Ctrl+Alt+<letter> (delete_session, new_session, incognito,
open_calendar), so a non-US user typing a special character could
silently fire them.

Guard the shortcut matcher, the editor keydown handler, and the rebind
capture with getModifierState('AltGraph'), which is true for AltGr but
false for a genuine left Ctrl+Alt. macOS is excluded: there the Option
key legitimately sets AltGraph and there is no AltGr/Ctrl+Alt collision
to guard against, so the guard would otherwise break Ctrl+Option /
Cmd+Option shortcuts (notably in Firefox).

The detection lives in one place — isAltGrEvent / IS_MAC in
static/js/platform.js — and all three call sites route through it, so the
guards can't drift apart.

The editor handler only skips the Ctrl+Alt chord block, so layout
shortcuts reachable via AltGr (e.g. [ ] brush size = AltGr+5/+8 on
AZERTY) keep working.

* Require Ctrl+Alt for the AltGr guard and consolidate keybind test marks

isAltGrEvent now also checks ctrlKey+altKey so it only suppresses the
"AltGr reported as Ctrl+Alt" collision; an event asserting AltGraph on
its own (a Linux ISO_Level3_Shift layout, a stray modifier) is left
alone. Pin it with test_isaltgr_false_when_altgraph_set_but_not_ctrl_alt.

Collapse the 12 per-test node skipif marks into one module-level
pytestmark, and note in platform.js why IS_MAC intentionally covers
iPad/iPhone and mirrors the isMac checks in calendar.js / sessions.js.

static/js/editor/keyboard-shortcuts.js

@@ -50,6 +50,7 @@
  * }} deps
  */
 import { state } from './state.js';
+import { isAltGrEvent } from '../platform.js';
 
 export function wireKeyboardShortcuts(deps) {
   const {
@@ -79,7 +80,11 @@ export function wireKeyboardShortcuts(deps) {
       return;
     }
     if (e.key === 'Escape') return;
-    if (e.ctrlKey || e.metaKey) {
+    // Skip the Ctrl+Alt editor chords for an AltGr keystroke (see platform.js);
+    // only the chord block is skipped, so the layout-character handlers below
+    // still act — AltGr+5 / AltGr+8 stay as the [ ] brush-size shortcut on
+    // AZERTY / QWERTZ.
+    if ((e.ctrlKey || e.metaKey) && !isAltGrEvent(e)) {
       if (e.key === 'z') { e.preventDefault(); if (e.shiftKey) redo(); else undo(); }
       // Ctrl+Shift+D = Deselect: clears the wand selection (and
       // lasso if active) without affecting layers.

static/js/keyboard-shortcuts.js

@@ -2,6 +2,8 @@
 // Keyboard Shortcuts — dynamic keybinds
 // ============================================
 
+import { IS_MAC, isAltGrEvent } from './platform.js';
+
 const _defaultKeybinds = {
   search: 'ctrl+k', toggle_sidebar: 'ctrl+alt+b', new_session: 'ctrl+alt+n',
   fav_session: 'ctrl+alt+f', delete_session: 'ctrl+alt+d',
@@ -13,8 +15,11 @@ const _defaultKeybinds = {
   open_notes: '', open_tasks: '', open_theme: '',
 };
 
-function _matchesCombo(e, combo) {
+export function _matchesCombo(e, combo, isMac = IS_MAC) {
   if (!combo) return false;
+  // Drop AltGr keystrokes so typing characters on non-US layouts can't fire a
+  // Ctrl+Alt shortcut — e.g. the destructive delete_session. See platform.js.
+  if (isAltGrEvent(e, isMac)) return false;
   const parts = combo.split('+');
   const needCtrl = parts.includes('ctrl');
   const needAlt = parts.includes('alt');

static/js/platform.js

@@ -0,0 +1,47 @@
+// ============================================
+// Platform detection + AltGr-keystroke helper
+// ============================================
+// Shared by the keybind code: root keyboard-shortcuts.js, the editor's
+// keyboard-shortcuts.js, and settings.js. Single source of truth so the three
+// guards can't drift.
+
+// AltGr (right Alt on AZERTY/QWERTZ and most non-US layouts, used to type
+// @ # { } [ ] | \ and €) is reported by browsers as Ctrl+Alt. macOS is the
+// exception: there the Option key — a normal part of Mac shortcuts — also sets
+// the AltGraph modifier state, so it must NOT be treated as AltGr.
+//
+// IS_MAC covers all Apple platforms, iPad/iPhone included: a Magic Keyboard's
+// Option key sets AltGraph exactly like a Mac's, so they need the same carve-out
+// — narrowing to macOS-only would re-break them. The name and the
+// /Mac|iPhone|iPad/ test deliberately mirror the existing isMac checks in
+// calendar.js and sessions.js; this is their single shared source of truth.
+export const IS_MAC =
+  /Mac|iPhone|iPad/.test((typeof navigator !== 'undefined' && navigator.platform) || '') ||
+  /Mac/.test((typeof navigator !== 'undefined' && navigator.userAgent) || '');
+
+// True when `e` is an AltGr keystroke we should ignore for Ctrl+Alt shortcut
+// purposes. getModifierState('AltGraph') is true for AltGr but false for a
+// genuine left Ctrl+Alt, so real shortcuts still work. Always false on macOS,
+// where Option legitimately sets AltGraph.
+//
+// We also require ctrlKey+altKey: the collision we defend against is precisely
+// "AltGr reported AS Ctrl+Alt", so an event that asserts AltGraph WITHOUT
+// presenting as Ctrl+Alt (a Linux ISO_Level3_Shift layout, a stray modifier
+// state) is left alone instead of being swallowed.
+//
+// Trade-off: on Windows AltGr *is* Ctrl+right-Alt, so a deliberate
+// Ctrl+Alt+<char> shortcut typed via AltGr is unreachable too — accepted; use
+// the left Ctrl+Alt.
+//
+// NOTE: the AltGr -> AltGraph mapping is taken from the UI Events spec / MDN,
+// not proven by our tests. Older Firefox and some Linux setups historically did
+// not report AltGraph; where a browser sets ctrlKey+altKey without it this
+// guard is simply a no-op (the pre-fix behaviour) rather than a regression.
+export function isAltGrEvent(e, isMac = IS_MAC) {
+  return (
+    !isMac &&
+    !!e.ctrlKey &&
+    !!e.altKey &&
+    !!(e.getModifierState && e.getModifierState('AltGraph'))
+  );
+}

static/js/settings.js

@@ -6,6 +6,7 @@ import searchModule from './search.js';
 import { makeWindowDraggable } from './windowDrag.js';
 import { clearDockSide } from './modalSnap.js';
 import { sortModelIds } from './modelSort.js';
+import { isAltGrEvent } from './platform.js';
 
 let initialized = false;
 let modalEl = null;
@@ -1710,6 +1711,10 @@ function _formatKeyCaps(combo) {
 }
 
 function _comboFromEvent(e) {
+  // Drop a stray AltGr keystroke (e.g. AltGr+E to type €) so it isn't recorded
+  // as a bogus ctrl+alt+<char> binding — onKey ignores empty combos. See
+  // platform.js for the macOS carve-out and Windows trade-off.
+  if (isAltGrEvent(e)) return '';
   const parts = [];
   if (e.ctrlKey || e.metaKey) parts.push('ctrl');
   if (e.altKey) parts.push('alt');

tests/test_keybind_altgr_js.py

@@ -0,0 +1,183 @@
+"""Pin the AltGr-safety of the shared keybind predicate and the matcher.
+
+Driven through `node --input-type=module` so we exercise the real JS without a
+full Vitest/Jest setup (same approach as test_compare_js.py /
+test_reply_recipients_js.py). Skips when `node` is not installed rather than
+failing.
+
+Bug: browsers report the AltGr key (right Alt, essential on AZERTY/QWERTZ and
+many non-US layouts to type @ # { } [ ] | \\ and €) as ctrlKey=true AND
+altKey=true, so a user on a non-US layout typing a special character could
+silently fire a destructive ctrl+alt+<letter> default (new_session,
+delete_session, incognito, open_calendar). getModifierState('AltGraph') is true
+for AltGr but false for a genuine left Ctrl+Alt — except on macOS, where the
+Option key also sets it.
+
+The guard now lives in ONE place — `isAltGrEvent` in static/js/platform.js — and
+all three call sites (editor keyboard-shortcuts.js, root keyboard-shortcuts.js,
+settings.js) route through it. So these tests pin the shared *predicate*
+directly (both the isMac arg and the navigator-derived IS_MAC default), plus the
+`_matchesCombo` integration. They do NOT prove that real browsers actually set
+AltGraph for AltGr — that mapping is taken from the UI Events spec / MDN; older
+Firefox and some Linux setups historically did not report it (the guard is a
+no-op there, i.e. pre-fix behaviour, not a regression).
+"""
+import json
+import shutil
+import subprocess
+from pathlib import Path
+
+import pytest
+
+_REPO = Path(__file__).resolve().parent.parent
+_HELPER = _REPO / "static" / "js" / "keyboard-shortcuts.js"
+_PLATFORM = _REPO / "static" / "js" / "platform.js"
+_HAS_NODE = shutil.which("node") is not None
+
+# Every test here shells out to `node`; skip the whole module when it is absent
+# rather than repeating the mark per test (same convention as test_compare_js.py
+# / test_reply_recipients_js.py).
+pytestmark = pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
+
+
+def _run(js: str) -> str:
+    proc = subprocess.run(
+        ["node", "--input-type=module"],
+        input=js, capture_output=True, text=True, cwd=str(_REPO), timeout=30,
+    )
+    assert proc.returncode == 0, proc.stderr
+    return proc.stdout.strip()
+
+
+def _is_altgr(
+    altgraph: bool,
+    is_mac: bool = False,
+    has_modifier_state: bool = True,
+    ctrl: bool = True,
+    alt: bool = True,
+) -> bool:
+    """Return isAltGrEvent(ev, is_mac) — the predicate every guard routes through."""
+    modifier = (
+        f"ev.getModifierState = (m) => m === 'AltGraph' ? {json.dumps(altgraph)} : false;"
+        if has_modifier_state else "")
+    js = f"""
+    import {{ isAltGrEvent }} from '{_PLATFORM.as_uri()}';
+    const ev = {{ ctrlKey: {json.dumps(ctrl)}, altKey: {json.dumps(alt)} }};
+    {modifier}
+    console.log(JSON.stringify(isAltGrEvent(ev, {json.dumps(is_mac)})));
+    """
+    return json.loads(_run(js))
+
+
+def _is_mac_default(platform: str = "", user_agent: str = "") -> bool:
+    """Return platform.js IS_MAC as derived from a stubbed navigator at import time."""
+    # Node >=21 exposes a read-only global `navigator`, so assignment throws;
+    # defineProperty (configurable) overrides it for the import-time read.
+    js = f"""
+    Object.defineProperty(globalThis, 'navigator', {{
+      value: {{ platform: {json.dumps(platform)}, userAgent: {json.dumps(user_agent)} }},
+      configurable: true,
+    }});
+    const {{ IS_MAC }} = await import('{_PLATFORM.as_uri()}');
+    console.log(JSON.stringify(IS_MAC));
+    """
+    return json.loads(_run(js))
+
+
+def _matches(event: dict, combo: str, altgraph: bool, is_mac: bool = False) -> bool:
+    """Return _matchesCombo(event, combo, is_mac) with AltGraph active or not."""
+    js = f"""
+    import {{ _matchesCombo }} from '{_HELPER.as_uri()}';
+    const ev = {json.dumps(event)};
+    ev.getModifierState = (m) => m === 'AltGraph' ? {json.dumps(altgraph)} : false;
+    console.log(JSON.stringify(_matchesCombo(ev, {json.dumps(combo)}, {json.dumps(is_mac)})));
+    """
+    return json.loads(_run(js))
+
+
+# --- The shared predicate (covers all three guards) --------------------------
+
+def test_isaltgr_true_for_altgr_keystroke_off_mac():
+    # AZERTY/QWERTZ user holds AltGr: browser sets ctrlKey+altKey+AltGraph.
+    assert _is_altgr(altgraph=True, is_mac=False) is True
+
+
+def test_isaltgr_false_for_genuine_ctrl_alt():
+    # A real left Ctrl+Alt press leaves AltGraph unset.
+    assert _is_altgr(altgraph=False, is_mac=False) is False
+
+
+def test_isaltgr_false_when_altgraph_set_but_not_ctrl_alt():
+    # The collision we defend against is specifically "AltGr reported AS
+    # Ctrl+Alt". An event that asserts AltGraph WITHOUT presenting as Ctrl+Alt
+    # (e.g. a Linux ISO_Level3_Shift layout, or a stray modifier state) must NOT
+    # be swallowed — only a genuine Ctrl+Alt-presenting AltGr keystroke is.
+    assert _is_altgr(altgraph=True, ctrl=False, alt=False) is False
+    assert _is_altgr(altgraph=True, ctrl=True, alt=False) is False
+    assert _is_altgr(altgraph=True, ctrl=False, alt=True) is False
+
+
+def test_isaltgr_false_on_mac_even_with_altgraph():
+    # macOS reports AltGraph=true for the Option key, but Ctrl+Option / Cmd+Option
+    # are legitimate Mac shortcuts, so the predicate must never swallow them.
+    assert _is_altgr(altgraph=True, is_mac=True) is False
+
+
+def test_isaltgr_false_when_getmodifierstate_missing():
+    # Defensive: an event without getModifierState must not throw or report AltGr.
+    assert _is_altgr(altgraph=False, is_mac=False, has_modifier_state=False) is False
+
+
+# --- The navigator-derived IS_MAC default (dead in node without a stub) -------
+
+def test_is_mac_from_navigator_platform():
+    # navigator.platform reports "MacIntel" on EVERY Mac — Apple Silicon
+    # (M1/M2/M3...) included; the string was frozen for compatibility, so there
+    # is no "MacARM". The regex matches the "Mac" substring, not "Intel".
+    assert _is_mac_default(platform="MacIntel") is True
+
+
+def test_is_mac_apple_silicon_reports_macintel():
+    # Pin the quirk explicitly: an Apple Silicon Mac's UA still says Macintosh
+    # and its platform still says MacIntel, so the carve-out protects it too.
+    assert _is_mac_default(
+        platform="MacIntel",
+        user_agent="Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15",
+    ) is True
+
+
+def test_is_mac_from_user_agent_when_platform_blank():
+    # iPadOS / some browsers report a Mac userAgent with an unhelpful platform.
+    assert _is_mac_default(platform="", user_agent="Mozilla/5.0 (Macintosh; ...)") is True
+
+
+def test_is_not_mac_on_windows():
+    assert _is_mac_default(platform="Win32", user_agent="Mozilla/5.0 (Windows NT 10.0)") is False
+
+
+# --- _matchesCombo integration (the matcher predicate, end to end) -----------
+
+def test_altgr_keystroke_does_not_trigger_ctrl_alt_shortcut():
+    # AZERTY/QWERTZ user holds AltGr over a key that yields 'n'. This must NOT
+    # fire the destructive new_session combo.
+    ev = {"ctrlKey": True, "altKey": True, "shiftKey": False, "key": "n"}
+    assert _matches(ev, "ctrl+alt+n", altgraph=True, is_mac=False) is False
+
+
+def test_genuine_ctrl_alt_still_matches():
+    # A real left Ctrl+Alt press (AltGraph not set) must still work.
+    ev = {"ctrlKey": True, "altKey": True, "shiftKey": False, "key": "n"}
+    assert _matches(ev, "ctrl+alt+n", altgraph=False, is_mac=False) is True
+
+
+def test_mac_option_combo_still_matches():
+    # macOS reports AltGraph=true for the Option key, but Ctrl+Option / Cmd+Option
+    # are legitimate Mac shortcuts. On macOS the guard must NOT swallow them.
+    ev = {"ctrlKey": True, "altKey": True, "shiftKey": False, "key": "n"}
+    assert _matches(ev, "ctrl+alt+n", altgraph=True, is_mac=True) is True
+
+
+def test_plain_ctrl_shortcut_unaffected():
+    # Non-alt combos were never AltGr-ambiguous and must keep matching.
+    ev = {"ctrlKey": True, "altKey": False, "shiftKey": False, "key": "k"}
+    assert _matches(ev, "ctrl+k", altgraph=False, is_mac=False) is True