From e1d0a5ca2e569e6c3df5b6b50f8415e0fc4fda41 Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Fri, 3 Jul 2020 14:39:17 +0200 Subject: [PATCH 1/4] Resizing will not trigger other viewDocument#mousedown events. --- packages/ckeditor5-widget/src/widgetresize.js | 16 ++++-- .../tests/widgetresize-integration.js | 54 +++++++++++++++++++ .../ckeditor5-widget/tests/widgetresize.js | 12 ++++- .../tests/widgetresize/_utils/utils.js | 6 +-- 4 files changed, 79 insertions(+), 9 deletions(-) create mode 100644 packages/ckeditor5-widget/tests/widgetresize-integration.js diff --git a/packages/ckeditor5-widget/src/widgetresize.js b/packages/ckeditor5-widget/src/widgetresize.js index 41878ca9bce..76ce334ac05 100644 --- a/packages/ckeditor5-widget/src/widgetresize.js +++ b/packages/ckeditor5-widget/src/widgetresize.js @@ -12,6 +12,7 @@ import Resizer from './widgetresize/resizer'; import DomEmitterMixin from '@ckeditor/ckeditor5-utils/src/dom/emittermixin'; import global from '@ckeditor/ckeditor5-utils/src/dom/global'; import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin'; +import MouseObserver from '@ckeditor/ckeditor5-engine/src/view/observer/mouseobserver'; import mix from '@ckeditor/ckeditor5-utils/src/mix'; import { throttle } from 'lodash-es'; @@ -71,12 +72,13 @@ export default class WidgetResize extends Plugin { isFormatting: true } ); + this.editor.editing.view.addObserver( MouseObserver ); + this._observer = Object.create( DomEmitterMixin ); - this._observer.listenTo( domDocument, 'mousedown', this._mouseDownListener.bind( this ) ); + this.listenTo( this.editor.editing.view.document, 'mousedown', this._mouseDownListener.bind( this ), { priority: 'high' } ); this._observer.listenTo( domDocument, 'mousemove', this._mouseMoveListener.bind( this ) ); - this._observer.listenTo( domDocument, 'mouseup', this._mouseUpListener.bind( this ) ); const redrawFocusedResizer = () => { @@ -182,13 +184,19 @@ export default class WidgetResize extends Plugin { * @param {Event} domEventData Native DOM event. */ _mouseDownListener( event, domEventData ) { - if ( !Resizer.isResizeHandle( domEventData.target ) ) { + const resizeHandle = domEventData.domTarget; + + if ( !Resizer.isResizeHandle( resizeHandle ) ) { return; } - const resizeHandle = domEventData.target; + this._activeResizer = this._getResizerByHandle( resizeHandle ); + if ( this._activeResizer ) { this._activeResizer.begin( resizeHandle ); + + // Do not call other events when resizing. See: #6755. + event.stop(); } } diff --git a/packages/ckeditor5-widget/tests/widgetresize-integration.js b/packages/ckeditor5-widget/tests/widgetresize-integration.js new file mode 100644 index 00000000000..fb3b1c824fd --- /dev/null +++ b/packages/ckeditor5-widget/tests/widgetresize-integration.js @@ -0,0 +1,54 @@ +/** + * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/* global document */ + +import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'; + +import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; +import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; +import Image from '@ckeditor/ckeditor5-image/src/image'; +import ImageResize from '@ckeditor/ckeditor5-image/src/imageresize'; + +describe( 'WidgetResize - integration', () => { + let editor, model, view, viewDocument, editorElement; + + testUtils.createSinonSandbox(); + + beforeEach( () => { + editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); + + return ClassicEditor.create( editorElement, { plugins: [ Image, ImageResize ] } ) + .then( newEditor => { + editor = newEditor; + model = editor.model; + view = editor.editing.view; + viewDocument = view.document; + } ); + } ); + + afterEach( () => { + editorElement.remove(); + + return editor.destroy(); + } ); + + it( 'should not fire viewDocument#mousedown events after starting resizing', () => { + const eventSpy = sinon.spy().named( 'ViewDocument#mousedown' ); + + setModelData( model, '[]' ); + + const resizeSquareUI = [ ...viewDocument.getRoot().getChild( 0 ).getChildren() ] + .find( element => element.hasClass( 'ck-widget__resizer' ) ); + + const squareDomElement = view.domConverter.mapViewToDom( resizeSquareUI ).querySelector( '.ck-widget__resizer__handle-top-left' ); + + viewDocument.on( 'mousedown', eventSpy ); + viewDocument.fire( 'mousedown', { domTarget: squareDomElement } ); + + expect( eventSpy.called ).to.equal( false ); + } ); +} ); diff --git a/packages/ckeditor5-widget/tests/widgetresize.js b/packages/ckeditor5-widget/tests/widgetresize.js index cf039a8bb12..ce7bb1cb272 100644 --- a/packages/ckeditor5-widget/tests/widgetresize.js +++ b/packages/ckeditor5-widget/tests/widgetresize.js @@ -72,7 +72,7 @@ describe( 'WidgetResize', () => { const unrelatedElement = document.createElement( 'div' ); editor.plugins.get( WidgetResize )._mouseDownListener( {}, { - target: unrelatedElement + domTarget: unrelatedElement } ); } ); @@ -114,6 +114,16 @@ describe( 'WidgetResize', () => { resizerMouseSimulator.dragTo( editor, domParts.resizeHandle, initialPointerPosition ); // No exception should be thrown. } ); + + it( 'stops the event after starting resizing', () => { + const stopSpy = sinon.spy().named( 'stop' ); + + const domParts = getWidgetDomParts( editor, widget, 'top-right' ); + + resizerMouseSimulator.down( editor, domParts.resizeHandle, stopSpy ); + + expect( stopSpy.called ).to.be.equal( true ); + } ); } ); describe( 'visibility', () => { diff --git a/packages/ckeditor5-widget/tests/widgetresize/_utils/utils.js b/packages/ckeditor5-widget/tests/widgetresize/_utils/utils.js index a3fce10a0c1..19fe379d2ac 100644 --- a/packages/ckeditor5-widget/tests/widgetresize/_utils/utils.js +++ b/packages/ckeditor5-widget/tests/widgetresize/_utils/utils.js @@ -8,10 +8,8 @@ import WidgetResize from '../../../src/widgetresize'; import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect'; export const resizerMouseSimulator = { - down( editor, domTarget ) { - this._getPlugin( editor )._mouseDownListener( {}, { - target: domTarget - } ); + down( editor, domTarget, stop = sinon.spy().named( 'stop' ) ) { + this._getPlugin( editor )._mouseDownListener( { stop }, { domTarget } ); }, /** From b2a0a5f2c94d765682bc6f335435b9e616258acd Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Mon, 6 Jul 2020 10:19:35 +0200 Subject: [PATCH 2/4] Changed priorites for events. --- packages/ckeditor5-table/src/tablemouse.js | 10 +++++----- packages/ckeditor5-widget/src/widgetresize.js | 2 +- .../ckeditor5-widget/tests/widgetresize-integration.js | 5 +++-- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/packages/ckeditor5-table/src/tablemouse.js b/packages/ckeditor5-table/src/tablemouse.js index 94d5f783c8a..902e44d196c 100644 --- a/packages/ckeditor5-table/src/tablemouse.js +++ b/packages/ckeditor5-table/src/tablemouse.js @@ -85,11 +85,11 @@ export default class TableMouse extends Plugin { domEventData.preventDefault(); } - } ); + }, { priority: 'low' } ); this.listenTo( editor.editing.view.document, 'mouseup', () => { blockSelectionChange = false; - } ); + }, { priority: 'low' } ); // We need to ignore a `selectionChange` event that is fired after we render our new table cells selection. // When downcasting table cells selection to the view, we put the view selection in the last selected cell @@ -145,7 +145,7 @@ export default class TableMouse extends Plugin { } anchorCell = this._getModelTableCellFromDomEvent( domEventData ); - } ); + }, { priority: 'low' } ); this.listenTo( editor.editing.view.document, 'mousemove', ( evt, domEventData ) => { if ( !domEventData.domEvent.buttons ) { @@ -177,14 +177,14 @@ export default class TableMouse extends Plugin { tableSelection.setCellSelection( anchorCell, targetCell ); domEventData.preventDefault(); - } ); + }, { priority: 'low' } ); this.listenTo( editor.editing.view.document, 'mouseup', () => { beganCellSelection = false; blockSelectionChange = false; anchorCell = null; targetCell = null; - } ); + }, { priority: 'low' } ); // See the explanation in `_enableShiftClickSelection()`. this.listenTo( editor.editing.view.document, 'selectionChange', evt => { diff --git a/packages/ckeditor5-widget/src/widgetresize.js b/packages/ckeditor5-widget/src/widgetresize.js index 76ce334ac05..9256d063a5b 100644 --- a/packages/ckeditor5-widget/src/widgetresize.js +++ b/packages/ckeditor5-widget/src/widgetresize.js @@ -76,7 +76,7 @@ export default class WidgetResize extends Plugin { this._observer = Object.create( DomEmitterMixin ); - this.listenTo( this.editor.editing.view.document, 'mousedown', this._mouseDownListener.bind( this ), { priority: 'high' } ); + this.listenTo( this.editor.editing.view.document, 'mousedown', this._mouseDownListener.bind( this ) ); this._observer.listenTo( domDocument, 'mousemove', this._mouseMoveListener.bind( this ) ); this._observer.listenTo( domDocument, 'mouseup', this._mouseUpListener.bind( this ) ); diff --git a/packages/ckeditor5-widget/tests/widgetresize-integration.js b/packages/ckeditor5-widget/tests/widgetresize-integration.js index fb3b1c824fd..6b52d006411 100644 --- a/packages/ckeditor5-widget/tests/widgetresize-integration.js +++ b/packages/ckeditor5-widget/tests/widgetresize-integration.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -/* global document */ +/* global document, Event */ import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'; @@ -47,7 +47,8 @@ describe( 'WidgetResize - integration', () => { const squareDomElement = view.domConverter.mapViewToDom( resizeSquareUI ).querySelector( '.ck-widget__resizer__handle-top-left' ); viewDocument.on( 'mousedown', eventSpy ); - viewDocument.fire( 'mousedown', { domTarget: squareDomElement } ); + + squareDomElement.dispatchEvent( new Event( 'mousedown' ) ); expect( eventSpy.called ).to.equal( false ); } ); From 8a5751767b06a039c74ac976a97a3e20f6d5f38f Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Mon, 6 Jul 2020 12:40:22 +0200 Subject: [PATCH 3/4] After stopping the event, it should also block its default action. --- packages/ckeditor5-table/src/tablemouse.js | 10 +++++----- packages/ckeditor5-widget/src/widgetresize.js | 3 ++- .../ckeditor5-widget/src/widgetresize/resizer.js | 3 ++- packages/ckeditor5-widget/tests/widgetresize.js | 15 +++++++++++++-- .../tests/widgetresize/_utils/utils.js | 7 +++++-- 5 files changed, 27 insertions(+), 11 deletions(-) diff --git a/packages/ckeditor5-table/src/tablemouse.js b/packages/ckeditor5-table/src/tablemouse.js index 902e44d196c..94d5f783c8a 100644 --- a/packages/ckeditor5-table/src/tablemouse.js +++ b/packages/ckeditor5-table/src/tablemouse.js @@ -85,11 +85,11 @@ export default class TableMouse extends Plugin { domEventData.preventDefault(); } - }, { priority: 'low' } ); + } ); this.listenTo( editor.editing.view.document, 'mouseup', () => { blockSelectionChange = false; - }, { priority: 'low' } ); + } ); // We need to ignore a `selectionChange` event that is fired after we render our new table cells selection. // When downcasting table cells selection to the view, we put the view selection in the last selected cell @@ -145,7 +145,7 @@ export default class TableMouse extends Plugin { } anchorCell = this._getModelTableCellFromDomEvent( domEventData ); - }, { priority: 'low' } ); + } ); this.listenTo( editor.editing.view.document, 'mousemove', ( evt, domEventData ) => { if ( !domEventData.domEvent.buttons ) { @@ -177,14 +177,14 @@ export default class TableMouse extends Plugin { tableSelection.setCellSelection( anchorCell, targetCell ); domEventData.preventDefault(); - }, { priority: 'low' } ); + } ); this.listenTo( editor.editing.view.document, 'mouseup', () => { beganCellSelection = false; blockSelectionChange = false; anchorCell = null; targetCell = null; - }, { priority: 'low' } ); + } ); // See the explanation in `_enableShiftClickSelection()`. this.listenTo( editor.editing.view.document, 'selectionChange', evt => { diff --git a/packages/ckeditor5-widget/src/widgetresize.js b/packages/ckeditor5-widget/src/widgetresize.js index 9256d063a5b..43fa74ec04e 100644 --- a/packages/ckeditor5-widget/src/widgetresize.js +++ b/packages/ckeditor5-widget/src/widgetresize.js @@ -76,7 +76,7 @@ export default class WidgetResize extends Plugin { this._observer = Object.create( DomEmitterMixin ); - this.listenTo( this.editor.editing.view.document, 'mousedown', this._mouseDownListener.bind( this ) ); + this.listenTo( this.editor.editing.view.document, 'mousedown', this._mouseDownListener.bind( this ), { priority: 'high' } ); this._observer.listenTo( domDocument, 'mousemove', this._mouseMoveListener.bind( this ) ); this._observer.listenTo( domDocument, 'mouseup', this._mouseUpListener.bind( this ) ); @@ -197,6 +197,7 @@ export default class WidgetResize extends Plugin { // Do not call other events when resizing. See: #6755. event.stop(); + domEventData.preventDefault(); } } diff --git a/packages/ckeditor5-widget/src/widgetresize/resizer.js b/packages/ckeditor5-widget/src/widgetresize/resizer.js index 8999e88cd94..9ed23f80def 100644 --- a/packages/ckeditor5-widget/src/widgetresize/resizer.js +++ b/packages/ckeditor5-widget/src/widgetresize/resizer.js @@ -107,7 +107,8 @@ export default class Resizer { editingView.change( writer => { const viewResizerWrapper = writer.createUIElement( 'div', { - class: 'ck ck-reset_all ck-widget__resizer' + class: 'ck ck-reset_all ck-widget__resizer', + 'data-cke-ignore-events': true }, function( domDocument ) { const domElement = this.toDomElement( domDocument ); diff --git a/packages/ckeditor5-widget/tests/widgetresize.js b/packages/ckeditor5-widget/tests/widgetresize.js index ce7bb1cb272..899e272f6b8 100644 --- a/packages/ckeditor5-widget/tests/widgetresize.js +++ b/packages/ckeditor5-widget/tests/widgetresize.js @@ -72,7 +72,8 @@ describe( 'WidgetResize', () => { const unrelatedElement = document.createElement( 'div' ); editor.plugins.get( WidgetResize )._mouseDownListener( {}, { - domTarget: unrelatedElement + domTarget: unrelatedElement, + preventDefault: sinon.spy() } ); } ); @@ -120,10 +121,20 @@ describe( 'WidgetResize', () => { const domParts = getWidgetDomParts( editor, widget, 'top-right' ); - resizerMouseSimulator.down( editor, domParts.resizeHandle, stopSpy ); + resizerMouseSimulator.down( editor, domParts.resizeHandle, { stop: stopSpy } ); expect( stopSpy.called ).to.be.equal( true ); } ); + + it( 'prevents default action after starting resizing', () => { + const preventDefaultSpy = sinon.spy().named( 'preventDefault' ); + + const domParts = getWidgetDomParts( editor, widget, 'top-right' ); + + resizerMouseSimulator.down( editor, domParts.resizeHandle, { preventDefault: preventDefaultSpy } ); + + expect( preventDefaultSpy.called ).to.be.equal( true ); + } ); } ); describe( 'visibility', () => { diff --git a/packages/ckeditor5-widget/tests/widgetresize/_utils/utils.js b/packages/ckeditor5-widget/tests/widgetresize/_utils/utils.js index 19fe379d2ac..9b9d62248da 100644 --- a/packages/ckeditor5-widget/tests/widgetresize/_utils/utils.js +++ b/packages/ckeditor5-widget/tests/widgetresize/_utils/utils.js @@ -8,8 +8,11 @@ import WidgetResize from '../../../src/widgetresize'; import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect'; export const resizerMouseSimulator = { - down( editor, domTarget, stop = sinon.spy().named( 'stop' ) ) { - this._getPlugin( editor )._mouseDownListener( { stop }, { domTarget } ); + down( editor, domTarget, options = {} ) { + const preventDefault = options.preventDefault || sinon.spy().named( 'preventDefault' ); + const stop = options.stop || sinon.spy().named( 'stop' ); + + this._getPlugin( editor )._mouseDownListener( { stop }, { domTarget, preventDefault } ); }, /** From df7076685d9dfc8c0e1530182da472a113677cd0 Mon Sep 17 00:00:00 2001 From: Maciej Date: Mon, 6 Jul 2020 13:47:31 +0200 Subject: [PATCH 4/4] Remove data-cke-ignore-events from a resizer element. --- packages/ckeditor5-widget/src/widgetresize/resizer.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/ckeditor5-widget/src/widgetresize/resizer.js b/packages/ckeditor5-widget/src/widgetresize/resizer.js index 9ed23f80def..8999e88cd94 100644 --- a/packages/ckeditor5-widget/src/widgetresize/resizer.js +++ b/packages/ckeditor5-widget/src/widgetresize/resizer.js @@ -107,8 +107,7 @@ export default class Resizer { editingView.change( writer => { const viewResizerWrapper = writer.createUIElement( 'div', { - class: 'ck ck-reset_all ck-widget__resizer', - 'data-cke-ignore-events': true + class: 'ck ck-reset_all ck-widget__resizer' }, function( domDocument ) { const domElement = this.toDomElement( domDocument );