From 7175431221552cf5d37b5d7cb3c3582bfbdca50d Mon Sep 17 00:00:00 2001 From: Martijn Vermaat Date: Sat, 4 Apr 2015 21:43:06 +0200 Subject: [PATCH] [fixed] Don't try to access .ownerDocument on null We previously did not take into account that React.findDOMNode(this) can return null, (for example if you have a modal and only want to render the overlay). We now fallback to the root document in that case. https://github.com/react-bootstrap/react-bootstrap/commit/ee0382e --- src/AffixMixin.js | 2 +- src/DropdownStateMixin.js | 3 ++- src/FadeMixin.js | 4 +++- src/Modal.js | 9 ++++++--- src/OverlayMixin.js | 3 ++- src/utils/domUtils.js | 20 +++++++++++++++++--- test/OverlayMixinSpec.js | 20 ++++++++++++++++++++ 7 files changed, 51 insertions(+), 10 deletions(-) diff --git a/src/AffixMixin.js b/src/AffixMixin.js index 957bd360ed..11e1214ee8 100644 --- a/src/AffixMixin.js +++ b/src/AffixMixin.js @@ -104,7 +104,7 @@ const AffixMixin = { this._onWindowScrollListener = EventListener.listen(window, 'scroll', this.checkPosition); this._onDocumentClickListener = - EventListener.listen(React.findDOMNode(this).ownerDocument, 'click', this.checkPositionWithEventLoop); + EventListener.listen(domUtils.ownerDocument(this), 'click', this.checkPositionWithEventLoop); }, componentWillUnmount() { diff --git a/src/DropdownStateMixin.js b/src/DropdownStateMixin.js index a72bcdebe1..586e905217 100644 --- a/src/DropdownStateMixin.js +++ b/src/DropdownStateMixin.js @@ -1,4 +1,5 @@ import React from 'react'; +import domUtils from './utils/domUtils'; import EventListener from './utils/EventListener'; /** @@ -56,7 +57,7 @@ const DropdownStateMixin = { }, bindRootCloseHandlers() { - let doc = React.findDOMNode(this).ownerDocument; + let doc = domUtils.ownerDocument(this); this._onDocumentClickListener = EventListener.listen(doc, 'click', this.handleDocumentClick); diff --git a/src/FadeMixin.js b/src/FadeMixin.js index 0ed31da42c..5156153d6b 100644 --- a/src/FadeMixin.js +++ b/src/FadeMixin.js @@ -1,4 +1,5 @@ import React from 'react'; +import domUtils from './utils/domUtils'; // TODO: listen for onTransitionEnd to remove el function getElementsAndSelf (root, classes){ @@ -57,7 +58,8 @@ export default { componentWillUnmount: function () { let els = getElementsAndSelf(React.findDOMNode(this), ['fade']), - container = (this.props.container && React.findDOMNode(this.props.container)) || React.findDOMNode(this).ownerDocument.body; + container = (this.props.container && React.findDOMNode(this.props.container)) || + domUtils.ownerDocument(this).body; if (els.length) { this._fadeOutEl = document.createElement('div'); diff --git a/src/Modal.js b/src/Modal.js index 3299fb6a65..659852d2b1 100644 --- a/src/Modal.js +++ b/src/Modal.js @@ -2,6 +2,7 @@ import React from 'react'; import classSet from 'classnames'; import BootstrapMixin from './BootstrapMixin'; import FadeMixin from './FadeMixin'; +import domUtils from './utils/domUtils'; import EventListener from './utils/EventListener'; @@ -128,9 +129,10 @@ const Modal = React.createClass({ componentDidMount() { this._onDocumentKeyupListener = - EventListener.listen(React.findDOMNode(this).ownerDocument, 'keyup', this.handleDocumentKeyUp); + EventListener.listen(domUtils.ownerDocument(this), 'keyup', this.handleDocumentKeyUp); - let container = (this.props.container && React.findDOMNode(this.props.container)) || React.findDOMNode(this).ownerDocument.body; + let container = (this.props.container && React.findDOMNode(this.props.container)) || + domUtils.ownerDocument(this).body; container.className += container.className.length ? ' modal-open' : 'modal-open'; if (this.props.backdrop) { @@ -146,7 +148,8 @@ const Modal = React.createClass({ componentWillUnmount() { this._onDocumentKeyupListener.remove(); - let container = (this.props.container && React.findDOMNode(this.props.container)) || React.findDOMNode(this).ownerDocument.body; + let container = (this.props.container && React.findDOMNode(this.props.container)) || + domUtils.ownerDocument(this).body; container.className = container.className.replace(/ ?modal-open/, ''); }, diff --git a/src/OverlayMixin.js b/src/OverlayMixin.js index 5e99799ecf..76524ed61c 100644 --- a/src/OverlayMixin.js +++ b/src/OverlayMixin.js @@ -1,5 +1,6 @@ import React from 'react'; import CustomPropTypes from './utils/CustomPropTypes'; +import domUtils from './utils/domUtils'; export default { propTypes: { @@ -63,6 +64,6 @@ export default { }, getContainerDOMNode() { - return React.findDOMNode(this.props.container || React.findDOMNode(this).ownerDocument.body); + return React.findDOMNode(this.props.container) || domUtils.ownerDocument(this).body; } }; diff --git a/src/utils/domUtils.js b/src/utils/domUtils.js index 8d62e11bf9..d69c292e1f 100644 --- a/src/utils/domUtils.js +++ b/src/utils/domUtils.js @@ -1,3 +1,16 @@ +import React from 'react'; + +/** + * Get elements owner document + * + * @param {ReactComponent|HTMLElement} componentOrElement + * @returns {HTMLElement} + */ +function ownerDocument(componentOrElement) { + let elem = React.findDOMNode(componentOrElement); + return (elem && elem.ownerDocument) || document; +} + /** * Shortcut to compute element style * @@ -5,7 +18,7 @@ * @returns {CssStyle} */ function getComputedStyles(elem) { - return elem.ownerDocument.defaultView.getComputedStyle(elem, null); + return ownerDocument(elem).defaultView.getComputedStyle(elem, null); } /** @@ -21,7 +34,7 @@ function getOffset(DOMNode) { return window.jQuery(DOMNode).offset(); } - let docElem = DOMNode.ownerDocument.documentElement; + let docElem = ownerDocument(DOMNode).documentElement; let box = { top: 0, left: 0 }; // If we don't have gBCR, just use 0,0 rather than error @@ -89,7 +102,7 @@ function getPosition(elem, offsetParent) { * @returns {HTMLElement} */ function offsetParentFunc(elem) { - let docElem = elem.ownerDocument.documentElement; + let docElem = ownerDocument(elem).documentElement; let offsetParent = elem.offsetParent || docElem; while ( offsetParent && ( offsetParent.nodeName !== 'HTML' && @@ -101,6 +114,7 @@ function offsetParentFunc(elem) { } export default { + ownerDocument: ownerDocument, getComputedStyles: getComputedStyles, getOffset: getOffset, getPosition: getPosition, diff --git a/test/OverlayMixinSpec.js b/test/OverlayMixinSpec.js index 21df6d6106..218b6d94af 100644 --- a/test/OverlayMixinSpec.js +++ b/test/OverlayMixinSpec.js @@ -60,4 +60,24 @@ describe('OverlayMixin', function () { assert.equal(instance.refs.overlay.getOverlayDOMNode(), null); }); + + it('Should render only an overlay', function() { + let OnlyOverlay = React.createClass({ + mixins: [OverlayMixin], + + render: function() { + return null; + }, + + renderOverlay: function() { + return this.props.overlay; + } + }); + + let overlayInstance = ReactTestUtils.renderIntoDocument( + } /> + ); + + assert.equal(overlayInstance.getOverlayDOMNode().nodeName, 'DIV'); + }); });