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

Commit

Permalink
Tests: Provided missing tests for Android typing.
Browse files Browse the repository at this point in the history
  • Loading branch information
scofalik committed Jul 4, 2019
1 parent 85ea134 commit 94891ee
Show file tree
Hide file tree
Showing 6 changed files with 343 additions and 11 deletions.
11 changes: 4 additions & 7 deletions src/delete.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ export default class Delete extends Plugin {
// on DOM selection level, because on `keyup` the model selection is still the same as it was just after deletion, so it
// wouldn't be changed and the fix would do nothing.
//
/* istanbul ignore if */
if ( env.isAndroid ) {
let domSelectionAfterDeletion = null;

Expand All @@ -83,14 +82,12 @@ export default class Delete extends Plugin {
}, { priority: 'lowest' } );

this.listenTo( viewDocument, 'keyup', ( evt, data ) => {
if ( domSelectionAfterDeletion ) {
const domSelection = data.domTarget.ownerDocument.defaultView.getSelection();
const domSelection = data.domTarget.ownerDocument.defaultView.getSelection();

domSelection.collapse( domSelectionAfterDeletion.anchorNode, domSelectionAfterDeletion.anchorOffset );
domSelection.extend( domSelectionAfterDeletion.focusNode, domSelectionAfterDeletion.focusOffset );
domSelection.collapse( domSelectionAfterDeletion.anchorNode, domSelectionAfterDeletion.anchorOffset );
domSelection.extend( domSelectionAfterDeletion.focusNode, domSelectionAfterDeletion.focusOffset );

domSelectionAfterDeletion = null;
}
domSelectionAfterDeletion = null;
} );
}
}
Expand Down
1 change: 0 additions & 1 deletion src/deleteobserver.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ export default class DeleteObserver extends Observer {
} );

// `beforeinput` is handled only for Android devices. Desktop Chrome and iOS are skipped because they are working fine now.
/* istanbul ignore if */
if ( env.isAndroid ) {
document.on( 'beforeinput', ( evt, data ) => {
// If event type is other than `deleteContentBackward` then this is not deleting.
Expand Down
1 change: 0 additions & 1 deletion src/utils/injectunsafekeystrokeshandling.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ export default function injectUnsafeKeystrokesHandling( editor ) {
const inputCommand = editor.commands.get( 'input' );

// For Android, we want to handle keystrokes on `beforeinput` to be sure that code in `DeleteObserver` already had a chance to be fired.
/* istanbul ignore if */
if ( env.isAndroid ) {
view.document.on( 'beforeinput', ( evt, evtData ) => handleUnsafeKeystroke( evtData ), { priority: 'lowest' } );
} else {
Expand Down
89 changes: 88 additions & 1 deletion tests/delete.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
*/

import Delete from '../src/delete';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import DomEventData from '@ckeditor/ckeditor5-engine/src/view/observer/domeventdata';
import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';
import env from '@ckeditor/ckeditor5-utils/src/env';

/* globals document */
/* globals window, document */

describe( 'Delete feature', () => {
let element, editor, viewDocument;
Expand Down Expand Up @@ -106,3 +108,88 @@ describe( 'Delete feature', () => {
};
}
} );

describe( 'Delete feature - Android', () => {
let element, editor, oldEnvIsAndroid;

before( () => {
oldEnvIsAndroid = env.isAndroid;
env.isAndroid = true;
} );

beforeEach( () => {
element = document.createElement( 'div' );
document.body.appendChild( element );

return ClassicTestEditor
.create( element, { plugins: [ Delete, Paragraph ] } )
.then( newEditor => {
editor = newEditor;

const modelRoot = editor.model.document.getRoot();

editor.model.change( writer => {
writer.insertElement( 'paragraph', modelRoot, 0 );
writer.insertText( 'Foobar', modelRoot.getChild( 0 ), 0 );

writer.setSelection( modelRoot.getChild( 0 ), 3 );
} );
} );
} );

afterEach( () => {
element.remove();
return editor.destroy();
} );

after( () => {
env.isAndroid = oldEnvIsAndroid;
} );

it( 'should re-set selection on keyup event if it was changed after deletion but before the input was fired (Android)', () => {
// This test covers a quirk on Android. We will recreate what browser does in this scenario.
// The test is not perfect because there are difficulties converting model selection to DOM in unit tests.
const view = editor.editing.view;
const viewDocument = view.document;

const domEvt = {
preventDefault: sinon.spy()
};

const domRoot = view.getDomRoot();
const domSelection = window.getSelection();
const domText = domRoot.childNodes[ 0 ].childNodes[ 0 ];

// Change the selection ("manual conversion").
// Because it all works quite bad the selection will be moved to quite a random place after delete is fired but all we care is
// checking if the selection is reversed on `keyup` event.
domSelection.collapse( domText, 3 );

// On `delete` the selection is saved.
viewDocument.fire( 'delete', new DomEventData( viewDocument, domEvt, {
direction: 'backward',
unit: 'character',
sequence: 1,
domTarget: domRoot
} ) );

// Store what was the selection when it was saved in `delete`.
const anchorNodeBefore = domSelection.anchorNode;
const anchorOffsetBefore = domSelection.anchorOffset;
const focusNodeBefore = domSelection.focusNode;
const focusOffsetBefore = domSelection.focusOffset;

// Change the selection.
domSelection.collapse( domText, 0 );

// On `keyup` it should be reversed.
viewDocument.fire( 'keyup', new DomEventData( viewDocument, domEvt, {
domTarget: domRoot
} ) );

expect( domSelection.anchorNode ).to.equal( anchorNodeBefore );
expect( domSelection.anchorOffset ).to.equal( anchorOffsetBefore );
expect( domSelection.focusNode ).to.equal( focusNodeBefore );
expect( domSelection.focusOffset ).to.equal( focusOffsetBefore );
} );
} );
148 changes: 147 additions & 1 deletion tests/deleteobserver.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

/* globals document */
/* globals window, document */

import DeleteObserver from '../src/deleteobserver';
import View from '@ckeditor/ckeditor5-engine/src/view/view';
Expand Down Expand Up @@ -260,3 +260,149 @@ describe( 'DeleteObserver', () => {
};
}
} );

describe( 'DeleteObserver - Android', () => {
let view, viewDocument, oldEnvIsAndroid, domElement, viewRoot, domText;

testUtils.createSinonSandbox();

before( () => {
oldEnvIsAndroid = env.isAndroid;
env.isAndroid = true;
} );

beforeEach( () => {
domElement = document.createElement( 'div' );
domElement.contenteditable = true;

document.body.appendChild( domElement );

view = new View();
viewDocument = view.document;
view.addObserver( DeleteObserver );

viewRoot = createViewRoot( viewDocument );
view.attachDomRoot( domElement );

view.change( writer => {
const p = writer.createContainerElement( 'p' );

writer.insert( writer.createPositionAt( viewRoot, 0 ), p );

const text = writer.createText( 'foo' );

writer.insert( writer.createPositionAt( p, 0 ), text );
} );

domText = domElement.childNodes[ 0 ].childNodes[ 0 ];
} );

afterEach( () => {
view.destroy();
domElement.remove();
} );

after( () => {
env.isAndroid = oldEnvIsAndroid;
} );

describe( 'delete event', () => {
it( 'is fired on beforeinput', () => {
const spy = sinon.spy();

viewDocument.on( 'delete', spy );

setDomSelection( domText, 1, domText, 2 );

viewDocument.fire( 'beforeinput', new DomEventData( viewDocument, getDomEvent( 'deleteContentBackward' ), {
domTarget: domElement
} ) );

expect( spy.calledOnce ).to.be.true;

const data = spy.args[ 0 ][ 1 ];
expect( data ).to.have.property( 'direction', 'backward' );
expect( data ).to.have.property( 'unit', 'codepoint' );
expect( data ).to.have.property( 'sequence', 1 );
expect( data ).not.to.have.property( 'selectionToRemove' );
} );

it( 'should set selectionToRemove if DOM selection size is different than 1', () => {
// In real scenarios, before `beforeinput` is fired, browser changes DOM selection to a selection that contains
// all content that should be deleted. If the selection is big (> 1 character) we need to pass special parameter
// so that `DeleteCommand` will know what to delete. This test checks that case.
const spy = sinon.spy();

viewDocument.on( 'delete', spy );

setDomSelection( domText, 0, domText, 3 );

viewDocument.fire( 'beforeinput', new DomEventData( viewDocument, getDomEvent( 'deleteContentBackward' ), {
domTarget: domElement
} ) );

expect( spy.calledOnce ).to.be.true;

const data = spy.args[ 0 ][ 1 ];
expect( data ).to.have.property( 'selectionToRemove' );

const viewText = viewRoot.getChild( 0 ).getChild( 0 );
const range = data.selectionToRemove.getFirstRange();

expect( range.start.offset ).to.equal( 0 );
expect( range.start.parent ).to.equal( viewText );
expect( range.end.offset ).to.equal( 3 );
expect( range.end.parent ).to.equal( viewText );
} );

it( 'is not fired on beforeinput when event type is other than deleteContentBackward', () => {
const spy = sinon.spy();

viewDocument.on( 'delete', spy );

viewDocument.fire( 'keydown', new DomEventData( viewDocument, getDomEvent( 'insertText' ), {
domTarget: domElement
} ) );

expect( spy.calledOnce ).to.be.false;
} );

it( 'should stop beforeinput event when delete event is stopped', () => {
const keydownSpy = sinon.spy();
viewDocument.on( 'beforeinput', keydownSpy );
viewDocument.on( 'delete', evt => evt.stop() );

viewDocument.fire( 'beforeinput', new DomEventData( viewDocument, getDomEvent( 'deleteContentBackward' ), {
domTarget: domElement
} ) );

sinon.assert.notCalled( keydownSpy );
} );

it( 'should not stop keydown event when delete event is not stopped', () => {
const keydownSpy = sinon.spy();
viewDocument.on( 'beforeinput', keydownSpy );
viewDocument.on( 'delete', evt => evt.stop() );

viewDocument.fire( 'beforeinput', new DomEventData( viewDocument, getDomEvent( 'insertText' ), {
domTarget: domElement
} ) );

sinon.assert.calledOnce( keydownSpy );
} );
} );

function getDomEvent( inputType ) {
return {
inputType,
preventDefault: sinon.spy()
};
}

function setDomSelection( anchorNode, anchorOffset, focusNode, focusOffset ) {
const selection = window.getSelection();

selection.collapse( anchorNode, anchorOffset );
selection.extend( focusNode, focusOffset );
}
} );
Loading

0 comments on commit 94891ee

Please sign in to comment.