Merge pull request #208 from ckeditor/t/201

Fix: Bulletproofed `isDomNode()` helper when used in iframes. Removed `isWindow()` logic from the helper. Closes #201.
ckeditor · Dec 11, 2017 · 84ccda2 · 84ccda2
2 parents 28b34d5 + 6f2ad28
commit 84ccda2
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 9 deletions.
diff --git a/src/dom/emittermixin.js b/src/dom/emittermixin.js
@@ -11,6 +11,7 @@ import { default as EmitterMixin, _getEmitterListenedTo, _setEmitterId } from '.
 import uid from '../uid';
 import extend from '../lib/lodash/extend';
 import isDomNode from './isdomnode';
+import isWindow from './iswindow';
 
 /**
  * Mixin that injects the DOM events API into its host. It provides the API
@@ -56,7 +57,7 @@ const DomEmitterMixin = extend( {}, EmitterMixin, {
 
 		// Check if emitter is an instance of DOM Node. If so, replace the argument with
 		// corresponding ProxyEmitter (or create one if not existing).
-		if ( isDomNode( emitter ) ) {
+		if ( isDomNode( emitter ) || isWindow( emitter ) ) {
 			args[ 0 ] = this._getProxyEmitter( emitter ) || new ProxyEmitter( emitter );
 		}
 
@@ -85,7 +86,7 @@ const DomEmitterMixin = extend( {}, EmitterMixin, {
 		const emitter = args[ 0 ];
 
 		// Check if emitter is an instance of DOM Node. If so, replace the argument with corresponding ProxyEmitter.
-		if ( isDomNode( emitter ) ) {
+		if ( isDomNode( emitter ) || isWindow( emitter ) ) {
 			const proxy = this._getProxyEmitter( emitter );
 
 			// Element has no listeners.

diff --git a/src/dom/isdomnode.js b/src/dom/isdomnode.js
@@ -7,14 +7,20 @@
  * @module utils/dom/isdomnode
  */
 
-import isNative from '../lib/lodash/isNative';
-
 /**
  * Checks if the object is a native DOM Node.
  *
  * @param {*} obj
  * @returns {Boolean}
  */
 export default function isDomNode( obj ) {
-	return !!( obj && isNative( obj.addEventListener ) );
+	if ( obj ) {
+		if ( obj.defaultView ) {
+			return obj instanceof obj.defaultView.Document;
+		} else if ( obj.ownerDocument && obj.ownerDocument.defaultView ) {
+			return obj instanceof obj.ownerDocument.defaultView.Node;
+		}
+	}
+
+	return false;
 }
diff --git a/tests/dom/isdomnode.js b/tests/dom/isdomnode.js
@@ -9,14 +9,34 @@ import isDomNode from '../../src/dom/isdomnode';
 
 describe( 'isDomNode()', () => {
 	it( 'detects native DOM nodes', () => {
+		expect( isDomNode( document ) ).to.be.true;
+		expect( isDomNode( document.createElement( 'div' ) ) ).to.be.true;
+		expect( isDomNode( document.createTextNode( 'Foo' ) ) ).to.be.true;
+
 		expect( isDomNode( {} ) ).to.be.false;
 		expect( isDomNode( null ) ).to.be.false;
 		expect( isDomNode( undefined ) ).to.be.false;
 		expect( isDomNode( new Date() ) ).to.be.false;
 		expect( isDomNode( 42 ) ).to.be.false;
-		expect( isDomNode( window ) ).to.be.true;
-		expect( isDomNode( document ) ).to.be.true;
-		expect( isDomNode( document.createElement( 'div' ) ) ).to.be.true;
-		expect( isDomNode( document.createTextNode( 'Foo' ) ) ).to.be.true;
+		expect( isDomNode( window ) ).to.be.false;
+	} );
+
+	it( 'works for nodes in an iframe', done => {
+		const iframe = document.createElement( 'iframe' );
+
+		iframe.addEventListener( 'load', () => {
+			const iframeDocument = iframe.contentWindow.document;
+
+			expect( isDomNode( iframeDocument ) ).to.be.true;
+			expect( isDomNode( iframeDocument.createElement( 'div' ) ) ).to.be.true;
+			expect( isDomNode( iframeDocument.createTextNode( 'Foo' ) ) ).to.be.true;
+
+			expect( isDomNode( iframe.contentWindow ) ).to.be.false;
+
+			iframe.remove();
+			done();
+		} );
+
+		document.body.appendChild( iframe );
 	} );
 } );