fix(calls): hotkey rebind denylist; ignore modifiers; keep Space on buttons
- useKeyBind refuses Tab/Enter/arrows/Home/End/Page*/Escape and bare modifier codes, and refuses a code equal to the other call key, with an inline message (isBindableCallKey, unit-tested). - PTT and deafen handlers ignore events with Ctrl/Alt/Meta held (deafen also Shift), so Cmd+M / Ctrl+M no longer toggle deafen. - PTT only preventDefault()s when the target is not an interactive control, so Space still activates focused buttons during a call. Fixes #23 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PPmy3tPq869XDW4njjVaKA
This commit is contained in:
@@ -171,6 +171,10 @@ export function CallControls({ callEmbed }: CallControlsProps) {
|
|||||||
|
|
||||||
const onKeyDown = (e: KeyboardEvent) => {
|
const onKeyDown = (e: KeyboardEvent) => {
|
||||||
if (e.code !== pttKey || e.repeat) return;
|
if (e.code !== pttKey || e.repeat) return;
|
||||||
|
// [Gitea #23] Ignore the PTT key with Ctrl/Alt/Meta held so it doesn't
|
||||||
|
// hijack OS/app chords (e.g. Cmd+Space) that happen to share the code.
|
||||||
|
// Shift is allowed through — Shift+Space is a harmless combo for PTT.
|
||||||
|
if (e.ctrlKey || e.altKey || e.metaKey) return;
|
||||||
const target = e.target as HTMLElement;
|
const target = e.target as HTMLElement;
|
||||||
// BUG-7: use ownerDocument.body so isEditable works inside the EC iframe
|
// BUG-7: use ownerDocument.body so isEditable works inside the EC iframe
|
||||||
const isEditable = (el: HTMLElement): boolean => {
|
const isEditable = (el: HTMLElement): boolean => {
|
||||||
@@ -185,7 +189,23 @@ export function CallControls({ callEmbed }: CallControlsProps) {
|
|||||||
return false;
|
return false;
|
||||||
};
|
};
|
||||||
if (isEditable(target)) return;
|
if (isEditable(target)) return;
|
||||||
e.preventDefault();
|
// [Gitea #23] Don't swallow Space on a focused button/link/etc — PTT still
|
||||||
|
// engages the mic, but the key's default action (activating the control)
|
||||||
|
// is left alone so keyboard users can still Tab+Space the call buttons.
|
||||||
|
const isInteractive = (el: HTMLElement): boolean => {
|
||||||
|
const tag = el.tagName;
|
||||||
|
if (tag === 'BUTTON' || tag === 'A' || tag === 'SELECT') return true;
|
||||||
|
let node: HTMLElement | null = el;
|
||||||
|
while (node && node !== el.ownerDocument.body) {
|
||||||
|
const role = node.getAttribute('role');
|
||||||
|
if (role === 'button' || role === 'link' || role === 'menuitem' || role === 'tab') {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
node = node.parentElement;
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
};
|
||||||
|
if (!isInteractive(target)) e.preventDefault();
|
||||||
// C-M5: mark PTT active BEFORE unmuting so the mic echo (onMediaState)
|
// C-M5: mark PTT active BEFORE unmuting so the mic echo (onMediaState)
|
||||||
// doesn't treat this transient unmute as a user-initiated undeafen.
|
// doesn't treat this transient unmute as a user-initiated undeafen.
|
||||||
callEmbed.control.pttActive = true;
|
callEmbed.control.pttActive = true;
|
||||||
@@ -256,6 +276,10 @@ export function CallControls({ callEmbed }: CallControlsProps) {
|
|||||||
const onKeyDown = (e: KeyboardEvent) => {
|
const onKeyDown = (e: KeyboardEvent) => {
|
||||||
if (e.code !== deafenKey) return;
|
if (e.code !== deafenKey) return;
|
||||||
if (e.repeat) return;
|
if (e.repeat) return;
|
||||||
|
// [Gitea #23] Ignore the deafen key with any modifier held — with the
|
||||||
|
// default 'KeyM', Ctrl+M / Alt+M / Cmd+M are common OS/app chords that
|
||||||
|
// shouldn't also toggle deafen (and previously got preventDefault()ed).
|
||||||
|
if (e.ctrlKey || e.altKey || e.metaKey || e.shiftKey) return;
|
||||||
if (isEditable(e.target as HTMLElement)) return;
|
if (isEditable(e.target as HTMLElement)) return;
|
||||||
e.preventDefault();
|
e.preventDefault();
|
||||||
callEmbed.control.toggleSound();
|
callEmbed.control.toggleSound();
|
||||||
|
|||||||
@@ -117,6 +117,7 @@ import { playCallJoinSound } from '../../../utils/callSounds';
|
|||||||
import { previewRingtone, RINGTONE_OPTIONS } from '../../../utils/ringtones';
|
import { previewRingtone, RINGTONE_OPTIONS } from '../../../utils/ringtones';
|
||||||
import { DenoiseTester } from './DenoiseTester';
|
import { DenoiseTester } from './DenoiseTester';
|
||||||
import { SettingsSelect } from '../../../components/settings-select/SettingsSelect';
|
import { SettingsSelect } from '../../../components/settings-select/SettingsSelect';
|
||||||
|
import { isBindableCallKey } from '../../../utils/callKeybind';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* P5-47 — opt-in TDS window chrome toggle (desktop only). Renders nothing in the
|
* P5-47 — opt-in TDS window chrome toggle (desktop only). Renders nothing in the
|
||||||
@@ -1474,8 +1475,12 @@ function Privacy() {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
function useKeyBind(setter: (code: string) => void) {
|
// [Gitea #23] Denylist navigation-critical/modifier codes and reject a code that
|
||||||
|
// collides with the other call key (`otherKey`), so a rebind can never trap
|
||||||
|
// keyboard focus in-call or silently double-bind PTT and deafen to the same key.
|
||||||
|
function useKeyBind(setter: (code: string) => void, otherKey?: string) {
|
||||||
const [listening, setListening] = useState(false);
|
const [listening, setListening] = useState(false);
|
||||||
|
const [error, setError] = useState<string | null>(null);
|
||||||
const listenerRef = useRef<((e: KeyboardEvent) => void) | null>(null);
|
const listenerRef = useRef<((e: KeyboardEvent) => void) | null>(null);
|
||||||
|
|
||||||
useEffect(
|
useEffect(
|
||||||
@@ -1487,19 +1492,28 @@ function useKeyBind(setter: (code: string) => void) {
|
|||||||
|
|
||||||
const startListening = useCallback(() => {
|
const startListening = useCallback(() => {
|
||||||
if (listening) return;
|
if (listening) return;
|
||||||
|
setError(null);
|
||||||
setListening(true);
|
setListening(true);
|
||||||
const onKey = (e: KeyboardEvent) => {
|
const onKey = (e: KeyboardEvent) => {
|
||||||
e.preventDefault();
|
e.preventDefault();
|
||||||
if (e.code !== 'Escape') setter(e.code);
|
if (e.code === 'Escape') {
|
||||||
|
// Escape always cancels the rebind without changing the key.
|
||||||
|
} else if (!isBindableCallKey(e.code)) {
|
||||||
|
setError('That key can’t be bound — it’s needed for keyboard navigation.');
|
||||||
|
} else if (otherKey && e.code === otherKey) {
|
||||||
|
setError('That key is already bound to the other call shortcut.');
|
||||||
|
} else {
|
||||||
|
setter(e.code);
|
||||||
|
}
|
||||||
setListening(false);
|
setListening(false);
|
||||||
window.removeEventListener('keydown', onKey, true);
|
window.removeEventListener('keydown', onKey, true);
|
||||||
listenerRef.current = null;
|
listenerRef.current = null;
|
||||||
};
|
};
|
||||||
listenerRef.current = onKey;
|
listenerRef.current = onKey;
|
||||||
window.addEventListener('keydown', onKey, true);
|
window.addEventListener('keydown', onKey, true);
|
||||||
}, [listening, setter]);
|
}, [listening, setter, otherKey]);
|
||||||
|
|
||||||
return { listening, startListening };
|
return { listening, startListening, error };
|
||||||
}
|
}
|
||||||
|
|
||||||
const keyLabel = (code: string) =>
|
const keyLabel = (code: string) =>
|
||||||
@@ -1556,8 +1570,8 @@ function Calls() {
|
|||||||
previewRingtone(value, Math.max(0, Math.min(1, ringtoneVolume / 100)));
|
previewRingtone(value, Math.max(0, Math.min(1, ringtoneVolume / 100)));
|
||||||
};
|
};
|
||||||
|
|
||||||
const pttBind = useKeyBind(setPttKey);
|
const pttBind = useKeyBind(setPttKey, deafenKey);
|
||||||
const deafenBind = useKeyBind(setDeafenKey);
|
const deafenBind = useKeyBind(setDeafenKey, pttKey);
|
||||||
|
|
||||||
const mlSupported = isMLDenoiseSupported();
|
const mlSupported = isMLDenoiseSupported();
|
||||||
const selectedDenoiseModel = DENOISE_MODELS.find((m) => m.id === callDenoiseModel);
|
const selectedDenoiseModel = DENOISE_MODELS.find((m) => m.id === callDenoiseModel);
|
||||||
@@ -1823,7 +1837,7 @@ function Calls() {
|
|||||||
{pttMode && (
|
{pttMode && (
|
||||||
<SettingTile
|
<SettingTile
|
||||||
title="PTT Key"
|
title="PTT Key"
|
||||||
description="Press a key to bind it as your push-to-talk key."
|
description={pttBind.error ?? 'Press a key to bind it as your push-to-talk key.'}
|
||||||
after={
|
after={
|
||||||
<Button
|
<Button
|
||||||
size="300"
|
size="300"
|
||||||
@@ -1841,7 +1855,9 @@ function Calls() {
|
|||||||
)}
|
)}
|
||||||
<SettingTile
|
<SettingTile
|
||||||
title="Push to Deafen"
|
title="Push to Deafen"
|
||||||
description="Toggle speaker mute during a call. Press Escape to cancel rebind."
|
description={
|
||||||
|
deafenBind.error ?? 'Toggle speaker mute during a call. Press Escape to cancel rebind.'
|
||||||
|
}
|
||||||
after={
|
after={
|
||||||
<Button
|
<Button
|
||||||
size="300"
|
size="300"
|
||||||
|
|||||||
@@ -0,0 +1,47 @@
|
|||||||
|
import { test } from 'node:test';
|
||||||
|
import assert from 'node:assert/strict';
|
||||||
|
import { isBindableCallKey } from './callKeybind';
|
||||||
|
|
||||||
|
test('isBindableCallKey rejects navigation-critical codes', () => {
|
||||||
|
[
|
||||||
|
'Escape',
|
||||||
|
'Tab',
|
||||||
|
'Enter',
|
||||||
|
'NumpadEnter',
|
||||||
|
'ArrowUp',
|
||||||
|
'ArrowDown',
|
||||||
|
'ArrowLeft',
|
||||||
|
'ArrowRight',
|
||||||
|
'Home',
|
||||||
|
'End',
|
||||||
|
'PageUp',
|
||||||
|
'PageDown',
|
||||||
|
].forEach((code) => {
|
||||||
|
assert.equal(isBindableCallKey(code), false, `${code} should be unbindable`);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test('isBindableCallKey rejects bare modifier codes', () => {
|
||||||
|
[
|
||||||
|
'ShiftLeft',
|
||||||
|
'ShiftRight',
|
||||||
|
'ControlLeft',
|
||||||
|
'ControlRight',
|
||||||
|
'AltLeft',
|
||||||
|
'AltRight',
|
||||||
|
'MetaLeft',
|
||||||
|
'MetaRight',
|
||||||
|
].forEach((code) => {
|
||||||
|
assert.equal(isBindableCallKey(code), false, `${code} should be unbindable`);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test('isBindableCallKey accepts ordinary keys', () => {
|
||||||
|
['Space', 'KeyM', 'KeyQ', 'Digit1', 'F13'].forEach((code) => {
|
||||||
|
assert.equal(isBindableCallKey(code), true, `${code} should be bindable`);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test('isBindableCallKey rejects the empty string', () => {
|
||||||
|
assert.equal(isBindableCallKey(''), false);
|
||||||
|
});
|
||||||
@@ -0,0 +1,33 @@
|
|||||||
|
/**
|
||||||
|
* `KeyboardEvent.code` values the push-to-talk / push-to-deafen rebind must
|
||||||
|
* never accept. Binding one of these turns it into a keyboard trap (the call
|
||||||
|
* hotkey listener swallows the key everywhere outside an editable field for
|
||||||
|
* the rest of the call) or collides with a bare modifier chord.
|
||||||
|
*/
|
||||||
|
const UNBINDABLE_CALL_KEY_CODES = new Set<string>([
|
||||||
|
'Escape',
|
||||||
|
'Tab',
|
||||||
|
'Enter',
|
||||||
|
'NumpadEnter',
|
||||||
|
'ArrowUp',
|
||||||
|
'ArrowDown',
|
||||||
|
'ArrowLeft',
|
||||||
|
'ArrowRight',
|
||||||
|
'Home',
|
||||||
|
'End',
|
||||||
|
'PageUp',
|
||||||
|
'PageDown',
|
||||||
|
'ShiftLeft',
|
||||||
|
'ShiftRight',
|
||||||
|
'ControlLeft',
|
||||||
|
'ControlRight',
|
||||||
|
'AltLeft',
|
||||||
|
'AltRight',
|
||||||
|
'MetaLeft',
|
||||||
|
'MetaRight',
|
||||||
|
]);
|
||||||
|
|
||||||
|
/** Whether `code` is safe to bind as a call hotkey (PTT / push-to-deafen). */
|
||||||
|
export function isBindableCallKey(code: string): boolean {
|
||||||
|
return code.length > 0 && !UNBINDABLE_CALL_KEY_CODES.has(code);
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user