Skip to content

Commit

Permalink
Merge pull request #16289 from ckeditor/ck/epic/ime-typing
Browse files Browse the repository at this point in the history 
Fix (typing, engine): Predictive text should not get doubled while typing. Closes #16106.

Fix (engine): The reverse typing effect should not happen after the focus change. Closes #14702. Thanks, @urbanspr1nter!

Fix (engine, typing): Typing on Android should avoid modifying DOM while composing. Closes #13994. Closes #14707. Closes #13850. Closes #13693. Closes #14567. Closes: #11569.
  • Loading branch information
@niegowski
niegowski authored Jul 3, 2024
2 parents 09b31af + ce023f1 commit e4317d0
Show file tree
Hide file tree
Showing 18 changed files with 1,885 additions and 418 deletions.
9 changes: 9 additions & 0 deletions packages/ckeditor5-engine/src/dev-utils/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@

/* globals console */

// @if CK_DEBUG_TYPING // const { debounce } = require( 'lodash-es' );

/**
* Helper function, converts a map to the 'key1="value1" key2="value1"' format.
*
Expand Down Expand Up @@ -88,3 +90,10 @@ export function logDocument( document: any, version: any ): void {
console.log( 'Tree log unavailable for given version: ' + version );
}
}

// @if CK_DEBUG_TYPING // export const _debouncedLine = debounce( () => {
// @if CK_DEBUG_TYPING // console.log(
// @if CK_DEBUG_TYPING // '%c───────────────────────────────────────────────────────────────────────────────────────────────────────',
// @if CK_DEBUG_TYPING // 'font-weight: bold; color: red'
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // }, 300 );
1 change: 1 addition & 0 deletions packages/ckeditor5-engine/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ export type {
ViewDocumentCompositionEndEvent
} from './view/observer/compositionobserver.js';
export type { ViewDocumentInputEvent } from './view/observer/inputobserver.js';
export type { ViewDocumentMutationsEvent, MutationData } from './view/observer/mutationobserver.js';
export type { ViewDocumentKeyDownEvent, ViewDocumentKeyUpEvent, KeyEventData } from './view/observer/keyobserver.js';
export type { ViewDocumentLayoutChangedEvent } from './view/document.js';
export type {
Expand Down
3 changes: 3 additions & 0 deletions packages/ckeditor5-engine/src/view/observer/compositionobserver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import DomEventObserver from './domeventobserver.js';
import type View from '../view.js';
import type DomEventData from './domeventdata.js';

// @if CK_DEBUG_TYPING // const { _debouncedLine } = require( '../../dev-utils/utils.js' );

/**
* {@link module:engine/view/document~Document#event:compositionstart Compositionstart},
* {@link module:engine/view/document~Document#event:compositionupdate compositionupdate} and
Expand Down Expand Up @@ -58,6 +60,7 @@ export default class CompositionObserver extends DomEventObserver<'compositionst
*/
public onDomEvent( domEvent: CompositionEvent ): void {
// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // _debouncedLine();
// @if CK_DEBUG_TYPING // console.group( `%c[CompositionObserver]%c ${ domEvent.type }`, 'color: green', '' );
// @if CK_DEBUG_TYPING // }

Expand Down
89 changes: 59 additions & 30 deletions packages/ckeditor5-engine/src/view/observer/focusobserver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import DomEventObserver from './domeventobserver.js';
import type DomEventData from './domeventdata.js';
import type View from '../view.js';
import type { ViewDocumentInputEvent } from './inputobserver.js';

/**
* {@link module:engine/view/document~Document#event:focus Focus}
Expand Down Expand Up @@ -48,35 +49,18 @@ export default class FocusObserver extends DomEventObserver<'focus' | 'blur'> {
this.useCapture = true;
const document = this.document;

document.on<ViewDocumentFocusEvent>( 'focus', () => {
this._isFocusChanging = true;

// Unfortunately native `selectionchange` event is fired asynchronously.
// We need to wait until `SelectionObserver` handle the event and then render. Otherwise rendering will
// overwrite new DOM selection with selection from the view.
// See https://github.com/ckeditor/ckeditor5-engine/issues/795 for more details.
// Long timeout is needed to solve #676 and https://github.com/ckeditor/ckeditor5-engine/issues/1157 issues.
//
// Using `view.change()` instead of `view.forceRender()` to prevent double rendering
// in a situation where `selectionchange` already caused selection change.
this._renderTimeoutId = setTimeout( () => {
this.flush();
view.change( () => {} );
}, 50 );
} );

document.on<ViewDocumentBlurEvent>( 'blur', ( evt, data ) => {
const selectedEditable = document.selection.editableElement;

if ( selectedEditable === null || selectedEditable === data.target ) {
document.isFocused = false;
this._isFocusChanging = false;

// Re-render the document to update view elements
// (changing document.isFocused already marked view as changed since last rendering).
view.change( () => {} );
document.on<ViewDocumentFocusEvent>( 'focus', () => this._handleFocus() );
document.on<ViewDocumentBlurEvent>( 'blur', ( evt, data ) => this._handleBlur( data ) );

// Focus the editor in cases where browser dispatches `beforeinput` event to a not-focused editable element.
// This is flushed by the beforeinput listener in the `InsertTextObserver`.
// Note that focus is set only if the document is not focused yet.
// See https://github.com/ckeditor/ckeditor5/issues/14702.
document.on<ViewDocumentInputEvent>( 'beforeinput', () => {
if ( !document.isFocused ) {
this._handleFocus();
}
} );
}, { priority: 'highest' } );
}

/**
Expand All @@ -100,11 +84,56 @@ export default class FocusObserver extends DomEventObserver<'focus' | 'blur'> {
* @inheritDoc
*/
public override destroy(): void {
this._clearTimeout();
super.destroy();
}

/**
* The `focus` event handler.
*/
private _handleFocus(): void {
this._clearTimeout();
this._isFocusChanging = true;

// Unfortunately native `selectionchange` event is fired asynchronously.
// We need to wait until `SelectionObserver` handle the event and then render. Otherwise rendering will
// overwrite new DOM selection with selection from the view.
// See https://github.com/ckeditor/ckeditor5-engine/issues/795 for more details.
// Long timeout is needed to solve #676 and https://github.com/ckeditor/ckeditor5-engine/issues/1157 issues.
//
// Using `view.change()` instead of `view.forceRender()` to prevent double rendering
// in a situation where `selectionchange` already caused selection change.
this._renderTimeoutId = setTimeout( () => {
this._renderTimeoutId = 0;
this.flush();
this.view.change( () => {} );
}, 50 );
}

/**
* The `blur` event handler.
*/
private _handleBlur( data: DomEventData<FocusEvent> ): void {
const selectedEditable = this.document.selection.editableElement;

if ( selectedEditable === null || selectedEditable === data.target ) {
this.document.isFocused = false;
this._isFocusChanging = false;

// Re-render the document to update view elements
// (changing document.isFocused already marked view as changed since last rendering).
this.view.change( () => {} );
}
}

/**
* Clears timeout.
*/
private _clearTimeout(): void {
if ( this._renderTimeoutId ) {
clearTimeout( this._renderTimeoutId );
this._renderTimeoutId = 0;
}

super.destroy();
}
}

Expand Down
13 changes: 8 additions & 5 deletions packages/ckeditor5-engine/src/view/observer/inputobserver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import type ViewRange from '../range.js';
import DataTransfer from '../datatransfer.js';
import { env } from '@ckeditor/ckeditor5-utils';

// @if CK_DEBUG_TYPING // const { _debouncedLine } = require( '../../dev-utils/utils.js' );

/**
* Observer for events connected with data input.
*
Expand All @@ -30,6 +32,7 @@ export default class InputObserver extends DomEventObserver<'beforeinput'> {
*/
public onDomEvent( domEvent: InputEvent ): void {
// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // _debouncedLine();
// @if CK_DEBUG_TYPING // console.group( `%c[InputObserver]%c ${ domEvent.type }: ${ domEvent.inputType }`,
// @if CK_DEBUG_TYPING // 'color: green', 'color: default'
// @if CK_DEBUG_TYPING // );
Expand All @@ -52,15 +55,15 @@ export default class InputObserver extends DomEventObserver<'beforeinput'> {

// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.info( `%c[InputObserver]%c event data: %c${ JSON.stringify( data ) }`,
// @if CK_DEBUG_TYPING // 'color: green;font-weight: bold', 'font-weight:bold', 'color: blue;'
// @if CK_DEBUG_TYPING // 'color: green; font-weight: bold', 'font-weight: bold', 'color: blue;'
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // }
} else if ( dataTransfer ) {
data = dataTransfer.getData( 'text/plain' );

// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.info( `%c[InputObserver]%c event data transfer: %c${ JSON.stringify( data ) }`,
// @if CK_DEBUG_TYPING // 'color: green;font-weight: bold', 'font-weight:bold', 'color: blue;'
// @if CK_DEBUG_TYPING // 'color: green; font-weight: bold', 'font-weight: bold', 'color: blue;'
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // }
}
Expand All @@ -73,7 +76,7 @@ export default class InputObserver extends DomEventObserver<'beforeinput'> {

// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.info( '%c[InputObserver]%c using fake selection:',
// @if CK_DEBUG_TYPING // 'color: green;font-weight: bold', 'font-weight:bold', targetRanges,
// @if CK_DEBUG_TYPING // 'color: green; font-weight: bold', 'font-weight: bold', targetRanges,
// @if CK_DEBUG_TYPING // viewDocument.selection.isFake ? 'fake view selection' : 'fake DOM parent'
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // }
Expand All @@ -95,7 +98,7 @@ export default class InputObserver extends DomEventObserver<'beforeinput'> {

// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.info( '%c[InputObserver]%c using target ranges:',
// @if CK_DEBUG_TYPING // 'color: green;font-weight: bold', 'font-weight:bold', targetRanges
// @if CK_DEBUG_TYPING // 'color: green; font-weight: bold', 'font-weight: bold', targetRanges
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // }
}
Expand All @@ -108,7 +111,7 @@ export default class InputObserver extends DomEventObserver<'beforeinput'> {

// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.info( '%c[InputObserver]%c using selection ranges:',
// @if CK_DEBUG_TYPING // 'color: green;font-weight: bold', 'font-weight:bold', targetRanges
// @if CK_DEBUG_TYPING // 'color: green; font-weight: bold', 'font-weight: bold', targetRanges
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // }
}
Expand Down
64 changes: 47 additions & 17 deletions packages/ckeditor5-engine/src/view/observer/mutationobserver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ import { startsWithFiller } from '../filler.js';
import { isEqualWith } from 'lodash-es';

import type DomConverter from '../domconverter.js';
import type Renderer from '../renderer.js';
import type View from '../view.js';
import type ViewElement from '../element.js';
import type ViewNode from '../node.js';
import type ViewText from '../text.js';
import type { ChangeType } from '../document.js';

// @if CK_DEBUG_TYPING // const { _debouncedLine } = require( '../../dev-utils/utils.js' );

/**
* Mutation observer's role is to watch for any DOM changes inside the editor that weren't
Expand All @@ -37,11 +39,6 @@ export default class MutationObserver extends Observer {
*/
public readonly domConverter: DomConverter;

/**
* Reference to the {@link module:engine/view/view~View#_renderer}.
*/
public readonly renderer: Renderer;

/**
* Native mutation observer config.
*/
Expand Down Expand Up @@ -70,7 +67,6 @@ export default class MutationObserver extends Observer {
};

this.domConverter = view.domConverter;
this.renderer = view._renderer;

this._domElements = new Set();
this._mutationObserver = new window.MutationObserver( this._onMutations.bind( this ) );
Expand Down Expand Up @@ -205,11 +201,10 @@ export default class MutationObserver extends Observer {
// Now we build the list of mutations to mark elements. We did not do it earlier to avoid marking the
// same node multiple times in case of duplication.

let hasMutations = false;
const mutations: Array<MutationData> = [];

for ( const textNode of mutatedTextNodes ) {
hasMutations = true;
this.renderer.markToSync( 'text', textNode );
mutations.push( { type: 'text', node: textNode } );
}

for ( const viewElement of elementsWithMutatedChildren ) {
Expand All @@ -220,22 +215,20 @@ export default class MutationObserver extends Observer {
// It may happen that as a result of many changes (sth was inserted and then removed),
// both elements haven't really changed. #1031
if ( !isEqualWith( viewChildren, newViewChildren, sameNodes ) ) {
hasMutations = true;
this.renderer.markToSync( 'children', viewElement );
mutations.push( { type: 'children', node: viewElement } );
}
}

// In case only non-relevant mutations were recorded it skips the event and force render (#5600).
if ( hasMutations ) {
if ( mutations.length ) {
// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // _debouncedLine();
// @if CK_DEBUG_TYPING // console.group( '%c[MutationObserver]%c Mutations detected',
// @if CK_DEBUG_TYPING // 'font-weight:bold;color:green', ''
// @if CK_DEBUG_TYPING // 'font-weight: bold; color: green', 'font-weight: bold'
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // }

// At this point we have "dirty DOM" (changed) and de-synched view (which has not been changed).
// In order to "reset DOM" we render the view again.
this.view.forceRender();
this.document.fire<ViewDocumentMutationsEvent>( 'mutations', { mutations } );

// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.groupEnd();
Expand Down Expand Up @@ -282,3 +275,40 @@ function sameNodes( child1: ViewNode, child2: ViewNode ) {
// Not matching types.
return false;
}

/**
* Event fired on DOM mutations detected.
*
* This event is introduced by {@link module:engine/view/observer/mutationobserver~MutationObserver} and available
* by default in all editor instances (attached by {@link module:engine/view/view~View}).
*
* @eventName module:engine/view/document~Document#mutations
* @param data Event data containing detailed information about the event.
*/
export type ViewDocumentMutationsEvent = {
name: 'mutations';
args: [ data: MutationsEventData ];
};

/**
* The value of {@link ~ViewDocumentMutationsEvent}.
*/
export type MutationsEventData = {
mutations: Array<MutationData>;
};

/**
* A single entry in {@link ~MutationsEventData} mutations array.
*/
export type MutationData = {

/**
* Type of mutation detected.
*/
type: ChangeType;

/**
* The view node related to the detected mutation.
*/
node: ViewNode;
};
Loading

0 comments on commit e4317d0

Please sign in to comment.