Explorar o código

Merge branch 't/795' into t/857

Szymon Kupś %!s(int64=8) %!d(string=hai) anos
pai
achega
f446c6726d

+ 2 - 2
packages/ckeditor5-engine/src/view/observer/focusobserver.js

@@ -33,8 +33,8 @@ export default class FocusObserver extends DomEventObserver {
 			document.isFocused = true;
 
 			// Unfortunately native `selectionchange` event is fired asynchronously.
-			// Wait until `SelectionObserver` handle the event and then render. Otherwise rendering will overwrite new DOM
-			// selection with selection from the view.
+			// We need to wait until `SelectionObserver` handle the event and then render. Otherwise rendering will
+			// overwrite new DOM selection with selection from the view.
 			// See https://github.com/ckeditor/ckeditor5-engine/issues/795 for more details.
 			this.selectionObserver.once( 'selectionChangeHandling', () => {
 				document.render();

+ 3 - 3
packages/ckeditor5-engine/src/view/observer/selectionobserver.js

@@ -276,10 +276,10 @@ export default class SelectionObserver extends Observer {
  */
 
 /**
- * Fired when selection change is being handled.
+ * Fired when selection change is being handled. It can be used to execute code before or after actual `selectionchange`
+ * event handling.
  *
  * @event module:engine/view/observer/selectionobserver~SelectionObserver#event:selectionChangeHandling
  * @param {Object} data
- * @param {Document} domDocument DOM document on which selectionchange event was fired.
+ * @param {Document} domDocument DOM document on which `selectionchange` event was fired.
  */
-

+ 29 - 9
packages/ckeditor5-engine/tests/view/observer/focusobserver.js

@@ -3,11 +3,11 @@
  * For licensing, see LICENSE.md.
  */
 
-/* globals document */
-
 import FocusObserver from '../../../src/view/observer/focusobserver';
+import SelectionObserver from '../../../src/view/observer/selectionobserver';
 import ViewDocument from '../../../src/view/document';
 import ViewRange from '../../../src/view/range';
+import global from '@ckeditor/ckeditor5-utils/src/dom/global';
 
 describe( 'FocusObserver', () => {
 	let viewDocument, observer;
@@ -35,12 +35,12 @@ describe( 'FocusObserver', () => {
 
 			viewDocument.on( 'focus', spy );
 
-			observer.onDomEvent( { type: 'focus', target: document.body } );
+			observer.onDomEvent( { type: 'focus', target: global.document.body } );
 
 			expect( spy.calledOnce ).to.be.true;
 
 			const data = spy.args[ 0 ][ 1 ];
-			expect( data.domTarget ).to.equal( document.body );
+			expect( data.domTarget ).to.equal( global.document.body );
 		} );
 
 		it( 'should fire blur with the right event data', () => {
@@ -48,18 +48,18 @@ describe( 'FocusObserver', () => {
 
 			viewDocument.on( 'blur', spy );
 
-			observer.onDomEvent( { type: 'blur', target: document.body } );
+			observer.onDomEvent( { type: 'blur', target: global.document.body } );
 
 			expect( spy.calledOnce ).to.be.true;
 
 			const data = spy.args[ 0 ][ 1 ];
-			expect( data.domTarget ).to.equal( document.body );
+			expect( data.domTarget ).to.equal( global.document.body );
 		} );
 
 		it( 'should render document after blurring', () => {
 			const renderSpy = sinon.spy( viewDocument, 'render' );
 
-			observer.onDomEvent( { type: 'blur', target: document.body } );
+			observer.onDomEvent( { type: 'blur', target: global.document.body } );
 
 			sinon.assert.calledOnce( renderSpy );
 		} );
@@ -69,8 +69,8 @@ describe( 'FocusObserver', () => {
 		let domMain, domHeader, viewMain, viewHeader;
 
 		beforeEach( () => {
-			domMain = document.createElement( 'div' );
-			domHeader = document.createElement( 'h1' );
+			domMain = global.document.createElement( 'div' );
+			domHeader = global.document.createElement( 'h1' );
 
 			viewMain = viewDocument.createRoot( domMain );
 			viewHeader = viewDocument.createRoot( domHeader, 'header' );
@@ -115,5 +115,25 @@ describe( 'FocusObserver', () => {
 
 			expect( viewDocument.isFocused ).to.be.true;
 		} );
+
+		it( 'should call render after selectionChange event is handled', () => {
+			const renderSpy = sinon.spy( viewDocument, 'render' );
+			const selectionObserver = viewDocument.getObserver( SelectionObserver );
+
+			observer.onDomEvent( { type: 'focus', target: domMain } );
+			sinon.assert.notCalled( renderSpy );
+			selectionObserver.fire( 'selectionChangeHandling', { domDocument: global.document } );
+			sinon.assert.called( renderSpy );
+		} );
+
+		it( 'should call render only after first selectionChange after focus', () => {
+			const renderSpy = sinon.spy( viewDocument, 'render' );
+			const selectionObserver = viewDocument.getObserver( SelectionObserver );
+
+			observer.onDomEvent( { type: 'focus', target: domMain } );
+			selectionObserver.fire( 'selectionChangeHandling', { domDocument: global.document } );
+			selectionObserver.fire( 'selectionChangeHandling', { domDocument: global.document } );
+			sinon.assert.calledOnce( renderSpy );
+		} );
 	} );
 } );

+ 19 - 0
packages/ckeditor5-engine/tests/view/observer/selectionobserver.js

@@ -11,6 +11,7 @@ import ViewSelection from '../../../src/view/selection';
 import ViewDocument from '../../../src/view/document';
 import SelectionObserver from '../../../src/view/observer/selectionobserver';
 import MutationObserver from '../../../src/view/observer/mutationobserver';
+import FocusObserver from '../../../src/view/observer/focusobserver';
 
 import log from '@ckeditor/ckeditor5-utils/src/log';
 
@@ -232,6 +233,9 @@ describe( 'SelectionObserver', () => {
 
 		viewDocument.on( 'selectionChangeDone', spy );
 
+		// Disable focus observer to not re-render view on each focus.
+		viewDocument.getObserver( FocusObserver ).disable();
+
 		// Change selection.
 		changeDomSelection();
 
@@ -295,6 +299,21 @@ describe( 'SelectionObserver', () => {
 			}, 110 );
 		}, 100 );
 	} );
+
+	it( 'should fire selectionChangeHandling on standard priority', ( done ) => {
+		let spy1 = sinon.spy();
+		let spy2 = sinon.spy();
+
+		selectionObserver.on( 'selectionChangeHandling', spy1, { priority: 'high' } );
+		viewDocument.on( 'selectionChange', spy2 );
+
+		selectionObserver.on( 'selectionChangeHandling', () => {
+			sinon.assert.callOrder( spy1, spy2 );
+			done();
+		}, { priority: 'low' } );
+
+		changeDomSelection();
+	} );
 } );
 
 function changeDomSelection() {