Skip to content

Commit

Permalink
Merge pull request #10830 from ckeditor/ck/7830-dom-emitter
Browse files Browse the repository at this point in the history
Fix (utils): Fixes `DomEmitterMixin` to correctly manage listeners for different options set (`useCapture` & `usePassive`) for the same DOM node. Closes #7830.
  • Loading branch information
niegowski authored Nov 12, 2021
2 parents d83c003 + f0767eb commit fe11106
Show file tree
Hide file tree
Showing 2 changed files with 267 additions and 93 deletions.
168 changes: 117 additions & 51 deletions packages/ckeditor5-utils/src/dom/emittermixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,19 +52,21 @@ const DomEmitterMixin = extend( {}, EmitterMixin, {
* @param {Boolean} [options.usePassive=false] Indicates that the function specified by listener will never call preventDefault()
* and prevents blocking browser's main thread by this event handler.
*/
listenTo( emitter, ...rest ) {
// Check if emitter is an instance of DOM Node. If so, replace the argument with
// corresponding ProxyEmitter (or create one if not existing).
listenTo( emitter, event, callback, options = {} ) {
// Check if emitter is an instance of DOM Node. If so, use corresponding ProxyEmitter (or create one if not existing).
if ( isNode( emitter ) || isWindow( emitter ) ) {
const proxy = this._getProxyEmitter( emitter ) || new ProxyEmitter( emitter );
const proxyOptions = {
capture: !!options.useCapture,
passive: !!options.usePassive
};

proxy.attach( ...rest );
const proxyEmitter = this._getProxyEmitter( emitter, proxyOptions ) || new ProxyEmitter( emitter, proxyOptions );

emitter = proxy;
this.listenTo( proxyEmitter, event, callback, options );
} else {
// Execute parent class method with Emitter (or ProxyEmitter) instance.
EmitterMixin.listenTo.call( this, emitter, event, callback, options );
}

// Execute parent class method with Emitter (or ProxyEmitter) instance.
EmitterMixin.listenTo.call( this, emitter, ...rest );
},

/**
Expand All @@ -83,35 +85,49 @@ const DomEmitterMixin = extend( {}, EmitterMixin, {
* `event`.
*/
stopListening( emitter, event, callback ) {
// Check if emitter is an instance of DOM Node. If so, replace the argument with corresponding ProxyEmitter.
// Check if the emitter is an instance of DOM Node. If so, forward the call to the corresponding ProxyEmitters.
if ( isNode( emitter ) || isWindow( emitter ) ) {
const proxy = this._getProxyEmitter( emitter );
const proxyEmitters = this._getAllProxyEmitters( emitter );

// Element has no listeners.
if ( !proxy ) {
return;
for ( const proxy of proxyEmitters ) {
this.stopListening( proxy, event, callback );
}

emitter = proxy;
} else {
// Execute parent class method with Emitter (or ProxyEmitter) instance.
EmitterMixin.stopListening.call( this, emitter, event, callback );
}
},

// Execute parent class method with Emitter (or ProxyEmitter) instance.
EmitterMixin.stopListening.call( this, emitter, event, callback );

if ( emitter instanceof ProxyEmitter ) {
emitter.detach( event );
}
/**
* Retrieves ProxyEmitter instance for given DOM Node residing in this Host and given options.
*
* @private
* @param {Node} node DOM Node of the ProxyEmitter.
* @param {Object} [options] Additional options.
* @param {Boolean} [options.useCapture=false] Indicates that events of this type will be dispatched to the registered
* listener before being dispatched to any EventTarget beneath it in the DOM tree.
* @param {Boolean} [options.usePassive=false] Indicates that the function specified by listener will never call preventDefault()
* and prevents blocking browser's main thread by this event handler.
* @returns {module:utils/dom/emittermixin~ProxyEmitter|null} ProxyEmitter instance bound to the DOM Node.
*/
_getProxyEmitter( node, options ) {
return _getEmitterListenedTo( this, getProxyEmitterId( node, options ) );
},

/**
* Retrieves ProxyEmitter instance for given DOM Node residing in this Host.
* Retrieves all the ProxyEmitter instances for given DOM Node residing in this Host.
*
* @private
* @param {Node} node DOM Node of the ProxyEmitter.
* @returns {module:utils/dom/emittermixin~ProxyEmitter} ProxyEmitter instance or null.
* @returns {Array.<module:utils/dom/emittermixin~ProxyEmitter>}
*/
_getProxyEmitter( node ) {
return _getEmitterListenedTo( this, getNodeUID( node ) );
_getAllProxyEmitters( node ) {
return [
{ capture: false, passive: false },
{ capture: false, passive: true },
{ capture: true, passive: false },
{ capture: true, passive: true }
].map( options => this._getProxyEmitter( node, options ) ).filter( proxy => !!proxy );
}
} );

Expand All @@ -120,6 +136,8 @@ export default DomEmitterMixin;
/**
* Creates a ProxyEmitter instance. Such an instance is a bridge between a DOM Node firing events
* and any Host listening to them. It is backwards compatible with {@link module:utils/emittermixin~EmitterMixin#on}.
* There is a separate instance for each combination of modes (useCapture & usePassive). The mode is concatenated with
* UID stored in HTMLElement to give each instance unique identifier.
*
* listenTo( click, ... )
* +-----------------------------------------+
Expand All @@ -128,14 +146,14 @@ export default DomEmitterMixin;
* | Host | | +---------------------------------------------+
* +----------------------------+ | | removeEventListener( click, ... ) |
* | _listeningTo: { | +----------v-------------+ |
* | UID: { | | ProxyEmitter | |
* | UID+mode: { | | ProxyEmitter | |
* | emitter: ProxyEmitter, | +------------------------+ +------------v----------+
* | callbacks: { | | events: { | | Node (HTMLElement) |
* | click: [ callbacks ] | | click: [ callbacks ] | +-----------------------+
* | } | | }, | | data-ck-expando: UID |
* | } | | _domNode: Node, | +-----------------------+
* | } | | _domListeners: {}, | |
* | +------------------------+ | | _emitterId: UID | |
* | +------------------------+ | | _emitterId: UID+mode | |
* | | DomEmitterMixin | | +--------------^---------+ |
* | +------------------------+ | | | |
* +--------------^-------------+ | +---------------------------------------------+
Expand All @@ -150,14 +168,21 @@ export default DomEmitterMixin;
class ProxyEmitter {
/**
* @param {Node} node DOM Node that fires events.
* @returns {Object} ProxyEmitter instance bound to the DOM Node.
* @param {Object} [options] Additional options.
* @param {Boolean} [options.useCapture=false] Indicates that events of this type will be dispatched to the registered
* listener before being dispatched to any EventTarget beneath it in the DOM tree.
* @param {Boolean} [options.usePassive=false] Indicates that the function specified by listener will never call preventDefault()
* and prevents blocking browser's main thread by this event handler.
*/
constructor( node ) {
constructor( node, options ) {
// Set emitter ID to match DOM Node "expando" property.
_setEmitterId( this, getNodeUID( node ) );
_setEmitterId( this, getProxyEmitterId( node, options ) );

// Remember the DOM Node this ProxyEmitter is bound to.
this._domNode = node;

// And given options.
this._options = options;
}
}

Expand All @@ -175,31 +200,23 @@ extend( ProxyEmitter.prototype, EmitterMixin, {
* It attaches a native DOM listener to the DOM Node. When fired,
* a corresponding Emitter event will also fire with DOM Event object as an argument.
*
* **Note**: This is automatically called by the
* {@link module:utils/emittermixin~EmitterMixin#listenTo `EmitterMixin#listenTo()`}.
*
* @method module:utils/dom/emittermixin~ProxyEmitter#attach
* @param {String} event The name of the event.
* @param {Function} callback The function to be called on event.
* @param {Object} [options={}] Additional options.
* @param {Boolean} [options.useCapture=false] Indicates that events of this type will be dispatched to the registered
* listener before being dispatched to any EventTarget beneath it in the DOM tree.
* @param {Boolean} [options.usePassive=false] Indicates that the function specified by listener will never call preventDefault()
* and prevents blocking browser's main thread by this event handler.
*/
attach( event, callback, options = {} ) {
attach( event ) {
// If the DOM Listener for given event already exist it is pointless
// to attach another one.
if ( this._domListeners && this._domListeners[ event ] ) {
return;
}

const listenerOptions = {
capture: !!options.useCapture,
passive: !!options.usePassive
};

const domListener = this._createDomListener( event, listenerOptions );
const domListener = this._createDomListener( event );

// Attach the native DOM listener to DOM Node.
this._domNode.addEventListener( event, domListener, listenerOptions );
this._domNode.addEventListener( event, domListener, this._options );

if ( !this._domListeners ) {
this._domListeners = {};
Expand All @@ -213,6 +230,9 @@ extend( ProxyEmitter.prototype, EmitterMixin, {
/**
* Stops executing the callback on the given event.
*
* **Note**: This is automatically called by the
* {@link module:utils/emittermixin~EmitterMixin#stopListening `EmitterMixin#stopListening()`}.
*
* @method module:utils/dom/emittermixin~ProxyEmitter#detach
* @param {String} event The name of the event.
*/
Expand All @@ -228,6 +248,36 @@ extend( ProxyEmitter.prototype, EmitterMixin, {
}
},

/**
* Adds callback to emitter for given event.
*
* @protected
* @method module:utils/dom/emittermixin~ProxyEmitter#_addEventListener
* @param {String} event The name of the event.
* @param {Function} callback The function to be called on event.
* @param {Object} [options={}] Additional options.
* @param {module:utils/priorities~PriorityString|Number} [options.priority='normal'] The priority of this event callback. The higher
* the priority value the sooner the callback will be fired. Events having the same priority are called in the
* order they were added.
*/
_addEventListener( event, callback, options ) {
this.attach( event );
EmitterMixin._addEventListener.call( this, event, callback, options );
},

/**
* Removes callback from emitter for given event.
*
* @protected
* @method module:utils/dom/emittermixin~ProxyEmitter#_removeEventListener
* @param {String} event The name of the event.
* @param {Function} callback The function to stop being called.
*/
_removeEventListener( event, callback ) {
EmitterMixin._removeEventListener.call( this, event, callback );
this.detach( event );
},

/**
* Creates a native DOM listener callback. When the native DOM event
* is fired it will fire corresponding event on this ProxyEmitter.
Expand All @@ -236,13 +286,9 @@ extend( ProxyEmitter.prototype, EmitterMixin, {
* @private
* @method module:utils/dom/emittermixin~ProxyEmitter#_createDomListener
* @param {String} event The name of the event.
* @param {Object} [options] Additional options.
* @param {Boolean} [options.capture=false] Indicates whether the listener was created for capturing event.
* @param {Boolean} [options.passive=false] Indicates that the function specified by listener will never call preventDefault()
* and prevents blocking browser's main thread by this event handler.
* @returns {Function} The DOM listener callback.
*/
_createDomListener( event, options ) {
_createDomListener( event ) {
const domListener = domEvt => {
this.fire( event, domEvt );
};
Expand All @@ -251,7 +297,7 @@ extend( ProxyEmitter.prototype, EmitterMixin, {
// detach it from the DOM Node, when it is no longer necessary.
// See: {@link detach}.
domListener.removeListener = () => {
this._domNode.removeEventListener( event, domListener, options );
this._domNode.removeEventListener( event, domListener, this._options );
delete this._domListeners[ event ];
};

Expand All @@ -268,6 +314,26 @@ function getNodeUID( node ) {
return node[ 'data-ck-expando' ] || ( node[ 'data-ck-expando' ] = uid() );
}

// Gets id of the ProxyEmitter for the given node.
//
// Combines DOM Node identifier and additional options.
//
// @private
// @param {Node} node
// @param {Object} options Additional options.
// @returns {String} ProxyEmitter id.
function getProxyEmitterId( node, options ) {
let id = getNodeUID( node );

for ( const option of Object.keys( options ).sort() ) {
if ( options[ option ] ) {
id += '-' + option;
}
}

return id;
}

/**
* Interface representing classes which mix in {@link module:utils/dom/emittermixin~EmitterMixin}.
*
Expand Down
Loading

0 comments on commit fe11106

Please sign in to comment.