Skip to content

Commit

Permalink
Merge pull request react-bootstrap#480 from martijnvermaat/null-owner…
Browse files Browse the repository at this point in the history
…document

[fixed] Don't try to access .ownerDocument on null
  • Loading branch information
mtscout6 committed Apr 7, 2015
2 parents a58cff9 + 7175431 commit eb3ecc4
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 10 deletions.
2 changes: 1 addition & 1 deletion src/AffixMixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
3 changes: 2 additions & 1 deletion src/DropdownStateMixin.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import domUtils from './utils/domUtils';
import EventListener from './utils/EventListener';

/**
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 3 additions & 1 deletion src/FadeMixin.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import domUtils from './utils/domUtils';

// TODO: listen for onTransitionEnd to remove el
function getElementsAndSelf (root, classes){
Expand Down Expand Up @@ -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');
Expand Down
9 changes: 6 additions & 3 deletions src/Modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';


Expand Down Expand Up @@ -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) {
Expand All @@ -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/, '');
},

Expand Down
3 changes: 2 additions & 1 deletion src/OverlayMixin.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import CustomPropTypes from './utils/CustomPropTypes';
import domUtils from './utils/domUtils';

export default {
propTypes: {
Expand Down Expand Up @@ -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;
}
};
20 changes: 17 additions & 3 deletions src/utils/domUtils.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,24 @@
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
*
* @param {HTMLElement} elem
* @returns {CssStyle}
*/
function getComputedStyles(elem) {
return elem.ownerDocument.defaultView.getComputedStyle(elem, null);
return ownerDocument(elem).defaultView.getComputedStyle(elem, null);
}

/**
Expand All @@ -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
Expand Down Expand Up @@ -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' &&
Expand All @@ -101,6 +114,7 @@ function offsetParentFunc(elem) {
}

export default {
ownerDocument: ownerDocument,
getComputedStyles: getComputedStyles,
getOffset: getOffset,
getPosition: getPosition,
Expand Down
20 changes: 20 additions & 0 deletions test/OverlayMixinSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<OnlyOverlay overlay={<div id="test1" />} />
);

assert.equal(overlayInstance.getOverlayDOMNode().nodeName, 'DIV');
});
});

0 comments on commit eb3ecc4

Please sign in to comment.