Skip to content

Commit

Permalink
Merge pull request #16300 from ckeditor/ck/13994-android-reverse-typing
Browse files Browse the repository at this point in the history
Fix (engine, typing): Typing on Android should avoid modifying DOM while composing. Closes #13994. Closes #14707. Closes #13850.
  • Loading branch information
niegowski authored Jun 13, 2024
2 parents ecb5168 + 4e72e57 commit f610387
Show file tree
Hide file tree
Showing 14 changed files with 1,658 additions and 383 deletions.
9 changes: 9 additions & 0 deletions packages/ckeditor5-engine/src/dev-utils/utils.ts
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
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,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
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
13 changes: 8 additions & 5 deletions packages/ckeditor5-engine/src/view/observer/inputobserver.ts
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
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;
};
25 changes: 21 additions & 4 deletions packages/ckeditor5-engine/src/view/observer/selectionobserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import type DomConverter from '../domconverter.js';
import type Selection from '../selection.js';
import type { ViewDocumentCompositionStartEvent } from './compositionobserver.js';

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

type DomSelection = globalThis.Selection;

/**
Expand Down Expand Up @@ -156,10 +158,11 @@ export default class SelectionObserver extends Observer {

this.listenTo( domDocument, 'selectionchange', ( evt, domEvent ) => {
// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // _debouncedLine();
// @if CK_DEBUG_TYPING // const domSelection = domDocument.defaultView!.getSelection();
// @if CK_DEBUG_TYPING // console.group( '%c[SelectionObserver]%c selectionchange', 'color:green', ''
// @if CK_DEBUG_TYPING // console.group( '%c[SelectionObserver]%c selectionchange', 'color: green', ''
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // console.info( '%c[SelectionObserver]%c DOM Selection:', 'font-weight:bold;color:green', '',
// @if CK_DEBUG_TYPING // console.info( '%c[SelectionObserver]%c DOM Selection:', 'font-weight: bold; color: green', '',
// @if CK_DEBUG_TYPING // { node: domSelection!.anchorNode, offset: domSelection!.anchorOffset },
// @if CK_DEBUG_TYPING // { node: domSelection!.focusNode, offset: domSelection!.focusOffset }
// @if CK_DEBUG_TYPING // );
Expand All @@ -170,7 +173,7 @@ export default class SelectionObserver extends Observer {
if ( this.document.isComposing && !env.isAndroid ) {
// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.info( '%c[SelectionObserver]%c Selection change ignored (isComposing)',
// @if CK_DEBUG_TYPING // 'font-weight:bold;color:green', ''
// @if CK_DEBUG_TYPING // 'font-weight: bold; color: green', ''
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // console.groupEnd();
// @if CK_DEBUG_TYPING // }
Expand All @@ -193,7 +196,21 @@ export default class SelectionObserver extends Observer {
// We do this synchronously (without waiting for the `selectionchange` DOM event) as browser updates
// the DOM selection (but not visually) to span the text that is under composition and could be replaced.
this.listenTo<ViewDocumentCompositionStartEvent>( this.view.document, 'compositionstart', () => {
// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // const domSelection = domDocument.defaultView!.getSelection();
// @if CK_DEBUG_TYPING // console.group( '%c[SelectionObserver]%c update selection on compositionstart', 'color: green', ''
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // console.info( '%c[SelectionObserver]%c DOM Selection:', 'font-weight: bold; color: green', '',
// @if CK_DEBUG_TYPING // { node: domSelection!.anchorNode, offset: domSelection!.anchorOffset },
// @if CK_DEBUG_TYPING // { node: domSelection!.focusNode, offset: domSelection!.focusOffset }
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // }

this._handleSelectionChange( domDocument );

// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.groupEnd();
// @if CK_DEBUG_TYPING // }
}, { priority: 'lowest' } );

this._documents.add( domDocument );
Expand Down Expand Up @@ -293,7 +310,7 @@ export default class SelectionObserver extends Observer {

// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.info( '%c[SelectionObserver]%c Fire selection change:',
// @if CK_DEBUG_TYPING // 'font-weight:bold;color:green', '',
// @if CK_DEBUG_TYPING // 'font-weight: bold; color: green', '',
// @if CK_DEBUG_TYPING // newViewSelection.getFirstRange()
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // }
Expand Down
Loading

0 comments on commit f610387

Please sign in to comment.