Skip to content

Commit

Permalink
[fixed] Use .ownerDocument instead of root document
Browse files Browse the repository at this point in the history
Event listeners are attached to the component DOM node's owner
document instead of to the window root document. The two are not
the same when the component is mounted inside an IFrame, for
example using react-frame-component [1].

[1] https://github.com/ryanseddon/react-frame-component
  • Loading branch information
martijnvermaat committed Mar 31, 2015
1 parent 560b0e8 commit ee0382e
Show file tree
Hide file tree
Showing 6 changed files with 12 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(document, 'click', this.checkPositionWithEventLoop);
EventListener.listen(React.findDOMNode(this).ownerDocument, 'click', this.checkPositionWithEventLoop);
},

componentWillUnmount() {
Expand Down
6 changes: 4 additions & 2 deletions src/DropdownStateMixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,12 @@ const DropdownStateMixin = {
},

bindRootCloseHandlers() {
let doc = React.findDOMNode(this).ownerDocument;

this._onDocumentClickListener =
EventListener.listen(document, 'click', this.handleDocumentClick);
EventListener.listen(doc, 'click', this.handleDocumentClick);
this._onDocumentKeyupListener =
EventListener.listen(document, 'keyup', this.handleDocumentKeyUp);
EventListener.listen(doc, 'keyup', this.handleDocumentKeyUp);
},

unbindRootCloseHandlers() {
Expand Down
2 changes: 1 addition & 1 deletion src/FadeMixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export default {

componentWillUnmount: function () {
let els = getElementsAndSelf(React.findDOMNode(this), ['fade']),
container = (this.props.container && React.findDOMNode(this.props.container)) || document.body;
container = (this.props.container && React.findDOMNode(this.props.container)) || React.findDOMNode(this).ownerDocument.body;

if (els.length) {
this._fadeOutEl = document.createElement('div');
Expand Down
6 changes: 3 additions & 3 deletions src/Modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,9 @@ const Modal = React.createClass({

componentDidMount() {
this._onDocumentKeyupListener =
EventListener.listen(document, 'keyup', this.handleDocumentKeyUp);
EventListener.listen(React.findDOMNode(this).ownerDocument, 'keyup', this.handleDocumentKeyUp);

let container = (this.props.container && React.findDOMNode(this.props.container)) || document.body;
let container = (this.props.container && React.findDOMNode(this.props.container)) || React.findDOMNode(this).ownerDocument.body;
container.className += container.className.length ? ' modal-open' : 'modal-open';

if (this.props.backdrop) {
Expand All @@ -146,7 +146,7 @@ const Modal = React.createClass({

componentWillUnmount() {
this._onDocumentKeyupListener.remove();
let container = (this.props.container && React.findDOMNode(this.props.container)) || document.body;
let container = (this.props.container && React.findDOMNode(this.props.container)) || React.findDOMNode(this).ownerDocument.body;
container.className = container.className.replace(/ ?modal-open/, '');
},

Expand Down
2 changes: 1 addition & 1 deletion src/OverlayMixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,6 @@ export default {
},

getContainerDOMNode() {
return React.findDOMNode(this.props.container || document.body);
return React.findDOMNode(this.props.container || React.findDOMNode(this).ownerDocument.body);
}
};
4 changes: 2 additions & 2 deletions src/utils/domUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ function getOffset(DOMNode) {
return window.jQuery(DOMNode).offset();
}

let docElem = document.documentElement;
let docElem = DOMNode.ownerDocument.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 +89,7 @@ function getPosition(elem, offsetParent) {
* @returns {HTMLElement}
*/
function offsetParentFunc(elem) {
let docElem = document.documentElement;
let docElem = elem.ownerDocument.documentElement;
let offsetParent = elem.offsetParent || docElem;

while ( offsetParent && ( offsetParent.nodeName !== 'HTML' &&
Expand Down

5 comments on commit ee0382e

@lukemcgregor
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

React.findDOMNode(this) can return null, (for example if you have a modal and only want to render the overlay)

@martijnvermaat
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lukemcgregor Can you give a small example that fails?

@lukemcgregor
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lukemcgregor
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can fix it by not returning null from the render but since a few versions ago null is a valid return type

@martijnvermaat
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #480

Please sign in to comment.