From be5dd5eea7389b5ccfa8fbb95f9f3c62b44c390c Mon Sep 17 00:00:00 2001 From: Illia Sheremetov Date: Tue, 1 Aug 2023 17:44:26 +0200 Subject: [PATCH 01/16] Fix (engine): detecting restricted objects in firefox --- .../src/view/observer/selectionobserver.ts | 22 ++++++++++ .../tests/view/observer/selectionobserver.js | 40 +++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/packages/ckeditor5-engine/src/view/observer/selectionobserver.ts b/packages/ckeditor5-engine/src/view/observer/selectionobserver.ts index b31a1ab2abd..d10b0e3513a 100644 --- a/packages/ckeditor5-engine/src/view/observer/selectionobserver.ts +++ b/packages/ckeditor5-engine/src/view/observer/selectionobserver.ts @@ -232,6 +232,10 @@ export default class SelectionObserver extends Observer { const domSelection = domDocument.defaultView!.getSelection()!; + if ( env.isGecko && this._isObjectRestricted( domSelection.getRangeAt( 0 ).startContainer ) ) { + return; + } + if ( this.checkShouldIgnoreEventFromTarget( domSelection.anchorNode! ) ) { return; } @@ -302,6 +306,24 @@ export default class SelectionObserver extends Observer { } } + // In firefox exists restricted objects. Trying get any property from restricted object, it will through an error. + // https://github.com/ckeditor/ckeditor5/issues/9635 + private _isObjectRestricted( obj: unknown ): boolean { + let isRestricted; + + try { + if ( Object.prototype.toString.call( obj ) ) { + isRestricted = false; + } else { + isRestricted = true; + } + } catch ( error ) { + isRestricted = true; + } + + return isRestricted; + } + /** * Clears `SelectionObserver` internal properties connected with preventing infinite loop. */ diff --git a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js index 32032666d5f..bcbf7400849 100644 --- a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js +++ b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js @@ -162,6 +162,46 @@ describe( 'SelectionObserver', () => { changeDomSelection(); } ); + it( 'should not pass non-valid object in firefox', done => { + testUtils.sinon.stub( env, 'isGecko' ).value( true ); + testUtils.sinon.stub( Object.prototype, 'toString' ).value( () => null ); + + const spy = sinon.spy( selectionObserver.mutationObserver, 'flush' ); + + changeDomSelection(); + setTimeout( () => { + sinon.assert.notCalled( spy ); + done(); + }, 100 ); + } ); + + it( 'should detect restricted object in firefox', done => { + testUtils.sinon.stub( env, 'isGecko' ).value( true ); + testUtils.sinon.stub( Object.prototype, 'toString' ).throws( + new Error( 'Permission denied to access property Symbol.toStringTag' ) + ); + + const spy = sinon.spy( selectionObserver.mutationObserver, 'flush' ); + + changeDomSelection(); + setTimeout( () => { + sinon.assert.notCalled( spy ); + done(); + }, 100 ); + } ); + + it( 'should pass valid object in firefox', done => { + testUtils.sinon.stub( env, 'isGecko' ).value( true ); + + const spy = sinon.spy( selectionObserver.mutationObserver, 'flush' ); + + changeDomSelection(); + setTimeout( () => { + sinon.assert.calledOnce( spy ); + done(); + }, 100 ); + } ); + it( 'should add only one #selectionChange listener to one document', done => { // Add second roots to ensure that listener is added once. createViewRoot( viewDocument, 'div', 'additional' ); From ea2baf9183de2bfcb2ca6f1bc6cefe8530b635d5 Mon Sep 17 00:00:00 2001 From: Illia Sheremetov Date: Tue, 1 Aug 2023 17:48:26 +0200 Subject: [PATCH 02/16] Test (engine): typo fix --- .../tests/view/observer/selectionobserver.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js index bcbf7400849..086e0cf8762 100644 --- a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js +++ b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js @@ -162,7 +162,7 @@ describe( 'SelectionObserver', () => { changeDomSelection(); } ); - it( 'should not pass non-valid object in firefox', done => { + it( 'should not pass non-valid object on Firefox', done => { testUtils.sinon.stub( env, 'isGecko' ).value( true ); testUtils.sinon.stub( Object.prototype, 'toString' ).value( () => null ); @@ -175,7 +175,7 @@ describe( 'SelectionObserver', () => { }, 100 ); } ); - it( 'should detect restricted object in firefox', done => { + it( 'should detect restricted object on Firefox', done => { testUtils.sinon.stub( env, 'isGecko' ).value( true ); testUtils.sinon.stub( Object.prototype, 'toString' ).throws( new Error( 'Permission denied to access property Symbol.toStringTag' ) @@ -190,7 +190,7 @@ describe( 'SelectionObserver', () => { }, 100 ); } ); - it( 'should pass valid object in firefox', done => { + it( 'should pass valid object on Firefox', done => { testUtils.sinon.stub( env, 'isGecko' ).value( true ); const spy = sinon.spy( selectionObserver.mutationObserver, 'flush' ); From 74d5f6dff926aadf165066a5d14ef128d5f0b5df Mon Sep 17 00:00:00 2001 From: Illia Sheremetov Date: Wed, 2 Aug 2023 12:06:09 +0200 Subject: [PATCH 03/16] Fix (engine): code review fixes --- .../src/view/observer/selectionobserver.ts | 39 ++++++------- .../tests/view/observer/selectionobserver.js | 55 ++++++++----------- 2 files changed, 42 insertions(+), 52 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/observer/selectionobserver.ts b/packages/ckeditor5-engine/src/view/observer/selectionobserver.ts index d10b0e3513a..8f670f5bdb4 100644 --- a/packages/ckeditor5-engine/src/view/observer/selectionobserver.ts +++ b/packages/ckeditor5-engine/src/view/observer/selectionobserver.ts @@ -232,7 +232,7 @@ export default class SelectionObserver extends Observer { const domSelection = domDocument.defaultView!.getSelection()!; - if ( env.isGecko && this._isObjectRestricted( domSelection.getRangeAt( 0 ).startContainer ) ) { + if ( isRestricted( domSelection ) ) { return; } @@ -306,24 +306,6 @@ export default class SelectionObserver extends Observer { } } - // In firefox exists restricted objects. Trying get any property from restricted object, it will through an error. - // https://github.com/ckeditor/ckeditor5/issues/9635 - private _isObjectRestricted( obj: unknown ): boolean { - let isRestricted; - - try { - if ( Object.prototype.toString.call( obj ) ) { - isRestricted = false; - } else { - isRestricted = true; - } - } catch ( error ) { - isRestricted = true; - } - - return isRestricted; - } - /** * Clears `SelectionObserver` internal properties connected with preventing infinite loop. */ @@ -353,6 +335,25 @@ export type ViewDocumentSelectionEventData = { domSelection: DomSelection | null; }; +// In firefox exists restricted objects. Trying get any property from restricted object, it will through an error. +// https://github.com/ckeditor/ckeditor5/issues/9635 +function isRestricted( domSelection: DomSelection ): boolean { + if ( !env.isGecko ) { + return false; + } + + let isRestricted = false; + + try { + const container = domSelection.getRangeAt( 0 ).startContainer; + Object.prototype.toString.call( container ); + } catch ( error ) { + isRestricted = true; + } + + return isRestricted; +} + /** * Fired when a selection has changed. This event is fired only when the selection change was the only change that happened * in the document, and the old selection is different then the new selection. diff --git a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js index 086e0cf8762..2667c24ffc5 100644 --- a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js +++ b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js @@ -162,44 +162,33 @@ describe( 'SelectionObserver', () => { changeDomSelection(); } ); - it( 'should not pass non-valid object on Firefox', done => { - testUtils.sinon.stub( env, 'isGecko' ).value( true ); - testUtils.sinon.stub( Object.prototype, 'toString' ).value( () => null ); + describe( 'Restricted objects handling in Gecko', () => { + it( 'should detect restricted object on Firefox', done => { + testUtils.sinon.stub( env, 'isGecko' ).value( true ); + testUtils.sinon.stub( Object.prototype, 'toString' ).throws( + new Error( 'Permission denied to access property Symbol.toStringTag' ) + ); - const spy = sinon.spy( selectionObserver.mutationObserver, 'flush' ); + const spy = sinon.spy( selectionObserver.mutationObserver, 'flush' ); - changeDomSelection(); - setTimeout( () => { - sinon.assert.notCalled( spy ); - done(); - }, 100 ); - } ); - - it( 'should detect restricted object on Firefox', done => { - testUtils.sinon.stub( env, 'isGecko' ).value( true ); - testUtils.sinon.stub( Object.prototype, 'toString' ).throws( - new Error( 'Permission denied to access property Symbol.toStringTag' ) - ); - - const spy = sinon.spy( selectionObserver.mutationObserver, 'flush' ); - - changeDomSelection(); - setTimeout( () => { - sinon.assert.notCalled( spy ); - done(); - }, 100 ); - } ); + changeDomSelection(); + setTimeout( () => { + sinon.assert.notCalled( spy ); + done(); + }, 100 ); + } ); - it( 'should pass valid object on Firefox', done => { - testUtils.sinon.stub( env, 'isGecko' ).value( true ); + it( 'should pass valid object on Firefox', done => { + testUtils.sinon.stub( env, 'isGecko' ).value( true ); - const spy = sinon.spy( selectionObserver.mutationObserver, 'flush' ); + const spy = sinon.spy( selectionObserver.mutationObserver, 'flush' ); - changeDomSelection(); - setTimeout( () => { - sinon.assert.calledOnce( spy ); - done(); - }, 100 ); + changeDomSelection(); + setTimeout( () => { + sinon.assert.calledOnce( spy ); + done(); + }, 100 ); + } ); } ); it( 'should add only one #selectionChange listener to one document', done => { From 6060d9e39a027bccff2356053ac27f52211b871a Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 2 Aug 2023 16:48:57 +0200 Subject: [PATCH 04/16] Code refactoring. --- .../src/view/observer/selectionobserver.ts | 39 +++++++++---------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/observer/selectionobserver.ts b/packages/ckeditor5-engine/src/view/observer/selectionobserver.ts index 8f670f5bdb4..86e1b6b723a 100644 --- a/packages/ckeditor5-engine/src/view/observer/selectionobserver.ts +++ b/packages/ckeditor5-engine/src/view/observer/selectionobserver.ts @@ -232,7 +232,7 @@ export default class SelectionObserver extends Observer { const domSelection = domDocument.defaultView!.getSelection()!; - if ( isRestricted( domSelection ) ) { + if ( isGeckoRestrictedDomSelection( domSelection ) ) { return; } @@ -335,25 +335,6 @@ export type ViewDocumentSelectionEventData = { domSelection: DomSelection | null; }; -// In firefox exists restricted objects. Trying get any property from restricted object, it will through an error. -// https://github.com/ckeditor/ckeditor5/issues/9635 -function isRestricted( domSelection: DomSelection ): boolean { - if ( !env.isGecko ) { - return false; - } - - let isRestricted = false; - - try { - const container = domSelection.getRangeAt( 0 ).startContainer; - Object.prototype.toString.call( container ); - } catch ( error ) { - isRestricted = true; - } - - return isRestricted; -} - /** * Fired when a selection has changed. This event is fired only when the selection change was the only change that happened * in the document, and the old selection is different then the new selection. @@ -386,3 +367,21 @@ export type ViewDocumentSelectionChangeDoneEvent = { name: 'selectionChangeDone'; args: [ ViewDocumentSelectionEventData ]; }; + +// I certain cases, Firefox mysteriously assigns so called "restricted objects" to native DOM Range properties. +// Any attempt at accessing restricted object's properties causes errors. +// https://github.com/ckeditor/ckeditor5/issues/9635 +function isGeckoRestrictedDomSelection( domSelection: DomSelection ): boolean { + if ( !env.isGecko ) { + return false; + } + + try { + const container = domSelection.getRangeAt( 0 ).startContainer; + Object.prototype.toString.call( container ); + } catch ( error ) { + return true; + } + + return false; +} From 141942795555ef4d132ea3e5d09cbc6ef18cad84 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 2 Aug 2023 16:49:59 +0200 Subject: [PATCH 05/16] Tests: improvements to tests descriptions. --- .../tests/view/observer/selectionobserver.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js index 2667c24ffc5..c19d44fedd0 100644 --- a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js +++ b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js @@ -163,7 +163,7 @@ describe( 'SelectionObserver', () => { } ); describe( 'Restricted objects handling in Gecko', () => { - it( 'should detect restricted object on Firefox', done => { + it( 'should detect "restricted objects" in Firefox DOM ranges and prevent an error being thrown', done => { testUtils.sinon.stub( env, 'isGecko' ).value( true ); testUtils.sinon.stub( Object.prototype, 'toString' ).throws( new Error( 'Permission denied to access property Symbol.toStringTag' ) @@ -172,18 +172,20 @@ describe( 'SelectionObserver', () => { const spy = sinon.spy( selectionObserver.mutationObserver, 'flush' ); changeDomSelection(); + setTimeout( () => { sinon.assert.notCalled( spy ); done(); }, 100 ); } ); - it( 'should pass valid object on Firefox', done => { + it( 'should do nothing in Firefox if the DOM selection is correct', done => { testUtils.sinon.stub( env, 'isGecko' ).value( true ); const spy = sinon.spy( selectionObserver.mutationObserver, 'flush' ); changeDomSelection(); + setTimeout( () => { sinon.assert.calledOnce( spy ); done(); From df10cb7c0e25214912c751dbc80c9318a5fc9e01 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 2 Aug 2023 17:00:40 +0200 Subject: [PATCH 06/16] Tests: Made the restricted object test more specific. --- .../tests/view/observer/selectionobserver.js | 39 +++++++++---------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js index c19d44fedd0..0e3e22efba8 100644 --- a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js +++ b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -/* globals setTimeout, document, console, Event */ +/* globals setTimeout, document, console, Event, Selection */ import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; @@ -163,33 +163,30 @@ describe( 'SelectionObserver', () => { } ); describe( 'Restricted objects handling in Gecko', () => { - it( 'should detect "restricted objects" in Firefox DOM ranges and prevent an error being thrown', done => { + beforeEach( () => { testUtils.sinon.stub( env, 'isGecko' ).value( true ); - testUtils.sinon.stub( Object.prototype, 'toString' ).throws( - new Error( 'Permission denied to access property Symbol.toStringTag' ) - ); + } ); - const spy = sinon.spy( selectionObserver.mutationObserver, 'flush' ); + it( 'should detect "restricted objects" in Firefox DOM ranges and prevent an error being thrown', done => { + const domRangeStubWithRestrictedObject = domDocument.createRange(); - changeDomSelection(); + sinon.stub( domRangeStubWithRestrictedObject, 'startContainer' ).get( () => { + throw new Error( 'Permission denied to access property Symbol.toStringTag' ); + } ); - setTimeout( () => { - sinon.assert.notCalled( spy ); - done(); - }, 100 ); + testUtils.sinon.stub( Selection.prototype, 'getRangeAt' ).returns( domRangeStubWithRestrictedObject ); + + expect( () => { + changeDomSelection(); + setTimeout( done, 100 ); + } ).to.not.throw(); } ); it( 'should do nothing in Firefox if the DOM selection is correct', done => { - testUtils.sinon.stub( env, 'isGecko' ).value( true ); - - const spy = sinon.spy( selectionObserver.mutationObserver, 'flush' ); - - changeDomSelection(); - - setTimeout( () => { - sinon.assert.calledOnce( spy ); - done(); - }, 100 ); + expect( () => { + changeDomSelection(); + setTimeout( done, 100 ); + } ).to.not.throw(); } ); } ); From 6a5460622c2260051800d89365cb90260b4b0652 Mon Sep 17 00:00:00 2001 From: Illia Sheremetov Date: Thu, 3 Aug 2023 13:03:55 +0200 Subject: [PATCH 07/16] Fix (engine): moving implementation to domconverter --- .../ckeditor5-engine/src/view/domconverter.ts | 25 ++++++++++++++++++- .../src/view/observer/selectionobserver.ts | 22 ---------------- 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/domconverter.ts b/packages/ckeditor5-engine/src/view/domconverter.ts index d7c0ae8357a..2a0791a8372 100644 --- a/packages/ckeditor5-engine/src/view/domconverter.ts +++ b/packages/ckeditor5-engine/src/view/domconverter.ts @@ -31,7 +31,8 @@ import { isText, isComment, isValidAttributeName, - first + first, + env } from '@ckeditor/ckeditor5-utils'; import type ViewNode from './node'; @@ -732,6 +733,10 @@ export default class DomConverter { * @returns View selection. */ public domSelectionToView( domSelection: DomSelection ): ViewSelection { + if ( isGeckoRestrictedDomSelection( domSelection ) ) { + return new ViewSelection( [] ); + } + // DOM selection might be placed in fake selection container. // If container contains fake selection - return corresponding view selection. if ( domSelection.rangeCount === 1 ) { @@ -1787,6 +1792,24 @@ function _logUnsafeElement( elementName: string ): void { } } +// In certain cases, Firefox mysteriously assigns so called "restricted objects" to native DOM Range properties. +// Any attempt at accessing restricted object's properties causes errors. +// https://github.com/ckeditor/ckeditor5/issues/9635 +function isGeckoRestrictedDomSelection( domSelection: DomSelection ): boolean { + if ( !env.isGecko ) { + return false; + } + + try { + const container = domSelection.getRangeAt( 0 ).startContainer; + Object.prototype.toString.call( container ); + } catch ( error ) { + return true; + } + + return false; +} + /** * Enum representing the type of the block filler. * diff --git a/packages/ckeditor5-engine/src/view/observer/selectionobserver.ts b/packages/ckeditor5-engine/src/view/observer/selectionobserver.ts index 86e1b6b723a..b31a1ab2abd 100644 --- a/packages/ckeditor5-engine/src/view/observer/selectionobserver.ts +++ b/packages/ckeditor5-engine/src/view/observer/selectionobserver.ts @@ -232,10 +232,6 @@ export default class SelectionObserver extends Observer { const domSelection = domDocument.defaultView!.getSelection()!; - if ( isGeckoRestrictedDomSelection( domSelection ) ) { - return; - } - if ( this.checkShouldIgnoreEventFromTarget( domSelection.anchorNode! ) ) { return; } @@ -367,21 +363,3 @@ export type ViewDocumentSelectionChangeDoneEvent = { name: 'selectionChangeDone'; args: [ ViewDocumentSelectionEventData ]; }; - -// I certain cases, Firefox mysteriously assigns so called "restricted objects" to native DOM Range properties. -// Any attempt at accessing restricted object's properties causes errors. -// https://github.com/ckeditor/ckeditor5/issues/9635 -function isGeckoRestrictedDomSelection( domSelection: DomSelection ): boolean { - if ( !env.isGecko ) { - return false; - } - - try { - const container = domSelection.getRangeAt( 0 ).startContainer; - Object.prototype.toString.call( container ); - } catch ( error ) { - return true; - } - - return false; -} From d704e48926f0b141ba8ee8b9ee25ff8bf95aabd1 Mon Sep 17 00:00:00 2001 From: Illia Sheremetov Date: Thu, 3 Aug 2023 16:14:28 +0200 Subject: [PATCH 08/16] Fix (engine): code review fixes --- .../ckeditor5-engine/src/view/domconverter.ts | 16 +++++++++++----- .../tests/view/domconverter/dom-to-view.js | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/domconverter.ts b/packages/ckeditor5-engine/src/view/domconverter.ts index 2a0791a8372..6f66be40372 100644 --- a/packages/ckeditor5-engine/src/view/domconverter.ts +++ b/packages/ckeditor5-engine/src/view/domconverter.ts @@ -733,6 +733,7 @@ export default class DomConverter { * @returns View selection. */ public domSelectionToView( domSelection: DomSelection ): ViewSelection { + // https://github.com/ckeditor/ckeditor5/issues/9635 if ( isGeckoRestrictedDomSelection( domSelection ) ) { return new ViewSelection( [] ); } @@ -1792,17 +1793,22 @@ function _logUnsafeElement( elementName: string ): void { } } -// In certain cases, Firefox mysteriously assigns so called "restricted objects" to native DOM Range properties. -// Any attempt at accessing restricted object's properties causes errors. -// https://github.com/ckeditor/ckeditor5/issues/9635 +/** + * In certain cases, Firefox mysteriously assigns so called "restricted objects" to native DOM Range properties. + * Any attempt at accessing restricted object's properties causes errors. + * https://github.com/ckeditor/ckeditor5/issues/9635 + */ function isGeckoRestrictedDomSelection( domSelection: DomSelection ): boolean { if ( !env.isGecko ) { return false; } try { - const container = domSelection.getRangeAt( 0 ).startContainer; - Object.prototype.toString.call( container ); + if ( domSelection.rangeCount ) { + const container = domSelection.getRangeAt( 0 ).startContainer; + + Object.prototype.toString.call( container ); + } } catch ( error ) { return true; } diff --git a/packages/ckeditor5-engine/tests/view/domconverter/dom-to-view.js b/packages/ckeditor5-engine/tests/view/domconverter/dom-to-view.js index cb7d8a80782..04308630750 100644 --- a/packages/ckeditor5-engine/tests/view/domconverter/dom-to-view.js +++ b/packages/ckeditor5-engine/tests/view/domconverter/dom-to-view.js @@ -18,6 +18,7 @@ import { parse, stringify } from '../../../src/dev-utils/view'; import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; import count from '@ckeditor/ckeditor5-utils/src/count'; import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement'; +import env from '@ckeditor/ckeditor5-utils/src/env'; describe( 'DomConverter', () => { let converter, viewDocument; @@ -1287,5 +1288,23 @@ describe( 'DomConverter', () => { domContainer.remove(); } ); + + it( 'should detect "restricted objects" in Firefox DOM ranges and prevent an error being thrown', () => { + const domRangeStubWithRestrictedObject = document.createRange(); + + testUtils.sinon.stub( env, 'isGecko' ).value( true ); + + sinon.stub( domRangeStubWithRestrictedObject, 'startContainer' ).get( () => { + throw new Error( 'Permission denied to access property Symbol.toStringTag' ); + } ); + + const domSelection = document.getSelection(); + + testUtils.sinon.stub( domSelection, 'getRangeAt' ).withArgs( 0 ).returns( domRangeStubWithRestrictedObject ); + + expect( () => { + converter.domSelectionToView( domSelection ); + } ).to.not.throw(); + } ); } ); } ); From 24b1e1be7e63186ddcf83c352abae8ed6c5b39f0 Mon Sep 17 00:00:00 2001 From: Illia Sheremetov Date: Wed, 9 Aug 2023 11:38:23 +0200 Subject: [PATCH 09/16] Fix (engine): code review fix --- packages/ckeditor5-engine/src/view/domconverter.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/domconverter.ts b/packages/ckeditor5-engine/src/view/domconverter.ts index 6f66be40372..deebd3c70a6 100644 --- a/packages/ckeditor5-engine/src/view/domconverter.ts +++ b/packages/ckeditor5-engine/src/view/domconverter.ts @@ -1799,16 +1799,14 @@ function _logUnsafeElement( elementName: string ): void { * https://github.com/ckeditor/ckeditor5/issues/9635 */ function isGeckoRestrictedDomSelection( domSelection: DomSelection ): boolean { - if ( !env.isGecko ) { + if ( !env.isGecko || !domSelection.rangeCount ) { return false; } try { - if ( domSelection.rangeCount ) { - const container = domSelection.getRangeAt( 0 ).startContainer; + const container = domSelection.getRangeAt( 0 ).startContainer; - Object.prototype.toString.call( container ); - } + Object.prototype.toString.call( container ); } catch ( error ) { return true; } From 1dfc78349c07e306048912b50a9ad2db659b9df7 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Mon, 21 Aug 2023 17:10:04 +0200 Subject: [PATCH 10/16] Updated tests. --- .../ckeditor5-engine/src/view/domconverter.ts | 10 ++- .../tests/view/domconverter/dom-to-view.js | 87 ++++++++++++++++--- 2 files changed, 84 insertions(+), 13 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/domconverter.ts b/packages/ckeditor5-engine/src/view/domconverter.ts index deebd3c70a6..7b9aa9f1a01 100644 --- a/packages/ckeditor5-engine/src/view/domconverter.ts +++ b/packages/ckeditor5-engine/src/view/domconverter.ts @@ -1799,13 +1799,17 @@ function _logUnsafeElement( elementName: string ): void { * https://github.com/ckeditor/ckeditor5/issues/9635 */ function isGeckoRestrictedDomSelection( domSelection: DomSelection ): boolean { - if ( !env.isGecko || !domSelection.rangeCount ) { + if ( !env.isGecko ) { return false; } - try { - const container = domSelection.getRangeAt( 0 ).startContainer; + if ( !domSelection.rangeCount ) { + return false; + } + const container = domSelection.getRangeAt( 0 ).startContainer; + + try { Object.prototype.toString.call( container ); } catch ( error ) { return true; diff --git a/packages/ckeditor5-engine/tests/view/domconverter/dom-to-view.js b/packages/ckeditor5-engine/tests/view/domconverter/dom-to-view.js index 04308630750..9fed9fa73e2 100644 --- a/packages/ckeditor5-engine/tests/view/domconverter/dom-to-view.js +++ b/packages/ckeditor5-engine/tests/view/domconverter/dom-to-view.js @@ -9,6 +9,7 @@ import ViewElement from '../../../src/view/element'; import ViewUIElement from '../../../src/view/uielement'; import ViewDocument from '../../../src/view/document'; import ViewDocumentSelection from '../../../src/view/documentselection'; +import ViewSelection from '../../../src/view/selection'; import DomConverter from '../../../src/view/domconverter'; import ViewDocumentFragment from '../../../src/view/documentfragment'; import { BR_FILLER, INLINE_FILLER, INLINE_FILLER_LENGTH, NBSP_FILLER } from '../../../src/view/filler'; @@ -1289,22 +1290,88 @@ describe( 'DomConverter', () => { domContainer.remove(); } ); - it( 'should detect "restricted objects" in Firefox DOM ranges and prevent an error being thrown', () => { - const domRangeStubWithRestrictedObject = document.createRange(); + // See https://github.com/ckeditor/ckeditor5/issues/9635. + describe( 'restricted objects in Firefox', () => { + it( 'not throw if selection is anchored in the restricted object', () => { + testUtils.sinon.stub( env, 'isGecko' ).value( true ); - testUtils.sinon.stub( env, 'isGecko' ).value( true ); + const domFoo = document.createTextNode( 'foo' ); + const domP = createElement( document, 'p', null, [ domFoo ] ); - sinon.stub( domRangeStubWithRestrictedObject, 'startContainer' ).get( () => { - throw new Error( 'Permission denied to access property Symbol.toStringTag' ); + const viewP = parse( '

foo

' ); + + converter.bindElements( domP, viewP ); + + document.body.appendChild( domP ); + + const domRange = document.createRange(); + domRange.setStart( domFoo, 1 ); + domRange.setEnd( domFoo, 2 ); + + const domSelection = document.getSelection(); + domSelection.removeAllRanges(); + domSelection.addRange( domRange ); + + const viewSelection = converter.domSelectionToView( domSelection ); + + expect( viewSelection.rangeCount ).to.equal( 1 ); + expect( stringify( viewP, viewSelection.getFirstRange() ) ).to.equal( '

f{o}o

' ); + + // Now we know that there should be a valid view range. So let's test if the DOM node throws an error. + sinon.stub( domFoo, Symbol.toStringTag ).get( () => { + throw new Error( 'Permission denied to access property Symbol.toStringTag' ); + } ); + + let result = null; + + expect( () => { + result = converter.domSelectionToView( domSelection ); + } ).to.not.throw(); + + expect( result instanceof ViewSelection ).to.be.true; + expect( result.rangeCount ).to.equal( 0 ); + + domP.remove(); } ); - const domSelection = document.getSelection(); + it( 'should not check if restricted object on non-Gecko browsers', () => { + testUtils.sinon.stub( env, 'isGecko' ).value( false ); - testUtils.sinon.stub( domSelection, 'getRangeAt' ).withArgs( 0 ).returns( domRangeStubWithRestrictedObject ); + const domFoo = document.createTextNode( 'foo' ); + const domP = createElement( document, 'p', null, [ domFoo ] ); - expect( () => { - converter.domSelectionToView( domSelection ); - } ).to.not.throw(); + const viewP = parse( '

foo

' ); + + converter.bindElements( domP, viewP ); + + document.body.appendChild( domP ); + + const domRange = document.createRange(); + domRange.setStart( domFoo, 1 ); + domRange.setEnd( domFoo, 2 ); + + const domSelection = document.getSelection(); + domSelection.removeAllRanges(); + domSelection.addRange( domRange ); + + const viewSelection = converter.domSelectionToView( domSelection ); + + expect( viewSelection.rangeCount ).to.equal( 1 ); + expect( stringify( viewP, viewSelection.getFirstRange() ) ).to.equal( '

f{o}o

' ); + + domP.remove(); + } ); + + it( 'should convert empty selection to empty selection (in Gecko)', () => { + testUtils.sinon.stub( env, 'isGecko' ).value( true ); + + const domSelection = document.getSelection(); + domSelection.removeAllRanges(); + + const viewSelection = converter.domSelectionToView( domSelection ); + + expect( viewSelection.rangeCount ).to.equal( 0 ); + } ); } ); } ); } ); From 10fc9f2b4c861416877fdef06910d8a509ff41e6 Mon Sep 17 00:00:00 2001 From: Illia Sheremetov Date: Tue, 22 Aug 2023 12:01:57 +0200 Subject: [PATCH 11/16] Fix (engine): code review fixes --- .../ckeditor5-engine/src/view/domconverter.ts | 4 +-- .../tests/view/observer/selectionobserver.js | 30 +------------------ 2 files changed, 3 insertions(+), 31 deletions(-) diff --git a/packages/ckeditor5-engine/src/view/domconverter.ts b/packages/ckeditor5-engine/src/view/domconverter.ts index 7b9aa9f1a01..46410da8ec4 100644 --- a/packages/ckeditor5-engine/src/view/domconverter.ts +++ b/packages/ckeditor5-engine/src/view/domconverter.ts @@ -733,7 +733,7 @@ export default class DomConverter { * @returns View selection. */ public domSelectionToView( domSelection: DomSelection ): ViewSelection { - // https://github.com/ckeditor/ckeditor5/issues/9635 + // See: https://github.com/ckeditor/ckeditor5/issues/9635. if ( isGeckoRestrictedDomSelection( domSelection ) ) { return new ViewSelection( [] ); } @@ -1796,7 +1796,7 @@ function _logUnsafeElement( elementName: string ): void { /** * In certain cases, Firefox mysteriously assigns so called "restricted objects" to native DOM Range properties. * Any attempt at accessing restricted object's properties causes errors. - * https://github.com/ckeditor/ckeditor5/issues/9635 + * See: https://github.com/ckeditor/ckeditor5/issues/9635. */ function isGeckoRestrictedDomSelection( domSelection: DomSelection ): boolean { if ( !env.isGecko ) { diff --git a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js index 0e3e22efba8..32032666d5f 100644 --- a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js +++ b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -/* globals setTimeout, document, console, Event, Selection */ +/* globals setTimeout, document, console, Event */ import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; @@ -162,34 +162,6 @@ describe( 'SelectionObserver', () => { changeDomSelection(); } ); - describe( 'Restricted objects handling in Gecko', () => { - beforeEach( () => { - testUtils.sinon.stub( env, 'isGecko' ).value( true ); - } ); - - it( 'should detect "restricted objects" in Firefox DOM ranges and prevent an error being thrown', done => { - const domRangeStubWithRestrictedObject = domDocument.createRange(); - - sinon.stub( domRangeStubWithRestrictedObject, 'startContainer' ).get( () => { - throw new Error( 'Permission denied to access property Symbol.toStringTag' ); - } ); - - testUtils.sinon.stub( Selection.prototype, 'getRangeAt' ).returns( domRangeStubWithRestrictedObject ); - - expect( () => { - changeDomSelection(); - setTimeout( done, 100 ); - } ).to.not.throw(); - } ); - - it( 'should do nothing in Firefox if the DOM selection is correct', done => { - expect( () => { - changeDomSelection(); - setTimeout( done, 100 ); - } ).to.not.throw(); - } ); - } ); - it( 'should add only one #selectionChange listener to one document', done => { // Add second roots to ensure that listener is added once. createViewRoot( viewDocument, 'div', 'additional' ); From 1ff25a4b5cb83213a6c1c4a059246fe58c3672af Mon Sep 17 00:00:00 2001 From: Illia Sheremetov Date: Tue, 22 Aug 2023 13:09:00 +0200 Subject: [PATCH 12/16] Fix (engine): fix test --- .../tests/view/observer/selectionobserver.js | 80 ++++++++++++++++++- 1 file changed, 78 insertions(+), 2 deletions(-) diff --git a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js index 32032666d5f..0160d69f9ce 100644 --- a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js +++ b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js @@ -15,12 +15,14 @@ import SelectionObserver from '../../../src/view/observer/selectionobserver'; import FocusObserver from '../../../src/view/observer/focusobserver'; import MutationObserver from '../../../src/view/observer/mutationobserver'; import createViewRoot from '../_utils/createroot'; -import { parse } from '../../../src/dev-utils/view'; +import DomConverter from '../../../src/view/domconverter'; +import { parse, stringify } from '../../../src/dev-utils/view'; import { StylesProcessor } from '../../../src/view/stylesmap'; +import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement'; import env from '@ckeditor/ckeditor5-utils/src/env'; describe( 'SelectionObserver', () => { - let view, viewDocument, viewRoot, selectionObserver, domRoot, domMain, domDocument; + let view, viewDocument, viewRoot, selectionObserver, domRoot, domMain, domDocument, converter; testUtils.createSinonSandbox(); @@ -30,6 +32,7 @@ describe( 'SelectionObserver', () => { domRoot.innerHTML = '
'; domMain = domRoot.childNodes[ 0 ]; domDocument.body.appendChild( domRoot ); + converter = new DomConverter( viewDocument ); view = new View( new StylesProcessor() ); viewDocument = view.document; @@ -162,6 +165,79 @@ describe( 'SelectionObserver', () => { changeDomSelection(); } ); + describe( 'Restricted objects handling in Gecko', () => { + beforeEach( () => { + testUtils.sinon.stub( env, 'isGecko' ).value( true ); + } ); + + it( 'should detect "restricted objects" in Firefox DOM ranges and prevent an error being thrown', () => { + testUtils.sinon.stub( env, 'isGecko' ).value( true ); + + const domFoo = document.createTextNode( 'foo' ); + const domP = createElement( document, 'p', null, [ domFoo ] ); + + const viewP = parse( '

foo

' ); + + converter.bindElements( domP, viewP ); + + document.body.appendChild( domP ); + + const domRange = document.createRange(); + domRange.setStart( domFoo, 1 ); + domRange.setEnd( domFoo, 2 ); + + const domSelection = document.getSelection(); + domSelection.removeAllRanges(); + domSelection.addRange( domRange ); + + const viewSelection = converter.domSelectionToView( domSelection ); + + expect( viewSelection.rangeCount ).to.equal( 1 ); + expect( stringify( viewP, viewSelection.getFirstRange() ) ).to.equal( '

f{o}o

' ); + + // Now we know that there should be a valid view range. So let's test if the DOM node throws an error. + sinon.stub( domFoo, Symbol.toStringTag ).get( () => { + throw new Error( 'Permission denied to access property Symbol.toStringTag' ); + } ); + + let result = null; + + expect( () => { + result = converter.domSelectionToView( domSelection ); + } ).to.not.throw(); + + expect( result instanceof ViewSelection ).to.be.true; + expect( result.rangeCount ).to.equal( 0 ); + + domP.remove(); + } ); + + it( 'should do nothing in Firefox if the DOM selection is correct', done => { + expect( () => { + const domFoo = document.createTextNode( 'foo' ); + const domP = createElement( document, 'p', null, [ domFoo ] ); + + const viewP = parse( '

foo

' ); + + converter.bindElements( domP, viewP ); + + document.body.appendChild( domP ); + + const domRange = document.createRange(); + domRange.setStart( domFoo, 1 ); + domRange.setEnd( domFoo, 2 ); + + const domSelection = document.getSelection(); + domSelection.removeAllRanges(); + domSelection.addRange( domRange ); + + converter.domSelectionToView( domSelection ); + + setTimeout( done, 100 ); + } ).to.not.throw(); + } ); + } ); + it( 'should add only one #selectionChange listener to one document', done => { // Add second roots to ensure that listener is added once. createViewRoot( viewDocument, 'div', 'additional' ); From 34ace0193eb47767c0fab8fbaf7bb6910101be95 Mon Sep 17 00:00:00 2001 From: Illia Sheremetov Date: Tue, 22 Aug 2023 13:48:56 +0200 Subject: [PATCH 13/16] Fix (engine): fix test --- .../tests/view/observer/selectionobserver.js | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js index 0160d69f9ce..dcdd301c200 100644 --- a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js +++ b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js @@ -213,28 +213,30 @@ describe( 'SelectionObserver', () => { } ); it( 'should do nothing in Firefox if the DOM selection is correct', done => { - expect( () => { - const domFoo = document.createTextNode( 'foo' ); - const domP = createElement( document, 'p', null, [ domFoo ] ); + const domFoo = document.createTextNode( 'foo' ); + const domP = createElement( document, 'p', null, [ domFoo ] ); - const viewP = parse( '

foo

' ); + const viewP = parse( '

foo

' ); - converter.bindElements( domP, viewP ); + converter.bindElements( domP, viewP ); - document.body.appendChild( domP ); + document.body.appendChild( domP ); - const domRange = document.createRange(); - domRange.setStart( domFoo, 1 ); - domRange.setEnd( domFoo, 2 ); + const domRange = document.createRange(); + domRange.setStart( domFoo, 1 ); + domRange.setEnd( domFoo, 2 ); - const domSelection = document.getSelection(); - domSelection.removeAllRanges(); - domSelection.addRange( domRange ); + const domSelection = document.getSelection(); + domSelection.removeAllRanges(); + domSelection.addRange( domRange ); + expect( () => { converter.domSelectionToView( domSelection ); setTimeout( done, 100 ); } ).to.not.throw(); + + domP.remove(); } ); } ); From 7b953f139e08711943dc21961e56b96c5860a853 Mon Sep 17 00:00:00 2001 From: Illia Sheremetov Date: Tue, 29 Aug 2023 11:33:33 +0200 Subject: [PATCH 14/16] Test (engine): code review fix --- .../tests/view/observer/selectionobserver.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js index dcdd301c200..eca4c09cd60 100644 --- a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js +++ b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js @@ -15,14 +15,13 @@ import SelectionObserver from '../../../src/view/observer/selectionobserver'; import FocusObserver from '../../../src/view/observer/focusobserver'; import MutationObserver from '../../../src/view/observer/mutationobserver'; import createViewRoot from '../_utils/createroot'; -import DomConverter from '../../../src/view/domconverter'; import { parse, stringify } from '../../../src/dev-utils/view'; import { StylesProcessor } from '../../../src/view/stylesmap'; import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement'; import env from '@ckeditor/ckeditor5-utils/src/env'; describe( 'SelectionObserver', () => { - let view, viewDocument, viewRoot, selectionObserver, domRoot, domMain, domDocument, converter; + let view, viewDocument, viewRoot, selectionObserver, domRoot, domMain, domDocument; testUtils.createSinonSandbox(); @@ -32,8 +31,6 @@ describe( 'SelectionObserver', () => { domRoot.innerHTML = '
'; domMain = domRoot.childNodes[ 0 ]; domDocument.body.appendChild( domRoot ); - converter = new DomConverter( viewDocument ); - view = new View( new StylesProcessor() ); viewDocument = view.document; createViewRoot( viewDocument ); @@ -175,6 +172,7 @@ describe( 'SelectionObserver', () => { const domFoo = document.createTextNode( 'foo' ); const domP = createElement( document, 'p', null, [ domFoo ] ); + const converter = view.domConverter; const viewP = parse( '

foo

' ); @@ -215,6 +213,7 @@ describe( 'SelectionObserver', () => { it( 'should do nothing in Firefox if the DOM selection is correct', done => { const domFoo = document.createTextNode( 'foo' ); const domP = createElement( document, 'p', null, [ domFoo ] ); + const converter = view.domConverter; const viewP = parse( '

foo

' ); From 95afdeb4260dcae4a9562fde5d01899c0324497d Mon Sep 17 00:00:00 2001 From: Illia Sheremetov Date: Wed, 30 Aug 2023 09:58:35 +0200 Subject: [PATCH 15/16] Test (engine): test fix --- .../tests/view/observer/selectionobserver.js | 82 +++---------------- 1 file changed, 11 insertions(+), 71 deletions(-) diff --git a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js index eca4c09cd60..7ebe11a9de7 100644 --- a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js +++ b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js @@ -15,9 +15,8 @@ import SelectionObserver from '../../../src/view/observer/selectionobserver'; import FocusObserver from '../../../src/view/observer/focusobserver'; import MutationObserver from '../../../src/view/observer/mutationobserver'; import createViewRoot from '../_utils/createroot'; -import { parse, stringify } from '../../../src/dev-utils/view'; +import { parse } from '../../../src/dev-utils/view'; import { StylesProcessor } from '../../../src/view/stylesmap'; -import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement'; import env from '@ckeditor/ckeditor5-utils/src/env'; describe( 'SelectionObserver', () => { @@ -162,81 +161,23 @@ describe( 'SelectionObserver', () => { changeDomSelection(); } ); - describe( 'Restricted objects handling in Gecko', () => { - beforeEach( () => { - testUtils.sinon.stub( env, 'isGecko' ).value( true ); - } ); - - it( 'should detect "restricted objects" in Firefox DOM ranges and prevent an error being thrown', () => { - testUtils.sinon.stub( env, 'isGecko' ).value( true ); - - const domFoo = document.createTextNode( 'foo' ); - const domP = createElement( document, 'p', null, [ domFoo ] ); - const converter = view.domConverter; - - const viewP = parse( '

foo

' ); - - converter.bindElements( domP, viewP ); - - document.body.appendChild( domP ); - - const domRange = document.createRange(); - domRange.setStart( domFoo, 1 ); - domRange.setEnd( domFoo, 2 ); - - const domSelection = document.getSelection(); - domSelection.removeAllRanges(); - domSelection.addRange( domRange ); - - const viewSelection = converter.domSelectionToView( domSelection ); - - expect( viewSelection.rangeCount ).to.equal( 1 ); - expect( stringify( viewP, viewSelection.getFirstRange() ) ).to.equal( '

f{o}o

' ); - - // Now we know that there should be a valid view range. So let's test if the DOM node throws an error. - sinon.stub( domFoo, Symbol.toStringTag ).get( () => { - throw new Error( 'Permission denied to access property Symbol.toStringTag' ); - } ); + it( 'should detect "restricted objects" in Firefox DOM ranges and prevent an error being thrown', () => { + testUtils.sinon.stub( env, 'isGecko' ).value( true ); - let result = null; + changeDomSelection(); + domDocument.dispatchEvent( new Event( 'selectionchange' ) ); - expect( () => { - result = converter.domSelectionToView( domSelection ); - } ).to.not.throw(); + expect( view.hasDomSelection ).to.be.true; - expect( result instanceof ViewSelection ).to.be.true; - expect( result.rangeCount ).to.equal( 0 ); + const domFoo = domDocument.getSelection().anchorNode; - domP.remove(); + sinon.stub( domFoo, Symbol.toStringTag ).get( () => { + throw new Error( 'Permission denied to access property Symbol.toStringTag' ); } ); - it( 'should do nothing in Firefox if the DOM selection is correct', done => { - const domFoo = document.createTextNode( 'foo' ); - const domP = createElement( document, 'p', null, [ domFoo ] ); - const converter = view.domConverter; - - const viewP = parse( '

foo

' ); - - converter.bindElements( domP, viewP ); - - document.body.appendChild( domP ); - - const domRange = document.createRange(); - domRange.setStart( domFoo, 1 ); - domRange.setEnd( domFoo, 2 ); - - const domSelection = document.getSelection(); - domSelection.removeAllRanges(); - domSelection.addRange( domRange ); - - expect( () => { - converter.domSelectionToView( domSelection ); - - setTimeout( done, 100 ); - } ).to.not.throw(); + domDocument.dispatchEvent( new Event( 'selectionchange' ) ); - domP.remove(); - } ); + expect( view.hasDomSelection ).to.be.false; } ); it( 'should add only one #selectionChange listener to one document', done => { @@ -547,7 +488,6 @@ describe( 'SelectionObserver', () => { selectionObserver.listenTo( domMain, 'selectstart', selectStartChangedSpy, { priority: 'highest' } ); // The event was fired somewhere else in DOM. - domDocument.dispatchEvent( new Event( 'selectstart' ) ); expect( viewDocument.isSelecting ).to.be.false; sinon.assert.notCalled( selectStartChangedSpy ); From a95c15702e40f18cf45aad30a5bfd9a7f07fe531 Mon Sep 17 00:00:00 2001 From: Illia Sheremetov Date: Wed, 30 Aug 2023 12:58:07 +0200 Subject: [PATCH 16/16] Test (engine): fix test --- .../ckeditor5-engine/tests/view/observer/selectionobserver.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js index 7ebe11a9de7..7da556de42b 100644 --- a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js +++ b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js @@ -488,6 +488,7 @@ describe( 'SelectionObserver', () => { selectionObserver.listenTo( domMain, 'selectstart', selectStartChangedSpy, { priority: 'highest' } ); // The event was fired somewhere else in DOM. + domDocument.dispatchEvent( new Event( 'selectstart' ) ); expect( viewDocument.isSelecting ).to.be.false; sinon.assert.notCalled( selectStartChangedSpy );