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 #1783 from mycolorway:t/ckeditor5/1333.
Browse files Browse the repository at this point in the history
Fix: The renderer shouold not update DOM selection when document has active composition. Closes #1782. Closes ckeditor/ckeditor5#1333.
  • Loading branch information
jodator committed Sep 9, 2019
2 parents 62fe3e1 + 427ee66 commit c698683
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 0 deletions.
12 changes: 12 additions & 0 deletions src/view/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,13 @@ export default class Renderer {
*/
this.isFocused = false;

/**
* Indicates if the composition is in progress inside the view document view.
*
* @member {Boolean}
*/
this.isComposing = false;

/**
* The text node in which the inline filler was rendered.
*
Expand Down Expand Up @@ -771,6 +778,11 @@ export default class Renderer {
* @returns {Boolean}
*/
_domSelectionNeedsUpdate( domSelection ) {
// Remain DOM selection untouched while composing. See #1782.
if ( this.isComposing ) {
return false;
}

if ( !this.domConverter.isDomSelectionCorrect( domSelection ) ) {
// Current DOM selection is in incorrect position. We need to update it.
return true;
Expand Down
1 change: 1 addition & 0 deletions src/view/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ export default class View {
*/
this._renderer = new Renderer( this.domConverter, this.document.selection );
this._renderer.bind( 'isFocused' ).to( this.document );
this._renderer.bind( 'isComposing' ).to( this.document );

/**
* A DOM root attributes cache. It saves the initial values of DOM root attributes before the DOM element
Expand Down
42 changes: 42 additions & 0 deletions tests/view/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -3641,6 +3641,48 @@ describe( 'Renderer', () => {
return viewData.repeat( repeat );
}
} );

// #1782
it( 'should leave dom selection untouched while composing', () => {
const { view: viewP, selection: newSelection } = parse( '<container:p>[]</container:p>' );

viewRoot._appendChild( viewP );
selection._setTo( newSelection );

renderer.markToSync( 'children', viewRoot );
renderer.render();

// Mock IME typing in Safari: <p>[c]</p>.
renderer.isComposing = true;
const domText = document.createTextNode( 'c' );
domRoot.firstChild.appendChild( domText );
const range = document.createRange();
range.setStart( domText, 0 );
range.setEnd( domText, 1 );
const domSelection = document.getSelection();
domSelection.removeAllRanges();
domSelection.addRange( range );

// <container:p>c[]</container:p>
viewP._appendChild( new ViewText( 'c' ) );
selection._setTo( [
new ViewRange( new ViewPosition( viewP.getChild( 0 ), 1 ), new ViewPosition( viewP.getChild( 0 ), 1 ) )
] );

renderer.markToSync( 'children', viewP );
renderer.render();

expect( domRoot.childNodes.length ).to.equal( 1 );
expect( domRoot.firstChild.childNodes.length ).to.equal( 1 );
expect( domRoot.firstChild.firstChild.data ).to.equal( 'c' );

const currentRange = domSelection.getRangeAt( 0 );
expect( currentRange.collapsed ).to.equal( false );
expect( currentRange.startContainer ).to.equal( domRoot.firstChild.firstChild );
expect( currentRange.startOffset ).to.equal( 0 );
expect( currentRange.endContainer ).to.equal( domRoot.firstChild.firstChild );
expect( currentRange.endOffset ).to.equal( 1 );
} );
} );

describe( '#922', () => {
Expand Down

0 comments on commit c698683

Please sign in to comment.