diff --git a/packages/core/src/canvas/view/CanvasView.ts b/packages/core/src/canvas/view/CanvasView.ts index 7696aaa7b..6efaff60a 100644 --- a/packages/core/src/canvas/view/CanvasView.ts +++ b/packages/core/src/canvas/view/CanvasView.ts @@ -12,6 +12,7 @@ import { isTextNode, off, on, + preventDefault, } from '../../utils/dom'; import { getComponentView, getElement, getUiClass } from '../../utils/mixins'; import Canvas from '../model/Canvas'; @@ -145,10 +146,7 @@ export default class CanvasView extends ModuleView { } preventDefault(ev: Event) { - if (ev) { - ev.preventDefault(); - (ev as any)._parentEvent?.preventDefault(); - } + preventDefault(ev); } toggleListeners(enable: boolean) { diff --git a/packages/core/src/keymaps/index.ts b/packages/core/src/keymaps/index.ts index 9e5618847..fe10da018 100644 --- a/packages/core/src/keymaps/index.ts +++ b/packages/core/src/keymaps/index.ts @@ -38,6 +38,7 @@ import { isFunction, isString } from 'underscore'; import { Module } from '../abstract'; import EditorModel from '../editor/model/Editor'; import keymaster from '../utils/keymaster'; +import { preventDefault } from '../utils/dom'; import { hasWin } from '../utils/mixins'; import defConfig, { Keymap, KeymapOptions, KeymapsConfig } from './config'; import { KeymapsEvents } from './types'; @@ -99,9 +100,6 @@ export default class KeymapsModule extends Module { // It's safer putting handlers resolution inside the callback + const cmd = em.Commands; + const editor = em.getEditor(); const opt = { event: e, h }; const handlerRes = isString(handler) ? cmd.get(handler) : handler; - const ableTorun = !em.isEditing() && !editor.Canvas.isInputFocused(); + const ableTorun = !em.isEditing() && !em.Canvas.isInputFocused(); if (ableTorun || opts.force) { - opts.prevent && canvas.getCanvasView()?.preventDefault(e); + // Prevent as soon as possible, the default action of the key has to be + // avoided even if the handler is missing or throws. + opts.prevent && preventDefault(e); isFunction(handlerRes) ? handlerRes(editor, 0, opt) : cmd.runCommand(handlerRes, opt); const args = [id, h.shortcut, e]; // @ts-ignore diff --git a/packages/core/src/utils/dom.ts b/packages/core/src/utils/dom.ts index b54a70c81..3585bd652 100644 --- a/packages/core/src/utils/dom.ts +++ b/packages/core/src/utils/dom.ts @@ -21,6 +21,17 @@ export const motionsEv = 'transitionend oTransitionEnd transitionend webkitTrans export const isDoc = (el?: Node): el is Document => el?.nodeType === Node.DOCUMENT_NODE; +/** + * Prevent the default of an event. + * Events coming from the canvas frame are re-dispatched on the main document (see `createCustomEvent`), + * so the original one, kept in `_parentEvent`, has to be prevented as well. + */ +export const preventDefault = (ev?: Event) => { + if (!ev) return; + ev.preventDefault(); + (ev as any)._parentEvent?.preventDefault(); +}; + export const removeEl = (el?: HTMLElement) => { const parent = el && el.parentNode; parent && parent.removeChild(el); diff --git a/packages/core/test/specs/keymaps/index.js b/packages/core/test/specs/keymaps/index.js index 06eab1265..efb85385a 100644 --- a/packages/core/test/specs/keymaps/index.js +++ b/packages/core/test/specs/keymaps/index.js @@ -13,6 +13,11 @@ describe('Keymaps', () => { obj = editor.Keymaps; }); + afterEach(() => { + // Bindings are kept in a module-level registry, shared between editors + obj.removeAll(); + }); + test('Object exists', () => { expect(obj).toBeTruthy(); }); @@ -56,6 +61,51 @@ describe('Keymaps', () => { expect(called).toEqual(1); }); + describe('Prevent option', () => { + const dispatchKey = (props = {}) => { + const keyboardEvent = new KeyboardEvent('keydown', { + keyCode: 83, + which: 83, + ctrlKey: true, + cancelable: true, + bubbles: true, + }); + Object.assign(keyboardEvent, props); + document.dispatchEvent(keyboardEvent); + return keyboardEvent; + }; + + beforeEach(() => { + em.setEditing(0); + }); + + it('Should prevent the default action', () => { + const handler = jest.fn(); + obj.add('test', 'ctrl+s', handler, { prevent: true }); + const event = dispatchKey(); + + expect(handler).toHaveBeenCalled(); + expect(event.defaultPrevented).toBe(true); + }); + + it('Should prevent the default action of the event coming from the frame', () => { + obj.add('test', 'ctrl+s', () => {}, { prevent: true }); + // Events triggered inside the canvas frame are re-dispatched on the main + // document, the original one is kept in `_parentEvent`. + const parentEvent = new KeyboardEvent('keydown', { cancelable: true }); + dispatchKey({ _parentEvent: parentEvent }); + + expect(parentEvent.defaultPrevented).toBe(true); + }); + + it('Should not prevent the default action without the option', () => { + obj.add('test', 'ctrl+s', () => {}); + const event = dispatchKey(); + + expect(event.defaultPrevented).toBe(false); + }); + }); + describe('Given the edit is not on edit mode', () => { beforeEach(() => { em.setEditing(0);