Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Wrapped the view.change() into the try/catch block.
Browse files Browse the repository at this point in the history
  • Loading branch information
ma2ciek committed Oct 3, 2019
1 parent 87d7de0 commit 9a563c5
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 19 deletions.
59 changes: 40 additions & 19 deletions src/view/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -457,29 +457,50 @@ export default class View {
);
}

// Recursive call to view.change() method - execute listener immediately.
if ( this._ongoingChange ) {
return callback( this._writer );
}
// Use try-catch to catch the
try {
// Recursive call to view.change() method - execute listener immediately.
if ( this._ongoingChange ) {
return callback( this._writer );
}

// This lock will assure that all recursive calls to view.change() will end up in same block - one "render"
// event for all nested calls.
this._ongoingChange = true;
const callbackResult = callback( this._writer );
this._ongoingChange = false;
// This lock will assure that all recursive calls to view.change() will end up in same block - one "render"
// event for all nested calls.
this._ongoingChange = true;
const callbackResult = callback( this._writer );
this._ongoingChange = false;

// This lock is used by editing controller to render changes from outer most model.change() once. As plugins might call
// view.change() inside model.change() block - this will ensures that postfixers and rendering are called once after all
// changes. Also, we don't need to render anything if there're no changes since last rendering.
if ( !this._renderingDisabled && this._hasChangedSinceTheLastRendering ) {
this._postFixersInProgress = true;
this.document._callPostFixers( this._writer );
this._postFixersInProgress = false;

this.fire( 'render' );
}

// This lock is used by editing controller to render changes from outer most model.change() once. As plugins might call
// view.change() inside model.change() block - this will ensures that postfixers and rendering are called once after all changes.
// Also, we don't need to render anything if there're no changes since last rendering.
if ( !this._renderingDisabled && this._hasChangedSinceTheLastRendering ) {
this._postFixersInProgress = true;
this.document._callPostFixers( this._writer );
this._postFixersInProgress = false;
return callbackResult;
} catch ( err ) {
if ( err.is && err.is( 'CKEditorError' ) ) {
throw err;
}

this.fire( 'render' );
/**
* An unexpected error occurred inside the `view.change()` block. The `error.data.originalError` property
* shows the original error properties.
*
* @error view-change-unexpected-error
*/
throw new CKEditorError( 'view-change-unexpected-error', this, {
originalError: {
message: err.message,
stack: err.stack,
name: err.name
}
} );
}

return callbackResult;
}

/**
Expand Down
29 changes: 29 additions & 0 deletions tests/view/view/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import createViewRoot from '../_utils/createroot';
import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement';
import { expectToThrowCKEditorError } from '@ckeditor/ckeditor5-utils/tests/_utils/utils';
import env from '@ckeditor/ckeditor5-utils/src/env';
import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';

describe( 'view', () => {
const DEFAULT_OBSERVERS_COUNT = 6;
Expand Down Expand Up @@ -802,6 +803,34 @@ describe( 'view', () => {
expect( result2 ).to.equal( true );
expect( result3 ).to.undefined;
} );

it( 'should catch native errors and wrap them into the CKEditorError errors', () => {
const error = new TypeError( 'foo' );
error.stack = 'bar';

expectToThrowCKEditorError( () => {
view.change( () => {
throw error;
} );
}, /view-change-unexpected-error/, view, {
originalError: {
message: 'foo',
stack: 'bar',
name: 'TypeError'
}
} );
} );

it( 'should re-throw custom CKEditorError errors', () => {
const error = new TypeError( 'foo' );
error.stack = 'bar';

expectToThrowCKEditorError( () => {
view.change( () => {
throw new CKEditorError( 'foo', view );
} );
}, /foo/, view );
} );
} );

describe( 'createPositionAt()', () => {
Expand Down

0 comments on commit 9a563c5

Please sign in to comment.