fix: prevent Ctrl+Enter adding extra line break on send (Windows) (#14077)
# Pull Request Template ## Description This PR includes, On Windows, pressing **Ctrl+Enter** in the reply editor was inserting an unintended line break before sending. This led to two issues: * **Unexpected blank lines** After adding a line break with Shift+Enter and removing it with Backspace, the editor looked correct. However, sending with Ctrl+Enter reintroduced a hidden break, resulting in an extra blank line in the final message. * **Selected text being replaced** When text was selected and Ctrl+Enter was pressed, the selection was replaced with a line break instead of being sent. Fixes https://linear.app/chatwoot/issue/CW-6840/newline-bug-in-the-editor ### **Cause** Two keyboard handlers responded to **Ctrl+Enter** on Windows: * ProseMirror (`Mod-Enter`) inserted a hard break * ReplyBox (`$mod+Enter`) triggered send The existing guard only checked `metaKey` (Cmd), so it never worked on Windows. As a result, a line break was inserted just before sending. ### **Solution** Make the modifier check platform-aware so the editor correctly intercepts the send shortcut: * Added `detectOS`, `isMac`, and `OS` constants * Introduced `hasPressedMod` (uses `metaKey` on macOS, `ctrlKey` elsewhere) This ensures Ctrl+Enter sends the message without modifying content, while keeping existing behavior unchanged. **NB:** macOS behavior with Cmd+Enter remains unchanged ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? **Case 1: line break** 1. Type `hello` 2. Press Shift+Enter, then Backspace 3. Press Ctrl+Enter → Message contains an unexpected blank new line **Case 2: Selection replaced** 1. Type two lines using Shift+Enter 2. Select text on the second line 3. Press Ctrl+Enter → Selected text is replaced and not sent ### Screencast **Before** https://github.com/user-attachments/assets/d6d285a9-260b-4711-8bbd-d0c8519e8d20 **After** https://github.com/user-attachments/assets/c0ace1f7-5d22-44a2-8e08-22190ee21e61 ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [x] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules
This commit is contained in:
@@ -1,3 +1,5 @@
|
||||
import { isApple } from './platform';
|
||||
|
||||
export const isEnter = e => {
|
||||
return e.key === 'Enter';
|
||||
};
|
||||
@@ -14,13 +16,20 @@ export const hasPressedCommand = e => {
|
||||
return e.metaKey;
|
||||
};
|
||||
|
||||
// True when the platform's "command" modifier is held: Cmd (metaKey) on
|
||||
// Apple platforms (macOS, iOS/iPadOS hardware keyboards), Ctrl (ctrlKey)
|
||||
// elsewhere. Mirrors the `$mod` convention used by tinykeys and
|
||||
// prosemirror-keymap so the editor and the app agree on what counts as the
|
||||
// send modifier.
|
||||
export const hasPressedMod = e => Boolean(isApple() ? e.metaKey : e.ctrlKey);
|
||||
|
||||
export const hasPressedEnterAndNotCmdOrShift = e => {
|
||||
return isEnter(e) && !hasPressedCommand(e) && !hasPressedShift(e);
|
||||
return isEnter(e) && !hasPressedMod(e) && !hasPressedShift(e);
|
||||
};
|
||||
|
||||
export const hasPressedCommandAndEnter = e => {
|
||||
return hasPressedCommand(e) && isEnter(e);
|
||||
};
|
||||
// Detects the platform-aware "send" shortcut: Cmd+Enter on Apple platforms,
|
||||
// Ctrl+Enter on Windows/Linux.
|
||||
export const hasPressedCommandAndEnter = e => hasPressedMod(e) && isEnter(e);
|
||||
|
||||
// If layout is QWERTZ then we add the Shift+keysToModify to fix an known issue
|
||||
// https://github.com/chatwoot/chatwoot/issues/9492
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
// Detects the current OS using the modern User-Agent Client Hints API,
|
||||
// falling back to userAgent parsing on Safari/Firefox where it is unavailable.
|
||||
// Treats iPad on iOS 13+ (which spoofs Macintosh) as iOS via maxTouchPoints.
|
||||
|
||||
export const OS = Object.freeze({
|
||||
MAC: 'macos',
|
||||
WINDOWS: 'windows',
|
||||
LINUX: 'linux',
|
||||
ANDROID: 'android',
|
||||
IOS: 'ios',
|
||||
UNKNOWN: 'unknown',
|
||||
});
|
||||
|
||||
// navigator.userAgentData.platform → OS constant (lowercased keys)
|
||||
const UAD_MAP = {
|
||||
macos: OS.MAC,
|
||||
windows: OS.WINDOWS,
|
||||
linux: OS.LINUX,
|
||||
android: OS.ANDROID,
|
||||
ios: OS.IOS,
|
||||
};
|
||||
|
||||
export function detectOS() {
|
||||
if (typeof navigator === 'undefined') return OS.UNKNOWN;
|
||||
|
||||
// Trust userAgentData only when it maps to a known OS; otherwise fall
|
||||
// through to UA parsing so unmapped values (e.g. "Chrome OS") don't leak.
|
||||
const uad = navigator.userAgentData?.platform?.toLowerCase();
|
||||
if (uad && UAD_MAP[uad]) return UAD_MAP[uad];
|
||||
|
||||
const ua = navigator.userAgent || '';
|
||||
if (/android/i.test(ua)) return OS.ANDROID;
|
||||
if (/iPhone|iPod/.test(ua)) return OS.IOS;
|
||||
if (
|
||||
/iPad/.test(ua) ||
|
||||
(/Macintosh/.test(ua) && (navigator.maxTouchPoints || 0) > 1)
|
||||
) {
|
||||
return OS.IOS;
|
||||
}
|
||||
if (/Win/i.test(ua)) return OS.WINDOWS;
|
||||
if (/Mac/i.test(ua)) return OS.MAC;
|
||||
if (/Linux/i.test(ua)) return OS.LINUX;
|
||||
|
||||
return OS.UNKNOWN;
|
||||
}
|
||||
|
||||
export const isApple = () => {
|
||||
const os = detectOS();
|
||||
return os === OS.MAC || os === OS.IOS;
|
||||
};
|
||||
@@ -3,9 +3,29 @@ import {
|
||||
isEscape,
|
||||
hasPressedShift,
|
||||
hasPressedCommand,
|
||||
hasPressedMod,
|
||||
hasPressedCommandAndEnter,
|
||||
hasPressedEnterAndNotCmdOrShift,
|
||||
isActiveElementTypeable,
|
||||
} from '../KeyboardHelpers';
|
||||
|
||||
const setNavigator = navigatorValue => {
|
||||
Object.defineProperty(global, 'navigator', {
|
||||
value: navigatorValue,
|
||||
configurable: true,
|
||||
writable: true,
|
||||
});
|
||||
};
|
||||
|
||||
const onMac = () => setNavigator({ userAgentData: { platform: 'macOS' } });
|
||||
const onWindows = () =>
|
||||
setNavigator({ userAgentData: { platform: 'Windows' } });
|
||||
const onIOS = () =>
|
||||
setNavigator({
|
||||
userAgent:
|
||||
'Mozilla/5.0 (iPhone; CPU iPhone OS 16_0 like Mac OS X) AppleWebKit/605.1.15',
|
||||
});
|
||||
|
||||
describe('#KeyboardHelpers', () => {
|
||||
describe('#isEnter', () => {
|
||||
it('return correct values', () => {
|
||||
@@ -30,6 +50,112 @@ describe('#KeyboardHelpers', () => {
|
||||
expect(hasPressedCommand({ metaKey: true })).toEqual(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('#hasPressedMod', () => {
|
||||
const originalNavigator = global.navigator;
|
||||
|
||||
afterEach(() => {
|
||||
setNavigator(originalNavigator);
|
||||
});
|
||||
|
||||
it('uses metaKey on macOS', () => {
|
||||
onMac();
|
||||
expect(hasPressedMod({ metaKey: true, ctrlKey: false })).toBe(true);
|
||||
expect(hasPressedMod({ metaKey: false, ctrlKey: true })).toBe(false);
|
||||
});
|
||||
|
||||
it('uses ctrlKey on Windows', () => {
|
||||
onWindows();
|
||||
expect(hasPressedMod({ metaKey: false, ctrlKey: true })).toBe(true);
|
||||
expect(hasPressedMod({ metaKey: true, ctrlKey: false })).toBe(false);
|
||||
});
|
||||
|
||||
it('uses metaKey on iOS hardware keyboards', () => {
|
||||
onIOS();
|
||||
expect(hasPressedMod({ metaKey: true, ctrlKey: false })).toBe(true);
|
||||
expect(hasPressedMod({ metaKey: false, ctrlKey: true })).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false when no modifier is held', () => {
|
||||
onWindows();
|
||||
expect(hasPressedMod({ metaKey: false, ctrlKey: false })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('#hasPressedCommandAndEnter', () => {
|
||||
const originalNavigator = global.navigator;
|
||||
|
||||
afterEach(() => {
|
||||
setNavigator(originalNavigator);
|
||||
});
|
||||
|
||||
it('returns true for Cmd+Enter on macOS', () => {
|
||||
onMac();
|
||||
expect(hasPressedCommandAndEnter({ key: 'Enter', metaKey: true })).toBe(
|
||||
true
|
||||
);
|
||||
});
|
||||
|
||||
it('returns true for Ctrl+Enter on Windows (CW-6859 fix)', () => {
|
||||
onWindows();
|
||||
expect(hasPressedCommandAndEnter({ key: 'Enter', ctrlKey: true })).toBe(
|
||||
true
|
||||
);
|
||||
});
|
||||
|
||||
it('returns false for Ctrl+Enter on macOS (Mac uses Cmd, not Ctrl)', () => {
|
||||
onMac();
|
||||
expect(hasPressedCommandAndEnter({ key: 'Enter', ctrlKey: true })).toBe(
|
||||
false
|
||||
);
|
||||
});
|
||||
|
||||
it('returns true for Cmd+Enter on iOS hardware keyboards', () => {
|
||||
onIOS();
|
||||
expect(hasPressedCommandAndEnter({ key: 'Enter', metaKey: true })).toBe(
|
||||
true
|
||||
);
|
||||
});
|
||||
|
||||
it('returns false for plain Enter', () => {
|
||||
onWindows();
|
||||
expect(hasPressedCommandAndEnter({ key: 'Enter' })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('#hasPressedEnterAndNotCmdOrShift', () => {
|
||||
const originalNavigator = global.navigator;
|
||||
|
||||
afterEach(() => {
|
||||
setNavigator(originalNavigator);
|
||||
});
|
||||
|
||||
it('returns true for plain Enter on Windows', () => {
|
||||
onWindows();
|
||||
expect(hasPressedEnterAndNotCmdOrShift({ key: 'Enter' })).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false for Ctrl+Enter on Windows (mod is held)', () => {
|
||||
onWindows();
|
||||
expect(
|
||||
hasPressedEnterAndNotCmdOrShift({ key: 'Enter', ctrlKey: true })
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for Cmd+Enter on macOS (mod is held)', () => {
|
||||
onMac();
|
||||
expect(
|
||||
hasPressedEnterAndNotCmdOrShift({ key: 'Enter', metaKey: true })
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for Shift+Enter', () => {
|
||||
onWindows();
|
||||
expect(
|
||||
hasPressedEnterAndNotCmdOrShift({ key: 'Enter', shiftKey: true })
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('isActiveElementTypeable', () => {
|
||||
|
||||
@@ -0,0 +1,186 @@
|
||||
import { detectOS, isApple, OS } from '../platform';
|
||||
|
||||
const setNavigator = ({ userAgentData, userAgent, maxTouchPoints } = {}) => {
|
||||
Object.defineProperty(global, 'navigator', {
|
||||
value: { userAgentData, userAgent, maxTouchPoints },
|
||||
configurable: true,
|
||||
writable: true,
|
||||
});
|
||||
};
|
||||
|
||||
describe('detectOS', () => {
|
||||
const originalNavigator = global.navigator;
|
||||
|
||||
afterEach(() => {
|
||||
Object.defineProperty(global, 'navigator', {
|
||||
value: originalNavigator,
|
||||
configurable: true,
|
||||
writable: true,
|
||||
});
|
||||
});
|
||||
|
||||
describe('with userAgentData available', () => {
|
||||
it('returns OS.MAC for macOS', () => {
|
||||
setNavigator({ userAgentData: { platform: 'macOS' } });
|
||||
expect(detectOS()).toBe(OS.MAC);
|
||||
});
|
||||
|
||||
it('returns OS.WINDOWS for Windows', () => {
|
||||
setNavigator({ userAgentData: { platform: 'Windows' } });
|
||||
expect(detectOS()).toBe(OS.WINDOWS);
|
||||
});
|
||||
|
||||
it('returns OS.LINUX for Linux', () => {
|
||||
setNavigator({ userAgentData: { platform: 'Linux' } });
|
||||
expect(detectOS()).toBe(OS.LINUX);
|
||||
});
|
||||
|
||||
it('returns OS.ANDROID for Android', () => {
|
||||
setNavigator({ userAgentData: { platform: 'Android' } });
|
||||
expect(detectOS()).toBe(OS.ANDROID);
|
||||
});
|
||||
|
||||
it('falls through to userAgent for unmapped values like "Chrome OS"', () => {
|
||||
setNavigator({
|
||||
userAgentData: { platform: 'Chrome OS' },
|
||||
userAgent: 'Mozilla/5.0 (X11; CrOS x86_64) AppleWebKit/537.36',
|
||||
});
|
||||
// Not a mapped UAD value AND not a recognized UA pattern → unknown
|
||||
expect(detectOS()).toBe(OS.UNKNOWN);
|
||||
});
|
||||
|
||||
it('prefers userAgentData over userAgent when value is mapped', () => {
|
||||
setNavigator({
|
||||
userAgentData: { platform: 'Windows' },
|
||||
userAgent: 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7)',
|
||||
});
|
||||
expect(detectOS()).toBe(OS.WINDOWS);
|
||||
});
|
||||
});
|
||||
|
||||
describe('with userAgent fallback', () => {
|
||||
it('detects macOS from Safari userAgent', () => {
|
||||
setNavigator({
|
||||
userAgent:
|
||||
'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15',
|
||||
});
|
||||
expect(detectOS()).toBe(OS.MAC);
|
||||
});
|
||||
|
||||
it('detects Windows from userAgent', () => {
|
||||
setNavigator({
|
||||
userAgent:
|
||||
'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36',
|
||||
});
|
||||
expect(detectOS()).toBe(OS.WINDOWS);
|
||||
});
|
||||
|
||||
it('detects Linux from userAgent', () => {
|
||||
setNavigator({
|
||||
userAgent: 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36',
|
||||
});
|
||||
expect(detectOS()).toBe(OS.LINUX);
|
||||
});
|
||||
|
||||
it('detects Android from userAgent (before Linux match)', () => {
|
||||
setNavigator({
|
||||
userAgent:
|
||||
'Mozilla/5.0 (Linux; Android 13; Pixel 7) AppleWebKit/537.36',
|
||||
});
|
||||
expect(detectOS()).toBe(OS.ANDROID);
|
||||
});
|
||||
|
||||
it('detects iOS from iPhone userAgent', () => {
|
||||
setNavigator({
|
||||
userAgent:
|
||||
'Mozilla/5.0 (iPhone; CPU iPhone OS 16_0 like Mac OS X) AppleWebKit/605.1.15',
|
||||
});
|
||||
expect(detectOS()).toBe(OS.IOS);
|
||||
});
|
||||
|
||||
it('detects iPadOS spoofing Macintosh via maxTouchPoints', () => {
|
||||
setNavigator({
|
||||
userAgent:
|
||||
'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15',
|
||||
maxTouchPoints: 5,
|
||||
});
|
||||
expect(detectOS()).toBe(OS.IOS);
|
||||
});
|
||||
|
||||
it('returns OS.UNKNOWN when no match', () => {
|
||||
setNavigator({ userAgent: 'SomeRandomBot/1.0' });
|
||||
expect(detectOS()).toBe(OS.UNKNOWN);
|
||||
});
|
||||
|
||||
it('returns OS.UNKNOWN when userAgent is missing', () => {
|
||||
setNavigator({});
|
||||
expect(detectOS()).toBe(OS.UNKNOWN);
|
||||
});
|
||||
});
|
||||
|
||||
describe('without navigator', () => {
|
||||
it('returns OS.UNKNOWN when navigator is undefined', () => {
|
||||
Object.defineProperty(global, 'navigator', {
|
||||
value: undefined,
|
||||
configurable: true,
|
||||
writable: true,
|
||||
});
|
||||
expect(detectOS()).toBe(OS.UNKNOWN);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('isApple', () => {
|
||||
const originalNavigator = global.navigator;
|
||||
|
||||
afterEach(() => {
|
||||
Object.defineProperty(global, 'navigator', {
|
||||
value: originalNavigator,
|
||||
configurable: true,
|
||||
writable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('returns true on macOS', () => {
|
||||
setNavigator({ userAgentData: { platform: 'macOS' } });
|
||||
expect(isApple()).toBe(true);
|
||||
});
|
||||
|
||||
it('returns true on iOS (iPhone)', () => {
|
||||
setNavigator({
|
||||
userAgent:
|
||||
'Mozilla/5.0 (iPhone; CPU iPhone OS 16_0 like Mac OS X) AppleWebKit/605.1.15',
|
||||
});
|
||||
expect(isApple()).toBe(true);
|
||||
});
|
||||
|
||||
it('returns true on iPadOS spoofing Macintosh', () => {
|
||||
setNavigator({
|
||||
userAgent:
|
||||
'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15',
|
||||
maxTouchPoints: 5,
|
||||
});
|
||||
expect(isApple()).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false on Windows', () => {
|
||||
setNavigator({ userAgentData: { platform: 'Windows' } });
|
||||
expect(isApple()).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false on Linux', () => {
|
||||
setNavigator({ userAgentData: { platform: 'Linux' } });
|
||||
expect(isApple()).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false on Android', () => {
|
||||
setNavigator({ userAgentData: { platform: 'Android' } });
|
||||
expect(isApple()).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('OS constants', () => {
|
||||
it('is frozen so callers cannot mutate it', () => {
|
||||
expect(Object.isFrozen(OS)).toBe(true);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user