Explorar el Código

Fix: Made the FocusTracker#isFocused synchronous thanks to FocusEvent#relatedTarget. Closes #245.

Aleksander Nowodzinski hace 7 años
padre
commit
19190a3c42

+ 23 - 29
packages/ckeditor5-utils/src/focustracker.js

@@ -3,8 +3,6 @@
  * For licensing, see LICENSE.md.
  */
 
-/* global setTimeout, clearTimeout */
-
 /**
  * @module utils/focustracker
  */
@@ -53,14 +51,6 @@ export default class FocusTracker {
 		 * @member {Set.<HTMLElement>}
 		 */
 		this._elements = new Set();
-
-		/**
-		 * Event loop timeout.
-		 *
-		 * @private
-		 * @member {Number}
-		 */
-		this._nextEventLoopTimeout = null;
 	}
 
 	/**
@@ -74,7 +64,7 @@ export default class FocusTracker {
 		}
 
 		this.listenTo( element, 'focus', () => this._focus( element ), { useCapture: true } );
-		this.listenTo( element, 'blur', () => this._blur(), { useCapture: true } );
+		this.listenTo( element, 'blur', ( evt, domEvt ) => this._blur( domEvt ), { useCapture: true } );
 		this._elements.add( element );
 	}
 
@@ -101,35 +91,39 @@ export default class FocusTracker {
 	 * @param {HTMLElement} element Element which has been focused.
 	 */
 	_focus( element ) {
-		clearTimeout( this._nextEventLoopTimeout );
-
 		this.focusedElement = element;
 		this.isFocused = true;
 	}
 
 	/**
 	 * Clears currently focused element and set {@link #isFocused} as `false`.
-	 * This method uses `setTimeout` to change order of fires `blur` and `focus` events.
 	 *
 	 * @private
-	 * @fires blur
+	 * @param {FocusEvent} domEvt The native DOM FocusEvent instance.
 	 */
-	_blur() {
-		clearTimeout( this._nextEventLoopTimeout );
+	_blur( domEvt ) {
+		const relatedTarget = domEvt.relatedTarget;
+		const isInnerBlur = Array.from( this._elements ).some( element => {
+			// element.contains( element ) -> true
+			return element.contains( relatedTarget );
+		} );
+
+		// If the blur was caused by focusing an element which is either:
+		//
+		// * registered in this focus tracker,
+		// * a child of an element registered in this focus tracker
+		//
+		// then don't fire the event to prevent rapid #isFocused changes. The focus remains within
+		// the registered elements; announcing this change is pointless.
+		//
+		// Note: In DOM, the native blur always precedes the following focus.
+		if ( isInnerBlur ) {
+			return;
+		}
 
-		this._nextEventLoopTimeout = setTimeout( () => {
-			this.focusedElement = null;
-			this.isFocused = false;
-		}, 0 );
+		this.focusedElement = null;
+		this.isFocused = false;
 	}
-
-	/**
-	 * @event focus
-	 */
-
-	/**
-	 * @event blur
-	 */
 }
 
 mix( FocusTracker, DomEmitterMixin );

+ 17 - 20
packages/ckeditor5-utils/tests/focustracker.js

@@ -3,7 +3,7 @@
  * For licensing, see LICENSE.md.
  */
 
-/* global document, Event */
+/* global document, FocusEvent */
 
 import FocusTracker from '../src/focustracker';
 import CKEditorError from '../src/ckeditorerror';
@@ -60,7 +60,7 @@ describe( 'FocusTracker', () => {
 
 				expect( focusTracker.isFocused ).to.false;
 
-				containerFirstInput.dispatchEvent( new Event( 'focus' ) );
+				containerFirstInput.dispatchEvent( new FocusEvent( 'focus' ) );
 
 				expect( focusTracker.isFocused ).to.true;
 			} );
@@ -69,8 +69,7 @@ describe( 'FocusTracker', () => {
 				focusTracker.add( containerFirstInput );
 				focusTracker.isFocused = true;
 
-				containerFirstInput.dispatchEvent( new Event( 'blur' ) );
-				testUtils.sinon.clock.tick( 0 );
+				containerFirstInput.dispatchEvent( new FocusEvent( 'blur' ) );
 
 				expect( focusTracker.isFocused ).to.false;
 			} );
@@ -82,7 +81,7 @@ describe( 'FocusTracker', () => {
 
 				expect( focusTracker.isFocused ).to.false;
 
-				containerFirstInput.dispatchEvent( new Event( 'focus' ) );
+				containerFirstInput.dispatchEvent( new FocusEvent( 'focus' ) );
 
 				expect( focusTracker.isFocused ).to.true;
 			} );
@@ -91,8 +90,7 @@ describe( 'FocusTracker', () => {
 				focusTracker.add( container );
 				focusTracker.isFocused = true;
 
-				containerFirstInput.dispatchEvent( new Event( 'blur' ) );
-				testUtils.sinon.clock.tick( 0 );
+				containerFirstInput.dispatchEvent( new FocusEvent( 'blur' ) );
 
 				expect( focusTracker.isFocused ).to.false;
 			} );
@@ -102,15 +100,17 @@ describe( 'FocusTracker', () => {
 
 				focusTracker.add( container );
 
-				containerFirstInput.dispatchEvent( new Event( 'focus' ) );
+				containerFirstInput.dispatchEvent( new FocusEvent( 'focus' ) );
 
 				focusTracker.listenTo( focusTracker, 'change:isFocused', changeSpy );
 
 				expect( focusTracker.isFocused ).to.true;
 
-				containerFirstInput.dispatchEvent( new Event( 'blur' ) );
-				containerSecondInput.dispatchEvent( new Event( 'focus' ) );
-				testUtils.sinon.clock.tick( 0 );
+				containerFirstInput.dispatchEvent( new FocusEvent( 'blur', {
+					relatedTarget: containerSecondInput
+				} ) );
+
+				containerSecondInput.dispatchEvent( new FocusEvent( 'focus' ) );
 
 				expect( focusTracker.isFocused ).to.true;
 				expect( changeSpy.notCalled ).to.true;
@@ -121,10 +121,9 @@ describe( 'FocusTracker', () => {
 				focusTracker.add( container );
 				focusTracker.isFocused = true;
 
-				container.dispatchEvent( new Event( 'blur' ) );
-				containerFirstInput.dispatchEvent( new Event( 'blur' ) );
-				containerSecondInput.dispatchEvent( new Event( 'focus' ) );
-				testUtils.sinon.clock.tick( 0 );
+				container.dispatchEvent( new FocusEvent( 'blur' ) );
+				containerFirstInput.dispatchEvent( new FocusEvent( 'blur' ) );
+				containerSecondInput.dispatchEvent( new FocusEvent( 'focus' ) );
 
 				expect( focusTracker.isFocused ).to.be.true;
 			} );
@@ -142,7 +141,7 @@ describe( 'FocusTracker', () => {
 			focusTracker.add( containerFirstInput );
 			focusTracker.remove( containerFirstInput );
 
-			containerFirstInput.dispatchEvent( new Event( 'focus' ) );
+			containerFirstInput.dispatchEvent( new FocusEvent( 'focus' ) );
 
 			expect( focusTracker.isFocused ).to.false;
 		} );
@@ -152,20 +151,18 @@ describe( 'FocusTracker', () => {
 			focusTracker.remove( containerFirstInput );
 			focusTracker.isFocused = true;
 
-			containerFirstInput.dispatchEvent( new Event( 'blur' ) );
-			testUtils.sinon.clock.tick( 0 );
+			containerFirstInput.dispatchEvent( new FocusEvent( 'blur' ) );
 
 			expect( focusTracker.isFocused ).to.true;
 		} );
 
 		it( 'should blur element before removing when is focused', () => {
 			focusTracker.add( containerFirstInput );
-			containerFirstInput.dispatchEvent( new Event( 'focus' ) );
+			containerFirstInput.dispatchEvent( new FocusEvent( 'focus' ) );
 
 			expect( focusTracker.isFocused ).to.true;
 
 			focusTracker.remove( containerFirstInput );
-			testUtils.sinon.clock.tick( 0 );
 
 			expect( focusTracker.isFocused ).to.false;
 		} );