Skip to content

Ignore selection change events while editable is not focused #17870

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
niegowski merged 11 commits into from
Mar 4, 2025
Merged

Ignore selection change events while editable is not focused #17870

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7677f69
PoC.
niegowski Jan 30, 2025
fe6d067
Logger update.
niegowski Jan 31, 2025
b0225ba
Unified logging.
niegowski Feb 3, 2025
e4942ad
Merge branch 'master' into poc-typing-logger-enhanced
niegowski Feb 4, 2025
471a158
Added logging for focus observer and additional logs for rendering fo…
niegowski Feb 4, 2025
3fe7104
Added more logs.
niegowski Feb 5, 2025
db41d04
Selection change observer only handles events when editable is focuse…
niegowski Feb 22, 2025
2e045e0
SelectionObserver should ignore selection changes while document is n…
niegowski Feb 24, 2025
3dc4c9e
Reverted changes in FocusObserver. Fixed tests.
niegowski Feb 25, 2025
d5fd0da
Added tests.
niegowski Feb 25, 2025
5da51de
Merge branch 'master' into poc-typing-logger-enhanced
niegowski Feb 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions packages/ckeditor5-engine/src/dev-utils/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,40 @@ export function logDocument( document: any, version: any ): void {
// @if CK_DEBUG_TYPING // 'font-weight: bold; color: red'
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // }, 300 );

// @if CK_DEBUG_TYPING // export function _buildLogMessage( context, className, message = '', ...rest ) {
// @if CK_DEBUG_TYPING // const editor = _findAllEditorInstances().find( editor => (
// @if CK_DEBUG_TYPING // _getLogNodes( editor ).includes( context )
// @if CK_DEBUG_TYPING // ) );
// @if CK_DEBUG_TYPING // const editorName = editor && Object.getPrototypeOf( editor ).constructor.name;
// @if CK_DEBUG_TYPING // const editorPrefix = editor ? `${ editorName }-${ editor.id.slice( -4 ) } ` : 'UNKNOWN ';
// @if CK_DEBUG_TYPING // return [
// @if CK_DEBUG_TYPING // `%c${ editorPrefix }%c[${ className }]%c ${ message }`,
// @if CK_DEBUG_TYPING // 'font-weight: normal;',
// @if CK_DEBUG_TYPING // 'font-weight: bold; color: green',
// @if CK_DEBUG_TYPING // '',
// @if CK_DEBUG_TYPING // ...rest
// @if CK_DEBUG_TYPING // ];
// @if CK_DEBUG_TYPING // }

// @if CK_DEBUG_TYPING // function _findAllEditorInstances() {
// @if CK_DEBUG_TYPING // const editors = new Set();
// @if CK_DEBUG_TYPING // for ( const domEditable of document.querySelectorAll( '.ck.ck-content.ck-editor__editable' ) ) {
// @if CK_DEBUG_TYPING // if ( domEditable.ckeditorInstance ) {
// @if CK_DEBUG_TYPING // editors.add( domEditable.ckeditorInstance );
// @if CK_DEBUG_TYPING // }
// @if CK_DEBUG_TYPING // }
// @if CK_DEBUG_TYPING // return Array.from( editors );
// @if CK_DEBUG_TYPING // }

// @if CK_DEBUG_TYPING // function _getLogNodes( editor ) {
// @if CK_DEBUG_TYPING // return [
// @if CK_DEBUG_TYPING // editor.editing.view._renderer,
// @if CK_DEBUG_TYPING // editor.editing.view.domConverter,
// @if CK_DEBUG_TYPING // ...editor.editing.view._observers.values(),
// @if CK_DEBUG_TYPING // editor.plugins.get( 'Input' ),
// @if CK_DEBUG_TYPING // editor.plugins.get( 'WidgetTypeAround' ),
// @if CK_DEBUG_TYPING // editor.commands.get( 'delete' ),
// @if CK_DEBUG_TYPING // editor.commands.get( 'deleteForward' )
// @if CK_DEBUG_TYPING // ];
// @if CK_DEBUG_TYPING // }
66 changes: 42 additions & 24 deletions packages/ckeditor5-engine/src/view/domconverter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ import type EditableElement from './editableelement.js';
import type ViewTextProxy from './textproxy.js';
import type ViewRawElement from './rawelement.js';

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

type DomNode = globalThis.Node;
type DomElement = globalThis.HTMLElement;
type DomDocument = globalThis.Document;
Expand Down Expand Up @@ -1098,36 +1100,52 @@ export default class DomConverter {
public focus( viewEditable: EditableElement ): void {
const domEditable = this.mapViewToDom( viewEditable );

if ( domEditable && domEditable.ownerDocument.activeElement !== domEditable ) {
// Save the scrollX and scrollY positions before the focus.
const { scrollX, scrollY } = global.window;
const scrollPositions: Array<[ number, number ]> = [];
if ( !domEditable || domEditable.ownerDocument.activeElement === domEditable ) {
// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.info( ..._buildLogMessage( this, 'DomConverter',
// @if CK_DEBUG_TYPING // '%cDOM editable is already active or does not exist',
// @if CK_DEBUG_TYPING // 'font-style: italic'
// @if CK_DEBUG_TYPING // ) );
// @if CK_DEBUG_TYPING // }

// Save all scrollLeft and scrollTop values starting from domEditable up to
// document#documentElement.
forEachDomElementAncestor( domEditable, node => {
const { scrollLeft, scrollTop } = ( node as DomElement );
return;
}

scrollPositions.push( [ scrollLeft, scrollTop ] );
} );
// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.info( ..._buildLogMessage( this, 'DomConverter',
// @if CK_DEBUG_TYPING // 'Focus DOM editable:',
// @if CK_DEBUG_TYPING // { domEditable }
// @if CK_DEBUG_TYPING // ) );
// @if CK_DEBUG_TYPING // }

domEditable.focus();
// Save the scrollX and scrollY positions before the focus.
const { scrollX, scrollY } = global.window;
const scrollPositions: Array<[ number, number ]> = [];

// Restore scrollLeft and scrollTop values starting from domEditable up to
// document#documentElement.
// https://github.com/ckeditor/ckeditor5-engine/issues/951
// https://github.com/ckeditor/ckeditor5-engine/issues/957
forEachDomElementAncestor( domEditable, node => {
const [ scrollLeft, scrollTop ] = scrollPositions.shift() as [ number, number ];
// Save all scrollLeft and scrollTop values starting from domEditable up to
// document#documentElement.
forEachDomElementAncestor( domEditable, node => {
const { scrollLeft, scrollTop } = ( node as DomElement );

node.scrollLeft = scrollLeft;
node.scrollTop = scrollTop;
} );
scrollPositions.push( [ scrollLeft, scrollTop ] );
} );

// Restore the scrollX and scrollY positions after the focus.
// https://github.com/ckeditor/ckeditor5-engine/issues/951
global.window.scrollTo( scrollX, scrollY );
}
domEditable.focus();

// Restore scrollLeft and scrollTop values starting from domEditable up to
// document#documentElement.
// https://github.com/ckeditor/ckeditor5-engine/issues/951
// https://github.com/ckeditor/ckeditor5-engine/issues/957
forEachDomElementAncestor( domEditable, node => {
const [ scrollLeft, scrollTop ] = scrollPositions.shift() as [ number, number ];

node.scrollLeft = scrollLeft;
node.scrollTop = scrollTop;
} );

// Restore the scrollX and scrollY positions after the focus.
// https://github.com/ckeditor/ckeditor5-engine/issues/951
global.window.scrollTo( scrollX, scrollY );
}

/**
Expand Down
18 changes: 10 additions & 8 deletions packages/ckeditor5-engine/src/view/observer/compositionobserver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ 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' );
// @if CK_DEBUG_TYPING // const { _debouncedLine, _buildLogMessage } = require( '../../dev-utils/utils.js' );

/**
* {@link module:engine/view/document~Document#event:compositionstart Compositionstart},
Expand All @@ -36,20 +36,20 @@ export default class CompositionObserver extends DomEventObserver<'compositionst

document.on<ViewDocumentCompositionStartEvent>( 'compositionstart', () => {
// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.log( '%c[CompositionObserver] ' +
// @if CK_DEBUG_TYPING // '┌───────────────────────────── isComposing = true ─────────────────────────────┐',
// @if CK_DEBUG_TYPING // console.log( ..._buildLogMessage( this, 'CompositionObserver',
// @if CK_DEBUG_TYPING // '%c┌───────────────────────────── isComposing = true ─────────────────────────────┐',
// @if CK_DEBUG_TYPING // 'font-weight: bold; color: green'
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // ) );
// @if CK_DEBUG_TYPING // }
document.isComposing = true;
}, { priority: 'low' } );

document.on<ViewDocumentCompositionEndEvent>( 'compositionend', () => {
// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.log( '%c[CompositionObserver] ' +
// @if CK_DEBUG_TYPING // '└───────────────────────────── isComposing = false ─────────────────────────────┘',
// @if CK_DEBUG_TYPING // console.log( ..._buildLogMessage( this, 'CompositionObserver',
// @if CK_DEBUG_TYPING // '%c└───────────────────────────── isComposing = false ─────────────────────────────┘',
// @if CK_DEBUG_TYPING // 'font-weight: bold; color: green'
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // ) );
// @if CK_DEBUG_TYPING // }
document.isComposing = false;
}, { priority: 'low' } );
Expand All @@ -61,7 +61,9 @@ 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 // console.group( ..._buildLogMessage( this, 'CompositionObserver',
// @if CK_DEBUG_TYPING // `${ domEvent.type }`
// @if CK_DEBUG_TYPING // ) );
// @if CK_DEBUG_TYPING // }

this.fire( domEvent.type, domEvent, {
Expand Down
62 changes: 62 additions & 0 deletions packages/ckeditor5-engine/src/view/observer/focusobserver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import type DomEventData from './domeventdata.js';
import type View from '../view.js';
import type { ViewDocumentInputEvent } from './inputobserver.js';

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

/**
* {@link module:engine/view/document~Document#event:focus Focus}
* and {@link module:engine/view/document~Document#event:blur blur} events observer.
Expand Down Expand Up @@ -68,16 +70,48 @@ export default class FocusObserver extends DomEventObserver<'focus' | 'blur'> {
*/
public flush(): void {
if ( this._isFocusChanging ) {
// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // _debouncedLine();
// @if CK_DEBUG_TYPING // console.group( ..._buildLogMessage( this, 'FocusObserver',
// @if CK_DEBUG_TYPING // 'flush focus'
// @if CK_DEBUG_TYPING // ) );
// @if CK_DEBUG_TYPING // }

this._isFocusChanging = false;
this.document.isFocused = true;

// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.groupEnd();
// @if CK_DEBUG_TYPING // }
}
}

/**
* @inheritDoc
*/
public onDomEvent( domEvent: FocusEvent ): void {
// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // _debouncedLine();
// @if CK_DEBUG_TYPING // console.group( ..._buildLogMessage( this, 'FocusObserver',
// @if CK_DEBUG_TYPING // `${ domEvent.type } event`
// @if CK_DEBUG_TYPING // ) );
// @if CK_DEBUG_TYPING // console.info( ..._buildLogMessage( this, 'FocusObserver',
// @if CK_DEBUG_TYPING // 'DOM target:',
// @if CK_DEBUG_TYPING // { target: domEvent.target, relatedTarget: domEvent.relatedTarget }
// @if CK_DEBUG_TYPING // ) );
// @if CK_DEBUG_TYPING // const domSelection = window.getSelection();
// @if CK_DEBUG_TYPING // console.info( ..._buildLogMessage( this, 'FocusObserver',
// @if CK_DEBUG_TYPING // 'DOM Selection:',
// @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.fire( domEvent.type, domEvent );

// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.groupEnd();
// @if CK_DEBUG_TYPING // }
}

/**
Expand Down Expand Up @@ -105,8 +139,19 @@ export default class FocusObserver extends DomEventObserver<'focus' | 'blur'> {
// in a situation where `selectionchange` already caused selection change.
this._renderTimeoutId = setTimeout( () => {
this._renderTimeoutId = null;

// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.group( ..._buildLogMessage( this, 'FocusObserver',
// @if CK_DEBUG_TYPING // 'flush on timeout'
// @if CK_DEBUG_TYPING // ) );
// @if CK_DEBUG_TYPING // }

this.flush();
this.view.change( () => {} );

// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.groupEnd();
// @if CK_DEBUG_TYPING // }
}, 50 );
}

Expand All @@ -116,13 +161,30 @@ export default class FocusObserver extends DomEventObserver<'focus' | 'blur'> {
private _handleBlur( data: DomEventData<FocusEvent> ): void {
const selectedEditable = this.document.selection.editableElement;

// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.info( ..._buildLogMessage( this, 'FocusObserver',
// @if CK_DEBUG_TYPING // 'selectedEditable:',
// @if CK_DEBUG_TYPING // { selectedEditable }
// @if CK_DEBUG_TYPING // ) );
// @if CK_DEBUG_TYPING // }

if ( selectedEditable === null || selectedEditable === data.target ) {
// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.group( ..._buildLogMessage( this, 'FocusObserver',
// @if CK_DEBUG_TYPING // 'document no longer focused'
// @if CK_DEBUG_TYPING // ) );
// @if CK_DEBUG_TYPING // }

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( () => {} );

// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.groupEnd();
// @if CK_DEBUG_TYPING // }
}
}

Expand Down
48 changes: 29 additions & 19 deletions packages/ckeditor5-engine/src/view/observer/inputobserver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ 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' );
// @if CK_DEBUG_TYPING // const { _debouncedLine, _buildLogMessage } = require( '../../dev-utils/utils.js' );

/**
* Observer for events connected with data input.
Expand All @@ -33,9 +33,9 @@ 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 // );
// @if CK_DEBUG_TYPING // console.group( ..._buildLogMessage( this, 'InputObserver',
// @if CK_DEBUG_TYPING // `${ domEvent.type }: ${ domEvent.inputType }`
// @if CK_DEBUG_TYPING // ) );
// @if CK_DEBUG_TYPING // }

const domTargetRanges = domEvent.getTargetRanges();
Expand All @@ -54,17 +54,21 @@ export default class InputObserver extends DomEventObserver<'beforeinput'> {
data = domEvent.data;

// @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 // );
// @if CK_DEBUG_TYPING // console.info( ..._buildLogMessage( this, 'InputObserver',
// @if CK_DEBUG_TYPING // `%cevent data: %c${ JSON.stringify( data ) }`,
// @if CK_DEBUG_TYPING // 'font-weight: bold',
// @if CK_DEBUG_TYPING // '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 // );
// @if CK_DEBUG_TYPING // console.info( ..._buildLogMessage( this, 'InputObserver',
// @if CK_DEBUG_TYPING // `%cevent data transfer: %c${ JSON.stringify( data ) }`,
// @if CK_DEBUG_TYPING // 'font-weight: bold',
// @if CK_DEBUG_TYPING // 'color: blue;'
// @if CK_DEBUG_TYPING // ) );
// @if CK_DEBUG_TYPING // }
}

Expand All @@ -75,10 +79,12 @@ export default class InputObserver extends DomEventObserver<'beforeinput'> {
targetRanges = Array.from( viewDocument.selection.getRanges() );

// @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 // console.info( ..._buildLogMessage( this, 'InputObserver',
// @if CK_DEBUG_TYPING // '%cusing fake selection:',
// @if CK_DEBUG_TYPING // 'font-weight: bold',
// @if CK_DEBUG_TYPING // targetRanges,
// @if CK_DEBUG_TYPING // viewDocument.selection.isFake ? 'fake view selection' : 'fake DOM parent'
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // ) );
// @if CK_DEBUG_TYPING // }
} else if ( domTargetRanges.length ) {
targetRanges = domTargetRanges.map( domRange => {
Expand All @@ -97,9 +103,11 @@ export default class InputObserver extends DomEventObserver<'beforeinput'> {
} ).filter( ( range ): range is ViewRange => !!range );

// @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 // );
// @if CK_DEBUG_TYPING // console.info( ..._buildLogMessage( this, 'InputObserver',
// @if CK_DEBUG_TYPING // '%cusing target ranges:',
// @if CK_DEBUG_TYPING // 'font-weight: bold',
// @if CK_DEBUG_TYPING // targetRanges
// @if CK_DEBUG_TYPING // ) );
// @if CK_DEBUG_TYPING // }
}
// For Android devices we use a fallback to the current DOM selection, Android modifies it according
Expand All @@ -110,9 +118,11 @@ export default class InputObserver extends DomEventObserver<'beforeinput'> {
targetRanges = Array.from( view.domConverter.domSelectionToView( domSelection ).getRanges() );

// @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 // );
// @if CK_DEBUG_TYPING // console.info( ..._buildLogMessage( this, 'InputObserver',
// @if CK_DEBUG_TYPING // '%cusing selection ranges:',
// @if CK_DEBUG_TYPING // 'font-weight: bold',
// @if CK_DEBUG_TYPING // targetRanges
// @if CK_DEBUG_TYPING // ) );
// @if CK_DEBUG_TYPING // }
}

Expand Down
Loading