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); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 864320c94..ecffe34f9 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -130,7 +130,7 @@ importers: version: 14.0.3 postcss: specifier: '8' - version: 8.5.18 + version: 8.5.23 sass: specifier: 1.80.3 version: 1.80.3 @@ -267,7 +267,7 @@ importers: version: 4.1.5 postcss: specifier: '8' - version: 8.5.18 + version: 8.5.23 pretty: specifier: 2.0.0 version: 2.0.0 @@ -2816,8 +2816,8 @@ packages: brace-expansion@2.0.1: resolution: {integrity: sha512-XnAIvQ8eM+kC6aULx6wuQiwVsnzsi9d3WxzV3FpWTGA19F621kwdbsAcFKXgKUHZWsy+mY6iL1sHTxWEFCytDA==} - brace-expansion@2.1.3: - resolution: {integrity: sha512-DRdx5neNsG/QXbniLFWi2YmC/68oeOOmKz6zOjVk6ZS1ZLXgLIKqVEc6hWsmkjBbgii0SwaBTcJ5XKj5gzY/4A==} + brace-expansion@2.1.4: + resolution: {integrity: sha512-hGfVzPxthbf3+2yjg/RBs60cB0FhqBS/zvdV/4wn4/BmN0bNMMHPc4V/BbFieqf1TKAGGAHnY4eSjajCl0f2Xg==} braces@2.3.2: resolution: {integrity: sha512-aNdbnj9P8PjdXU4ybaWLK2IF3jc/EoDYbC7AazW6to3TRsfXxscC9UXOB5iDiEQrkyIbWp2SLQda4+QAa7nc3w==} @@ -6553,8 +6553,8 @@ packages: nan@2.22.0: resolution: {integrity: sha512-nbajikzWTMwsW+eSsNm3QwlOs7het9gGJU5dDZzRTQGk03vyBOauxgI4VakDzE0PtsGTmXPsXTbbjVhRwR5mpw==} - nanoid@3.3.16: - resolution: {integrity: sha512-bzlKTyNJ7+LdGIIwy8ijFpIqEQIvafahV7eYykJ8Cvh42EdJeODoJ6gUJXpQJvej1BddH8OqTXZNE/KfbWAu8Q==} + nanoid@3.3.17: + resolution: {integrity: sha512-xQLf0A3HOMlgHq0n247/LRuAOYmB7dXJ/DvAxGvsSBij45XtBSmQycu+F8ODbHwns/XyFZagyL1+J0Offw1E0g==} engines: {node: ^10 || ^12 || ^13.7 || ^14 || >=15.0.1} hasBin: true @@ -7252,12 +7252,12 @@ packages: resolution: {integrity: sha512-yioayjNbHn6z1/Bywyb2Y4s3yvDAeXGOyxqD+LnVOinq6Mdmd++SW2wUNVzavyyHxd6+DxzWGIuosg6P1Rj8uA==} engines: {node: '>=6.0.0'} - postcss@8.5.18: - resolution: {integrity: sha512-xdB1oSLHbz1vRWgCDalrCqEFTWzFlhqFC5tIHLMOSUIjhm3XXQ1qrFy8S/ESr1JYRRXqM3c1QFiMZUJdUTqyMQ==} + postcss@8.5.23: + resolution: {integrity: sha512-g50586zr4bZmwFiTlflMu8E0bDTb5I5gertgwAKmsdUlTQIhZtunzUlD1WSzwcVWPoAVpsrA6vlfCD7oXvRwgg==} engines: {node: ^10 || ^12 || >=14} - postcss@8.5.25: - resolution: {integrity: sha512-DTPx3RWSSnWyzLxQnlH0rJP+EW5ekl16ZU4/psbIhA0e53kJfdgaN5vKM+xP7yJtXVu+nfdVFmlgFDEKAe4Pyw==} + postcss@8.5.26: + resolution: {integrity: sha512-u82N74LFzG8ca+dD8puPnplTXoGH4fTPpVGuIbt36G3qvNlkvfD0lEAZSxaly3KX8TS/L1A1gsCEmvKmBcVbkQ==} engines: {node: ^10 || ^12 || >=14} prelude-ls@1.2.1: @@ -11670,7 +11670,7 @@ snapshots: '@vue/compiler-sfc@2.7.16': dependencies: '@babel/parser': 7.25.8 - postcss: 8.5.18 + postcss: 8.5.23 source-map: 0.6.1 optionalDependencies: prettier: 2.8.8 @@ -11684,7 +11684,7 @@ snapshots: '@vue/shared': 3.5.12 estree-walker: 2.0.2 magic-string: 0.30.12 - postcss: 8.5.18 + postcss: 8.5.23 source-map-js: 1.2.1 '@vue/compiler-ssr@3.5.12': @@ -12736,7 +12736,7 @@ snapshots: dependencies: balanced-match: 1.0.2 - brace-expansion@2.1.3: + brace-expansion@2.1.4: dependencies: balanced-match: 1.0.2 optional: true @@ -13436,12 +13436,12 @@ snapshots: css-loader@7.1.2(webpack@5.94.0): dependencies: - icss-utils: 5.1.0(postcss@8.5.25) - postcss: 8.5.25 - postcss-modules-extract-imports: 3.1.0(postcss@8.5.25) - postcss-modules-local-by-default: 4.0.5(postcss@8.5.25) - postcss-modules-scope: 3.2.0(postcss@8.5.25) - postcss-modules-values: 4.0.0(postcss@8.5.25) + icss-utils: 5.1.0(postcss@8.5.26) + postcss: 8.5.26 + postcss-modules-extract-imports: 3.1.0(postcss@8.5.26) + postcss-modules-local-by-default: 4.0.5(postcss@8.5.26) + postcss-modules-scope: 3.2.0(postcss@8.5.26) + postcss-modules-values: 4.0.0(postcss@8.5.26) postcss-value-parser: 4.2.0 semver: 7.6.3 optionalDependencies: @@ -15454,9 +15454,9 @@ snapshots: dependencies: postcss: 7.0.39 - icss-utils@5.1.0(postcss@8.5.25): + icss-utils@5.1.0(postcss@8.5.26): dependencies: - postcss: 8.5.25 + postcss: 8.5.26 ieee754@1.2.1: {} @@ -17123,7 +17123,7 @@ snapshots: minimatch@5.1.9: dependencies: - brace-expansion: 2.1.3 + brace-expansion: 2.1.4 optional: true minimatch@8.0.4: @@ -17219,7 +17219,7 @@ snapshots: nan@2.22.0: optional: true - nanoid@3.3.16: {} + nanoid@3.3.17: {} nanomatch@1.2.13(supports-color@6.1.0): dependencies: @@ -17864,9 +17864,9 @@ snapshots: dependencies: postcss: 7.0.39 - postcss-modules-extract-imports@3.1.0(postcss@8.5.25): + postcss-modules-extract-imports@3.1.0(postcss@8.5.26): dependencies: - postcss: 8.5.25 + postcss: 8.5.26 postcss-modules-local-by-default@2.0.6: dependencies: @@ -17874,10 +17874,10 @@ snapshots: postcss-selector-parser: 6.1.2 postcss-value-parser: 3.3.1 - postcss-modules-local-by-default@4.0.5(postcss@8.5.25): + postcss-modules-local-by-default@4.0.5(postcss@8.5.26): dependencies: - icss-utils: 5.1.0(postcss@8.5.25) - postcss: 8.5.25 + icss-utils: 5.1.0(postcss@8.5.26) + postcss: 8.5.26 postcss-selector-parser: 6.1.2 postcss-value-parser: 4.2.0 @@ -17886,9 +17886,9 @@ snapshots: postcss: 7.0.39 postcss-selector-parser: 6.1.2 - postcss-modules-scope@3.2.0(postcss@8.5.25): + postcss-modules-scope@3.2.0(postcss@8.5.26): dependencies: - postcss: 8.5.25 + postcss: 8.5.26 postcss-selector-parser: 6.1.2 postcss-modules-values@2.0.0: @@ -17896,10 +17896,10 @@ snapshots: icss-replace-symbols: 1.1.0 postcss: 7.0.39 - postcss-modules-values@4.0.0(postcss@8.5.25): + postcss-modules-values@4.0.0(postcss@8.5.26): dependencies: - icss-utils: 5.1.0(postcss@8.5.25) - postcss: 8.5.25 + icss-utils: 5.1.0(postcss@8.5.26) + postcss: 8.5.26 postcss-normalize-charset@4.0.1: dependencies: @@ -18011,15 +18011,15 @@ snapshots: picocolors: 0.2.1 source-map: 0.6.1 - postcss@8.5.18: + postcss@8.5.23: dependencies: - nanoid: 3.3.16 + nanoid: 3.3.17 picocolors: 1.1.1 source-map-js: 1.2.1 - postcss@8.5.25: + postcss@8.5.26: dependencies: - nanoid: 3.3.16 + nanoid: 3.3.17 picocolors: 1.1.1 source-map-js: 1.2.1