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 #896 from ckeditor/t/887
Browse files Browse the repository at this point in the history
Fix: Renderer should not change the native selection if the one it's about to render is visually similar to the current one. Closes #887. Closes #880.
  • Loading branch information
Reinmar authored Mar 30, 2017
2 parents 8b859fb + b7f8491 commit d8ee5fa
Show file tree
Hide file tree
Showing 8 changed files with 417 additions and 0 deletions.
45 changes: 45 additions & 0 deletions src/view/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@

import ViewText from './text';
import ViewPosition from './position';
import Selection from './selection';
import { INLINE_FILLER, INLINE_FILLER_LENGTH, startsWithFiller, isInlineFiller, isBlockFiller } from './filler';

import mix from '@ckeditor/ckeditor5-utils/src/mix';
import diff from '@ckeditor/ckeditor5-utils/src/diff';
import insertAt from '@ckeditor/ckeditor5-utils/src/dom/insertat';
import remove from '@ckeditor/ckeditor5-utils/src/dom/remove';
import log from '@ckeditor/ckeditor5-utils/src/log';
import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin';
import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';

Expand Down Expand Up @@ -580,6 +582,17 @@ export default class Renderer {
return;
}

if ( oldViewSelection && areSimilarSelections( oldViewSelection, this.selection ) ) {
const data = {
oldSelection: oldViewSelection,
currentSelection: this.selection
};

log.warn( 'renderer-skipped-selection-rendering: The selection was not rendered due to its similarity to the current one.', data );

return;
}

// Multi-range selection is not available in most browsers, and, at least in Chrome, trying to
// set such selection, that is not continuous, throws an error. Because of that, we will just use anchor
// and focus of view selection.
Expand Down Expand Up @@ -642,3 +655,35 @@ export default class Renderer {
}

mix( Renderer, ObservableMixin );

// Checks if two given selections are similar. Selections are considered similar if they are non-collapsed
// and their trimmed (see {@link #_trimSelection}) representations are equal.
//
// @private
// @param {module:engine/view/selection~Selection} selection1
// @param {module:engine/view/selection~Selection} selection2
// @returns {Boolean}
function areSimilarSelections( selection1, selection2 ) {
return !selection1.isCollapsed && trimSelection( selection1 ).isEqual( trimSelection( selection2 ) );
}

// Creates a copy of a given selection with all of its ranges
// trimmed (see {@link module:engine/view/range~Range#getTrimmed getTrimmed}).
//
// @private
// @param {module:engine/view/selection~Selection} selection
// @returns {module:engine/view/selection~Selection} Selection copy with all ranges trimmed.
function trimSelection( selection ) {
const newSelection = Selection.createFromSelection( selection );
const ranges = newSelection.getRanges();

let trimmedRanges = [];

for ( let range of ranges ) {
trimmedRanges.push( range.getTrimmed() );
}

newSelection.setRanges( trimmedRanges, newSelection.isBackward );

return newSelection;
}
3 changes: 3 additions & 0 deletions tests/manual/tickets/880/1.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<div id="editor">
<p>This is an <strong>editor</strong> instance.</p>
</div>
27 changes: 27 additions & 0 deletions tests/manual/tickets/880/1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/**
* @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

/* globals console, window, document */

import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classic';
import EssentialsPreset from '@ckeditor/ckeditor5-presets/src/essentials';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold';

ClassicEditor.create( document.querySelector( '#editor' ), {
plugins: [ EssentialsPreset, Paragraph, Bold ],
toolbar: [ 'undo', 'redo' ]
} )
.then( editor => {
window.editor = editor;

editor.editing.view.on( 'selectionChange', () => {
editor.document.enqueueChanges( () => {} );
console.log( 'selectionChange', ( new Date() ).getTime() );
} );
} )
.catch( err => {
console.error( err.stack );
} );
14 changes: 14 additions & 0 deletions tests/manual/tickets/880/1.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
## Renderer - infinite `selectionChange` event

Place the selection like this:

```
This is {an <strong>editor}</strong> instance.
```

(it must end at the end of the inline style)

**Expected**:

* Every time selection is changed, console log with `selectionChange` is printed once.
* While creating selection from **2.**, there might be `renderer-selection-similar` warning visible in the console.
3 changes: 3 additions & 0 deletions tests/manual/tickets/887/1.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<div id="editor">
<p>This is an <strong>editor</strong> instance.</p>
</div>
22 changes: 22 additions & 0 deletions tests/manual/tickets/887/1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/**
* @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

/* globals console, window, document */

import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classic';
import EssentialsPreset from '@ckeditor/ckeditor5-presets/src/essentials';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold';

ClassicEditor.create( document.querySelector( '#editor' ), {
plugins: [ EssentialsPreset, Paragraph, Bold ],
toolbar: [ 'undo', 'redo' ]
} )
.then( editor => {
window.editor = editor;
} )
.catch( err => {
console.error( err.stack );
} );
10 changes: 10 additions & 0 deletions tests/manual/tickets/887/1.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
## Renderer - MacOS accent balloon on inline element boundary

1. Place the selection like this:

`This is an <strong>editor{}</strong> instance.`
2. Open MacOS accent balloon (e.g. long `a` press).
3. Navigate through the balloon panel using arrow keys.

**Expected**: It is possible to navigate with arrow keys inside the MacOS balloon panel. While navigating, there
might be `renderer-selection-similar` warning visible in the console.
Loading

0 comments on commit d8ee5fa

Please sign in to comment.