Bladeren bron

Merge pull request #79 from ckeditor/t/ckeditor5/1463

Fix: Triple clicking inside a nested editable should not select the entire widget in Safari. Closes ckeditor/ckeditor5#1463.
Aleksander Nowodzinski 6 jaren geleden
bovenliggende
commit
511b9f8154

+ 12 - 1
packages/ckeditor5-widget/src/widget.js

@@ -11,6 +11,7 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
 import MouseObserver from '@ckeditor/ckeditor5-engine/src/view/observer/mouseobserver';
 import { getLabel, isWidget, WIDGET_SELECTED_CLASS_NAME } from './utils';
 import { getCode, keyCodes, parseKeystroke } from '@ckeditor/ckeditor5-utils/src/keyboard';
+import env from '@ckeditor/ckeditor5-utils/src/env';
 
 import '../theme/widget.css';
 
@@ -114,8 +115,18 @@ export default class Widget extends Plugin {
 		const viewDocument = view.document;
 		let element = domEventData.target;
 
-		// Do nothing if inside nested editable.
+		// Do nothing for single or double click inside nested editable.
 		if ( isInsideNestedEditable( element ) ) {
+			// But at least triple click inside nested editable causes broken selection in Safari.
+			// For such event, we select the entire nested editable element.
+			// See: https://github.com/ckeditor/ckeditor5/issues/1463.
+			if ( env.isSafari && domEventData.domEvent.detail >= 3 ) {
+				this.editor.editing.view.change( writer => {
+					domEventData.preventDefault();
+					writer.setSelection( element, 'in' );
+				} );
+			}
+
 			return;
 		}
 

+ 232 - 0
packages/ckeditor5-widget/tests/widget-integration.js

@@ -0,0 +1,232 @@
+/**
+ * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+/* global document */
+
+import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor';
+import Typing from '@ckeditor/ckeditor5-typing/src/typing';
+import Widget from '../src/widget';
+import DomEventData from '@ckeditor/ckeditor5-engine/src/view/observer/domeventdata';
+
+import { toWidget } from '../src/utils';
+import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
+import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view';
+
+import env from '@ckeditor/ckeditor5-utils/src/env';
+import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
+
+describe( 'Widget - integration', () => {
+	let editor, model, view, viewDocument, editorElement;
+
+	testUtils.createSinonSandbox();
+
+	beforeEach( () => {
+		// Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`.
+		testUtils.sinon.stub( env, 'isEdge' ).get( () => false );
+		testUtils.sinon.stub( env, 'isSafari' ).get( () => true );
+
+		editorElement = document.createElement( 'div' );
+		document.body.appendChild( editorElement );
+
+		return ClassicEditor.create( editorElement, { plugins: [ Widget, Typing ] } )
+			.then( newEditor => {
+				editor = newEditor;
+				model = editor.model;
+				view = editor.editing.view;
+				viewDocument = view.document;
+
+				model.schema.register( 'widget', {
+					inheritAllFrom: '$block',
+					isObject: true
+				} );
+				model.schema.register( 'paragraph', {
+					inheritAllFrom: '$block',
+					allowIn: '$root'
+				} );
+				model.schema.register( 'nested', {
+					allowIn: 'widget',
+					isLimit: true
+				} );
+				model.schema.extend( '$text', {
+					allowIn: [ 'nested', 'editable', 'inline-widget' ]
+				} );
+				model.schema.register( 'editable', {
+					allowIn: [ 'widget', '$root' ]
+				} );
+				model.schema.register( 'inline-widget', {
+					allowWhere: '$text',
+					isObject: true,
+					isInline: true
+				} );
+
+				editor.conversion.for( 'downcast' )
+					.elementToElement( { model: 'paragraph', view: 'p' } )
+					.elementToElement( { model: 'inline', view: 'figure' } )
+					.elementToElement( { model: 'image', view: 'img' } )
+					.elementToElement( {
+						model: 'widget',
+						view: ( modelItem, viewWriter ) => {
+							const div = viewWriter.createContainerElement( 'div' );
+
+							return toWidget( div, viewWriter, { label: 'element label' } );
+						}
+					} )
+					.elementToElement( {
+						model: 'inline-widget',
+						view: ( modelItem, viewWriter ) => {
+							const span = viewWriter.createContainerElement( 'span' );
+
+							return toWidget( span, viewWriter );
+						}
+					} )
+					.elementToElement( {
+						model: 'nested',
+						view: ( modelItem, viewWriter ) => viewWriter.createEditableElement( 'figcaption', { contenteditable: true } )
+					} )
+					.elementToElement( {
+						model: 'editable',
+						view: ( modelItem, viewWriter ) => viewWriter.createEditableElement( 'figcaption', { contenteditable: true } )
+					} );
+			} );
+	} );
+
+	afterEach( () => {
+		editorElement.remove();
+
+		return editor.destroy();
+	} );
+
+	it( 'should do nothing if clicked inside a nested editable', () => {
+		setModelData( model, '[]<widget><nested>foo bar</nested></widget>' );
+		const viewDiv = viewDocument.getRoot().getChild( 0 );
+		const viewFigcaption = viewDiv.getChild( 0 );
+
+		const preventDefault = sinon.spy();
+
+		const domEventDataMock = new DomEventData( view, {
+			target: view.domConverter.mapViewToDom( viewFigcaption ),
+			preventDefault,
+			detail: 1
+		} );
+
+		viewDocument.fire( 'mousedown', domEventDataMock );
+
+		sinon.assert.notCalled( preventDefault );
+
+		expect( getViewData( view ) ).to.equal(
+			'[]<div class="ck-widget" contenteditable="false"><figcaption contenteditable="true">foo bar</figcaption></div>'
+		);
+	} );
+
+	it( 'should select the entire nested editable if triple clicked', () => {
+		setModelData( model, '[]<widget><nested>foo bar</nested></widget>' );
+
+		const viewDiv = viewDocument.getRoot().getChild( 0 );
+		const viewFigcaption = viewDiv.getChild( 0 );
+		const preventDefault = sinon.spy();
+		const domEventDataMock = new DomEventData( view, {
+			target: view.domConverter.mapViewToDom( viewFigcaption ),
+			preventDefault,
+			detail: 3
+		} );
+
+		viewDocument.fire( 'mousedown', domEventDataMock );
+
+		sinon.assert.called( preventDefault );
+
+		expect( getViewData( view ) ).to.equal(
+			'<div class="ck-widget" contenteditable="false"><figcaption contenteditable="true">[foo bar]</figcaption></div>'
+		);
+	} );
+
+	it( 'should select proper nested editable if triple clicked', () => {
+		setModelData( model, '[]<widget><nested>foo</nested><nested>bar</nested></widget>' );
+
+		const viewDiv = viewDocument.getRoot().getChild( 0 );
+		const secondViewFigcaption = viewDiv.getChild( 1 );
+		const preventDefault = sinon.spy();
+		const domEventDataMock = new DomEventData( view, {
+			target: view.domConverter.mapViewToDom( secondViewFigcaption ),
+			preventDefault,
+			detail: 3
+		} );
+
+		viewDocument.fire( 'mousedown', domEventDataMock );
+
+		sinon.assert.called( preventDefault );
+
+		expect( getViewData( view ) ).to.equal(
+			'<div class="ck-widget" contenteditable="false">' +
+				'<figcaption contenteditable="true">foo</figcaption>' +
+				'<figcaption contenteditable="true">[bar]</figcaption>' +
+			'</div>'
+		);
+	} );
+
+	it( 'should select the entire nested editable if quadra clicked', () => {
+		setModelData( model, '[]<widget><nested>foo bar</nested></widget>' );
+
+		const viewDiv = viewDocument.getRoot().getChild( 0 );
+		const viewFigcaption = viewDiv.getChild( 0 );
+		const preventDefault = sinon.spy();
+		const domEventDataMock = new DomEventData( view, {
+			target: view.domConverter.mapViewToDom( viewFigcaption ),
+			preventDefault,
+			detail: 4
+		} );
+
+		viewDocument.fire( 'mousedown', domEventDataMock );
+
+		sinon.assert.called( preventDefault );
+
+		expect( getViewData( view ) ).to.equal(
+			'<div class="ck-widget" contenteditable="false"><figcaption contenteditable="true">[foo bar]</figcaption></div>'
+		);
+	} );
+
+	it( 'should select the inline widget if triple clicked', () => {
+		setModelData( model, '<paragraph>Foo<inline-widget>foo bar</inline-widget>Bar</paragraph>' );
+
+		const viewParagraph = viewDocument.getRoot().getChild( 0 );
+		const viewInlineWidget = viewParagraph.getChild( 1 );
+		const preventDefault = sinon.spy();
+		const domEventDataMock = new DomEventData( view, {
+			target: view.domConverter.mapViewToDom( viewInlineWidget ),
+			preventDefault,
+			detail: 3
+		} );
+
+		viewDocument.fire( 'mousedown', domEventDataMock );
+
+		sinon.assert.called( preventDefault );
+
+		expect( getViewData( view ) ).to.equal(
+			'<p>Foo{<span class="ck-widget ck-widget_selected" contenteditable="false">foo bar</span>}Bar</p>'
+		);
+	} );
+
+	it( 'should does nothing for non-Safari browser', () => {
+		testUtils.sinon.stub( env, 'isSafari' ).get( () => false );
+
+		setModelData( model, '[]<widget><nested>foo bar</nested></widget>' );
+
+		const viewDiv = viewDocument.getRoot().getChild( 0 );
+		const viewFigcaption = viewDiv.getChild( 0 );
+		const preventDefault = sinon.spy();
+		const domEventDataMock = new DomEventData( view, {
+			target: view.domConverter.mapViewToDom( viewFigcaption ),
+			preventDefault,
+			detail: 4
+		} );
+
+		viewDocument.fire( 'mousedown', domEventDataMock );
+
+		sinon.assert.notCalled( preventDefault );
+
+		expect( getViewData( view ) ).to.equal(
+			'[]<div class="ck-widget" contenteditable="false"><figcaption contenteditable="true">foo bar</figcaption></div>'
+		);
+	} );
+} );

+ 0 - 15
packages/ckeditor5-widget/tests/widget.js

@@ -151,21 +151,6 @@ describe( 'Widget', () => {
 		sinon.assert.calledOnce( domEventDataMock.preventDefault );
 	} );
 
-	it( 'should do nothing if clicked inside nested editable', () => {
-		setModelData( model, '[]<widget><nested>foo bar</nested></widget>' );
-		const viewDiv = viewDocument.getRoot().getChild( 0 );
-		const viewFigcaption = viewDiv.getChild( 0 );
-
-		const domEventDataMock = {
-			target: viewFigcaption,
-			preventDefault: sinon.spy()
-		};
-
-		viewDocument.fire( 'mousedown', domEventDataMock );
-
-		sinon.assert.notCalled( domEventDataMock.preventDefault );
-	} );
-
 	it( 'should do nothing if clicked in non-widget element', () => {
 		setModelData( model, '<paragraph>[]foo bar</paragraph><widget></widget>' );
 		const viewP = viewDocument.getRoot().getChild( 0 );