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

Commit

Permalink
Merge a900f95 into b2a8189
Browse files Browse the repository at this point in the history
  • Loading branch information
scofalik committed Jan 21, 2020
2 parents b2a8189 + a900f95 commit d1a5f5d
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 31 deletions.
13 changes: 9 additions & 4 deletions src/view/observer/selectionobserver.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,7 @@ export default class SelectionObserver extends Observer {
* @param {Document} domDocument DOM document.
*/
_handleSelectionChange( domDocument ) {
// Selection is handled when document is not focused but is read-only. This is because in read-only
// mode contenteditable is set as false and editor won't receive focus but we still need to know
// selection position.
if ( !this.isEnabled || ( !this.document.isFocused && !this.document.isReadOnly ) ) {
if ( !this.isEnabled ) {
return;
}

Expand All @@ -141,6 +138,14 @@ export default class SelectionObserver extends Observer {
const domSelection = domDocument.defaultView.getSelection();
const newViewSelection = this.domConverter.domSelectionToView( domSelection );

// Do not convert selection change if the new view selection has no ranges in it.
//
// It means that the DOM selection is in some way incorrect. Ranges that were in the DOM selection could not be
// converted to the view. This happens when the DOM selection was moved outside of the editable element.
if ( newViewSelection.rangeCount == 0 ) {
return;
}

if ( this.selection.isEqual( newViewSelection ) && this.domConverter.isDomSelectionCorrect( domSelection ) ) {
return;
}
Expand Down
39 changes: 12 additions & 27 deletions tests/view/observer/selectionobserver.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ describe( 'SelectionObserver', () => {
} );
} );

it( 'should not fired if observer is disabled', done => {
it( 'should not fire if observer is disabled', done => {
view.getObserver( SelectionObserver ).disable();

viewDocument.on( 'selectionChange', () => {
Expand All @@ -120,43 +120,28 @@ describe( 'SelectionObserver', () => {
changeDomSelection();
} );

it( 'should not fired if there is no focus', done => {
viewDocument.isFocused = false;

// changeDomSelection() may focus the editable element (happens on Chrome)
// so cancel this because it sets the isFocused flag.
viewDocument.on( 'focus', evt => evt.stop(), { priority: 'highest' } );

viewDocument.on( 'selectionChange', () => {
// Validate the correctness of the test. May help tracking issue with this test.
expect( viewDocument.isFocused ).to.be.false;
it( 'should not fire if the DOM selection was set outside editable', done => {
const viewFoo = viewDocument.getRoot().getChild( 0 ).getChild( 0 );

throw 'selectionChange on render';
view.change( writer => {
writer.setSelection( viewFoo, 0 );
} );

setTimeout( done, 70 );

changeDomSelection();
} );

it( 'should fired if there is no focus but document is read-only', done => {
const spy = sinon.spy();

viewDocument.isFocused = false;
viewDocument.isReadOnly = true;

// changeDomSelection() may focus the editable element (happens on Chrome)
// so cancel this because it sets the isFocused flag.
viewDocument.on( 'focus', evt => evt.stop(), { priority: 'highest' } );

viewDocument.on( 'selectionChange', spy );

setTimeout( () => {
sinon.assert.called( spy );
expect( spy.called ).to.be.false;

done();
}, 70 );

changeDomSelection();
const domSelection = domDocument.getSelection();
const editable = domRoot.childNodes[ 1 ];
editable.focus();

domSelection.collapse( editable, 0 );
} );

it( 'should not enter infinite loop', () => {
Expand Down

0 comments on commit d1a5f5d

Please sign in to comment.