Skip to content

Commit

Permalink
Merge pull request #14706 from ckeditor/ck/9635-error-when-clicking-o…
Browse files Browse the repository at this point in the history
…n-label-elements-inside-editor

Fix (engine): Don't throw an error when clicking on a balloon with input on Firefox. Closes #9635.
  • Loading branch information
niegowski committed Aug 30, 2023
2 parents 52231a2 + a95c157 commit 54a4c22
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 2 deletions.
33 changes: 32 additions & 1 deletion packages/ckeditor5-engine/src/view/domconverter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ import {
isText,
isComment,
isValidAttributeName,
first
first,
env
} from '@ckeditor/ckeditor5-utils';

import type ViewNode from './node';
Expand Down Expand Up @@ -732,6 +733,11 @@ export default class DomConverter {
* @returns View selection.
*/
public domSelectionToView( domSelection: DomSelection ): ViewSelection {
// See: https://github.com/ckeditor/ckeditor5/issues/9635.
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 ) {
Expand Down Expand Up @@ -1787,6 +1793,31 @@ 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.
* See: https://github.com/ckeditor/ckeditor5/issues/9635.
*/
function isGeckoRestrictedDomSelection( domSelection: DomSelection ): boolean {
if ( !env.isGecko ) {
return false;
}

if ( !domSelection.rangeCount ) {
return false;
}

const container = domSelection.getRangeAt( 0 ).startContainer;

try {
Object.prototype.toString.call( container );
} catch ( error ) {
return true;
}

return false;
}

/**
* Enum representing the type of the block filler.
*
Expand Down
86 changes: 86 additions & 0 deletions packages/ckeditor5-engine/tests/view/domconverter/dom-to-view.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -18,6 +19,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;
Expand Down Expand Up @@ -1287,5 +1289,89 @@ describe( 'DomConverter', () => {

domContainer.remove();
} );

// 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 );

const domFoo = document.createTextNode( 'foo' );
const domP = createElement( document, 'p', null, [ domFoo ] );

const viewP = parse( '<p>foo</p>' );

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( '<p>f{o}o</p>' );

// 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 not check if restricted object on non-Gecko browsers', () => {
testUtils.sinon.stub( env, 'isGecko' ).value( false );

const domFoo = document.createTextNode( 'foo' );
const domP = createElement( document, 'p', null, [ domFoo ] );

const viewP = parse( '<p>foo</p>' );

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( '<p>f{o}o</p>' );

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 );
} );
} );
} );
} );
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ describe( 'SelectionObserver', () => {
domRoot.innerHTML = '<div contenteditable="true"></div><div contenteditable="true" id="additional"></div>';
domMain = domRoot.childNodes[ 0 ];
domDocument.body.appendChild( domRoot );

view = new View( new StylesProcessor() );
viewDocument = view.document;
createViewRoot( viewDocument );
Expand Down Expand Up @@ -162,6 +161,25 @@ describe( 'SelectionObserver', () => {
changeDomSelection();
} );

it( 'should detect "restricted objects" in Firefox DOM ranges and prevent an error being thrown', () => {
testUtils.sinon.stub( env, 'isGecko' ).value( true );

changeDomSelection();
domDocument.dispatchEvent( new Event( 'selectionchange' ) );

expect( view.hasDomSelection ).to.be.true;

const domFoo = domDocument.getSelection().anchorNode;

sinon.stub( domFoo, Symbol.toStringTag ).get( () => {
throw new Error( 'Permission denied to access property Symbol.toStringTag' );
} );

domDocument.dispatchEvent( new Event( 'selectionchange' ) );

expect( view.hasDomSelection ).to.be.false;
} );

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' );
Expand Down

0 comments on commit 54a4c22

Please sign in to comment.