Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #79 from ckeditor/t/ckeditor5/1463
Browse files Browse the repository at this point in the history 
Fix: Triple clicking inside a nested editable should not select the entire widget in Safari. Closes ckeditor/ckeditor5#1463.
  • Loading branch information
@oleq
oleq committed Mar 28, 2019
2 parents 1acb598 + 7bca9e1 commit b7c4765
Show file tree
Hide file tree
Showing 3 changed files with 244 additions and 16 deletions.
13 changes: 12 additions & 1 deletion src/widget.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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;
}

Expand Down
232 changes: 232 additions & 0 deletions tests/widget-integration.js
View file
Original file line number Diff line number Diff line change
@@ -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>'
);
} );
} );
15 changes: 0 additions & 15 deletions tests/widget.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down

0 comments on commit b7c4765

Please sign in to comment.