From ad6889fdd2c7b6a2ccea625d7231848571fe8e8d Mon Sep 17 00:00:00 2001 From: Michael Ratcliffe Date: Tue, 13 Feb 2018 13:26:03 +0000 Subject: [PATCH] Bug 1420130 - Remove React Proxy Monkeypatch and see how it affects DAMP r=gregtatum MozReview-Commit-ID: 9L9PYPDGTmj --HG-- extra : rebase_source : e5fbef2c7221b92d54903a53fbb1106cdeb2f550 --- .../boxmodel/components/BoxModelMain.js | 36 ++-- .../inspector/layout/components/Accordion.js | 6 +- .../client/shared/vendor/REACT_UPGRADING.md | 193 +----------------- .../client/shared/vendor/react-dom-dev.js | 163 --------------- devtools/client/shared/vendor/react-dom.js | 163 --------------- 5 files changed, 30 insertions(+), 531 deletions(-) diff --git a/devtools/client/inspector/boxmodel/components/BoxModelMain.js b/devtools/client/inspector/boxmodel/components/BoxModelMain.js index cb9ba89686cb..dc56dc7f7732 100644 --- a/devtools/client/inspector/boxmodel/components/BoxModelMain.js +++ b/devtools/client/inspector/boxmodel/components/BoxModelMain.js @@ -307,27 +307,34 @@ class BoxModelMain extends PureComponent { switch (keyCode) { case KeyCodes.DOM_VK_RETURN: if (!isEditable) { - this.setState({ focusable: true }); - let editableBox = this.layouts[level].get(keyCode); - if (editableBox) { - editableBox.boxModelEditable.focus(); - } + this.setState({ focusable: true }, () => { + let editableBox = this.layouts[level].get(keyCode); + if (editableBox) { + editableBox.boxModelEditable.focus(); + } + }); } break; case KeyCodes.DOM_VK_DOWN: case KeyCodes.DOM_VK_UP: if (!editingMode) { event.preventDefault(); - this.setState({ focusable: false }); + event.stopPropagation(); + this.setState({ focusable: false }, () => { + let nextLayout = this.layouts[level].get(keyCode); - let nextLayout = this.layouts[level].get(keyCode); - this.setAriaActive(nextLayout); + if (!nextLayout) { + return; + } - if (target && target._editable) { - target.blur(); - } + this.setAriaActive(nextLayout); - this.props.boxModelContainer.focus(); + if (target && target._editable) { + target.blur(); + } + + this.props.boxModelContainer.focus(); + }); } break; case KeyCodes.DOM_VK_TAB: @@ -340,8 +347,9 @@ class BoxModelMain extends PureComponent { if (target._editable) { event.preventDefault(); event.stopPropagation(); - this.setState({ focusable: false }); - this.props.boxModelContainer.focus(); + this.setState({ focusable: false }, () => { + this.props.boxModelContainer.focus(); + }); } break; default: diff --git a/devtools/client/inspector/layout/components/Accordion.js b/devtools/client/inspector/layout/components/Accordion.js index 7e40d640d951..435cd7c42dda 100644 --- a/devtools/client/inspector/layout/components/Accordion.js +++ b/devtools/client/inspector/layout/components/Accordion.js @@ -34,11 +34,13 @@ class Accordion extends PureComponent { this.renderContainer = this.renderContainer.bind(this); } - handleHeaderClick(i) { + handleHeaderClick(i, event) { const opened = [...this.state.opened]; const created = [...this.state.created]; const item = this.props.items[i]; + event.stopPropagation(); + opened[i] = !opened[i]; created[i] = true; @@ -67,7 +69,7 @@ class Accordion extends PureComponent { div( { className: "_header", - onClick: () => this.handleHeaderClick(i) }, + onClick: (event) => this.handleHeaderClick(i, event) }, span({ className: arrowClassName }), item.header ), diff --git a/devtools/client/shared/vendor/REACT_UPGRADING.md b/devtools/client/shared/vendor/REACT_UPGRADING.md index b841b5cbb201..744836f8406f 100644 --- a/devtools/client/shared/vendor/REACT_UPGRADING.md +++ b/devtools/client/shared/vendor/REACT_UPGRADING.md @@ -28,11 +28,14 @@ We need to disable minification and tree shaking as they overcomplicate the upgr - Find a method called `function getRollupOutputOptions()` - After `sourcemap: false` add `treeshake: false` and `freeze: false` - Change this: + ```js // Apply dead code elimination and/or minification. isProduction && ``` + To this: + ```js { transformBundle(source) { @@ -78,6 +81,7 @@ if (process.env.NODE_ENV === 'production') { ``` To this: + ```js module.exports = require('./cjs/react-test-renderer-shallow.development.js'); ``` @@ -112,192 +116,3 @@ cp build/packages/react-test-renderer/react-test-renderer-shallow.js ``` From this point we will no longer need your react repository so feel free to delete it. - -## Patching - -### Patching react-dom - -Open `devtools/client/shared/vendor/react-dom.js` and `devtools/client/shared/vendor/react-dom-dev.js`. - -The following change should be made to **BOTH** files. - -To have React's event system working correctly in certain XUL situations, ReactDOM must be monkey patched with a fix. - -Turn this: - -```js -var ReactDOM$2 = Object.freeze({ - default: ReactDOM -}); -``` - -Into this: - -```js -//-------------------------------------------------------------------------------------- -// START MONKEY PATCH -/** - * This section contains a monkey patch for React DOM, so that it functions correctly in - * certain XUL situations. React centralizes events to specific DOM nodes by only - * binding a single listener to the document of the page. It then captures these events, - * and then uses a SyntheticEvent system to dispatch these throughout the page. - * - * In privileged XUL with a XUL iframe, and React in both documents, this system breaks. - * By design, these XUL frames can still talk to each other, while in a normal HTML - * situation, they would not be able to. The events from the XUL iframe propagate to the - * parent document as well. This leads to the React event system incorrectly dispatching - * TWO SyntheticEvents for for every ONE action. - * - * The fix here is trick React into thinking that the owning document for every node in - * a XUL iframe to be the toolbox.xul. This is done by creating a Proxy object that - * captures any usage of HTMLElement.ownerDocument, and then passing in the toolbox.xul - * document rather than (for example) the netmonitor.xul document. React will then hook - * up the event system correctly on the top level controlling document. - * - * @return {object} The proxied and monkey patched ReactDOM - */ -function monkeyPatchReactDOM(ReactDOM) { - // This is the actual monkey patched function. - const reactDomRender = monkeyPatchRender(ReactDOM); - - // Proxied method calls might need to be bound, but do this lazily with caching. - const lazyFunctionBinding = functionLazyBinder(); - - // Create a proxy, but the render property is not writable on the ReactDOM object, so - // a direct proxy will fail with an error. Instead, create a proxy on a a blank object. - // Pass on getting and setting behaviors. - return new Proxy({}, { - get: (target, name) => { - if (name === "render") { - return reactDomRender; - } - return lazyFunctionBinding(ReactDOM, name); - }, - set: (target, name, value) => { - ReactDOM[name] = value; - return true; - } - }); -}; - -/** - * Creates a function that replaces the ReactDOM.render method. It does this by - * creating a proxy for the dom node being mounted. This proxy rewrites the - * "ownerDocument" property to point to the toolbox.xul document. This function - * is only used for XUL iframes inside of a XUL document. - * - * @param {object} ReactDOM - * @return {function} The patched ReactDOM.render function. - */ -function monkeyPatchRender(ReactDOM) { - const elementProxyCache = new WeakMap(); - - return (...args) => { - const container = args[1]; - const toolboxDoc = getToolboxDocIfXulOnly(container); - - if (toolboxDoc) { - // Re-use any existing cached HTMLElement proxies. - let proxy = elementProxyCache.get(container); - - if (!proxy) { - // Proxied method calls need to be bound, but do this lazily. - const lazyFunctionBinding = functionLazyBinder(); - - // Create a proxy to the container HTMLElement. If React tries to access the - // ownerDocument, pass in the toolbox's document, as the event dispatching system - // is rooted from the toolbox document. - proxy = new Proxy(container, { - get: function (target, name) { - if (name === "ownerDocument") { - return toolboxDoc; - } - return lazyFunctionBinding(target, name); - } - }); - - elementProxyCache.set(container, proxy); - } - - // Update the args passed to ReactDOM.render. - args[1] = proxy; - } - - return ReactDOM.render.apply(this, args); - }; -} - -/** - * Try to access the containing toolbox XUL document, but only if all of the iframes - * in the heirarchy are XUL documents. Events dispatch differently in the case of all - * privileged XUL documents. Events that fire in an iframe propagate up to the parent - * frame. This does not happen when HTML is in the mix. Only return the toolbox if - * it matches the proper case of a XUL iframe inside of a XUL document. - * - * In addition to the XUL case, if the panel uses the toolbox's ReactDOM instance, - * this patch needs to be applied as well. This is the case for the inspector. - * - * @param {HTMLElement} node - The DOM node inside of an iframe. - * @return {XULDocument|null} The toolbox.xul document, or null. - */ -function getToolboxDocIfXulOnly(node) { - // This execution context doesn't know about XULDocuments, so don't get the toolbox. - if (typeof XULDocument !== "function") { - return null; - } - - let doc = node.ownerDocument; - const inspectorUrl = "chrome://devtools/content/inspector/inspector.xhtml"; - const netMonitorUrl = "chrome://devtools/content/netmonitor/netmonitor.xhtml"; - const webConsoleUrl = "chrome://devtools/content/webconsole/webconsole.xhtml"; - - while (doc instanceof XULDocument || - doc.location.href === inspectorUrl || - doc.location.href === netMonitorUrl || - doc.location.href === webConsoleUrl) { - const {frameElement} = doc.defaultView; - - if (!frameElement) { - // We're at the root element, and no toolbox was found. - return null; - } - - doc = frameElement.parentElement.ownerDocument; - if (doc.documentURI === "about:devtools-toolbox") { - return doc; - } - } - - return null; -} - -/** - * When accessing proxied functions, the instance becomes unbound to the method. This - * utility either passes a value through if it's not a function, or automatically binds a - * function and caches that bound function for repeated calls. - */ -function functionLazyBinder() { - const boundFunctions = {}; - - return (target, name) => { - if (typeof target[name] === "function") { - // Lazily cache function bindings. - if (boundFunctions[name]) { - return boundFunctions[name]; - } - boundFunctions[name] = target[name].bind(target); - return boundFunctions[name]; - } - return target[name]; - }; -} - -// END MONKEY PATCH -//-------------------------------------------------------------------------------------- - -ReactDOM = monkeyPatchReactDOM(ReactDOM); - -var ReactDOM$2 = Object.freeze({ - default: ReactDOM -}); -``` diff --git a/devtools/client/shared/vendor/react-dom-dev.js b/devtools/client/shared/vendor/react-dom-dev.js index f20486af2269..032f862fb594 100644 --- a/devtools/client/shared/vendor/react-dom-dev.js +++ b/devtools/client/shared/vendor/react-dom-dev.js @@ -16051,169 +16051,6 @@ var foundDevTools = DOMRenderer.injectIntoDevTools({ } } -//-------------------------------------------------------------------------------------- -// START MONKEY PATCH -/** - * This section contains a monkey patch for React DOM, so that it functions correctly in - * certain XUL situations. React centralizes events to specific DOM nodes by only - * binding a single listener to the document of the page. It then captures these events, - * and then uses a SyntheticEvent system to dispatch these throughout the page. - * - * In privileged XUL with a XUL iframe, and React in both documents, this system breaks. - * By design, these XUL frames can still talk to each other, while in a normal HTML - * situation, they would not be able to. The events from the XUL iframe propagate to the - * parent document as well. This leads to the React event system incorrectly dispatching - * TWO SyntheticEvents for for every ONE action. - * - * The fix here is trick React into thinking that the owning document for every node in - * a XUL iframe to be the toolbox.xul. This is done by creating a Proxy object that - * captures any usage of HTMLElement.ownerDocument, and then passing in the toolbox.xul - * document rather than (for example) the netmonitor.xul document. React will then hook - * up the event system correctly on the top level controlling document. - * - * @return {object} The proxied and monkey patched ReactDOM - */ -function monkeyPatchReactDOM(ReactDOM) { - // This is the actual monkey patched function. - const reactDomRender = monkeyPatchRender(ReactDOM); - - // Proxied method calls might need to be bound, but do this lazily with caching. - const lazyFunctionBinding = functionLazyBinder(); - - // Create a proxy, but the render property is not writable on the ReactDOM object, so - // a direct proxy will fail with an error. Instead, create a proxy on a a blank object. - // Pass on getting and setting behaviors. - return new Proxy({}, { - get: (target, name) => { - if (name === "render") { - return reactDomRender; - } - return lazyFunctionBinding(ReactDOM, name); - }, - set: (target, name, value) => { - ReactDOM[name] = value; - return true; - } - }); -}; - -/** - * Creates a function that replaces the ReactDOM.render method. It does this by - * creating a proxy for the dom node being mounted. This proxy rewrites the - * "ownerDocument" property to point to the toolbox.xul document. This function - * is only used for XUL iframes inside of a XUL document. - * - * @param {object} ReactDOM - * @return {function} The patched ReactDOM.render function. - */ -function monkeyPatchRender(ReactDOM) { - const elementProxyCache = new WeakMap(); - - return (...args) => { - const container = args[1]; - const toolboxDoc = getToolboxDocIfXulOnly(container); - - if (toolboxDoc) { - // Re-use any existing cached HTMLElement proxies. - let proxy = elementProxyCache.get(container); - - if (!proxy) { - // Proxied method calls need to be bound, but do this lazily. - const lazyFunctionBinding = functionLazyBinder(); - - // Create a proxy to the container HTMLElement. If React tries to access the - // ownerDocument, pass in the toolbox's document, as the event dispatching system - // is rooted from the toolbox document. - proxy = new Proxy(container, { - get: function (target, name) { - if (name === "ownerDocument") { - return toolboxDoc; - } - return lazyFunctionBinding(target, name); - } - }); - - elementProxyCache.set(container, proxy); - } - - // Update the args passed to ReactDOM.render. - args[1] = proxy; - } - - return ReactDOM.render.apply(this, args); - }; -} - -/** - * Try to access the containing toolbox XUL document, but only if all of the iframes - * in the heirarchy are XUL documents. Events dispatch differently in the case of all - * privileged XUL documents. Events that fire in an iframe propagate up to the parent - * frame. This does not happen when HTML is in the mix. Only return the toolbox if - * it matches the proper case of a XUL iframe inside of a XUL document. - * - * In addition to the XUL case, if the panel uses the toolbox's ReactDOM instance, - * this patch needs to be applied as well. This is the case for the inspector. - * - * @param {HTMLElement} node - The DOM node inside of an iframe. - * @return {XULDocument|null} The toolbox.xul document, or null. - */ -function getToolboxDocIfXulOnly(node) { - // This execution context doesn't know about XULDocuments, so don't get the toolbox. - if (typeof XULDocument !== "function") { - return null; - } - - let doc = node.ownerDocument; - const inspectorUrl = "chrome://devtools/content/inspector/inspector.xhtml"; - const netMonitorUrl = "chrome://devtools/content/netmonitor/netmonitor.xhtml"; - const webConsoleUrl = "chrome://devtools/content/webconsole/webconsole.xhtml"; - - while (doc instanceof XULDocument || - doc.location.href === inspectorUrl || - doc.location.href === netMonitorUrl || - doc.location.href === webConsoleUrl) { - const {frameElement} = doc.defaultView; - - if (!frameElement) { - // We're at the root element, and no toolbox was found. - return null; - } - - doc = frameElement.parentElement.ownerDocument; - if (doc.documentURI === "about:devtools-toolbox") { - return doc; - } - } - - return null; -} - -/** - * When accessing proxied functions, the instance becomes unbound to the method. This - * utility either passes a value through if it's not a function, or automatically binds a - * function and caches that bound function for repeated calls. - */ -function functionLazyBinder() { - const boundFunctions = {}; - - return (target, name) => { - if (typeof target[name] === "function") { - // Lazily cache function bindings. - if (boundFunctions[name]) { - return boundFunctions[name]; - } - boundFunctions[name] = target[name].bind(target); - return boundFunctions[name]; - } - return target[name]; - }; -} - -// END MONKEY PATCH -//-------------------------------------------------------------------------------------- - -ReactDOM = monkeyPatchReactDOM(ReactDOM); - var ReactDOM$2 = Object.freeze({ default: ReactDOM }); diff --git a/devtools/client/shared/vendor/react-dom.js b/devtools/client/shared/vendor/react-dom.js index 6fe18c4fe6aa..9cac90872d7d 100644 --- a/devtools/client/shared/vendor/react-dom.js +++ b/devtools/client/shared/vendor/react-dom.js @@ -12918,169 +12918,6 @@ var foundDevTools = DOMRenderer.injectIntoDevTools({ rendererPackageName: 'react-dom' }); -//-------------------------------------------------------------------------------------- -// START MONKEY PATCH -/** - * This section contains a monkey patch for React DOM, so that it functions correctly in - * certain XUL situations. React centralizes events to specific DOM nodes by only - * binding a single listener to the document of the page. It then captures these events, - * and then uses a SyntheticEvent system to dispatch these throughout the page. - * - * In privileged XUL with a XUL iframe, and React in both documents, this system breaks. - * By design, these XUL frames can still talk to each other, while in a normal HTML - * situation, they would not be able to. The events from the XUL iframe propagate to the - * parent document as well. This leads to the React event system incorrectly dispatching - * TWO SyntheticEvents for for every ONE action. - * - * The fix here is trick React into thinking that the owning document for every node in - * a XUL iframe to be the toolbox.xul. This is done by creating a Proxy object that - * captures any usage of HTMLElement.ownerDocument, and then passing in the toolbox.xul - * document rather than (for example) the netmonitor.xul document. React will then hook - * up the event system correctly on the top level controlling document. - * - * @return {object} The proxied and monkey patched ReactDOM - */ -function monkeyPatchReactDOM(ReactDOM) { - // This is the actual monkey patched function. - const reactDomRender = monkeyPatchRender(ReactDOM); - - // Proxied method calls might need to be bound, but do this lazily with caching. - const lazyFunctionBinding = functionLazyBinder(); - - // Create a proxy, but the render property is not writable on the ReactDOM object, so - // a direct proxy will fail with an error. Instead, create a proxy on a a blank object. - // Pass on getting and setting behaviors. - return new Proxy({}, { - get: (target, name) => { - if (name === "render") { - return reactDomRender; - } - return lazyFunctionBinding(ReactDOM, name); - }, - set: (target, name, value) => { - ReactDOM[name] = value; - return true; - } - }); -}; - -/** - * Creates a function that replaces the ReactDOM.render method. It does this by - * creating a proxy for the dom node being mounted. This proxy rewrites the - * "ownerDocument" property to point to the toolbox.xul document. This function - * is only used for XUL iframes inside of a XUL document. - * - * @param {object} ReactDOM - * @return {function} The patched ReactDOM.render function. - */ -function monkeyPatchRender(ReactDOM) { - const elementProxyCache = new WeakMap(); - - return (...args) => { - const container = args[1]; - const toolboxDoc = getToolboxDocIfXulOnly(container); - - if (toolboxDoc) { - // Re-use any existing cached HTMLElement proxies. - let proxy = elementProxyCache.get(container); - - if (!proxy) { - // Proxied method calls need to be bound, but do this lazily. - const lazyFunctionBinding = functionLazyBinder(); - - // Create a proxy to the container HTMLElement. If React tries to access the - // ownerDocument, pass in the toolbox's document, as the event dispatching system - // is rooted from the toolbox document. - proxy = new Proxy(container, { - get: function (target, name) { - if (name === "ownerDocument") { - return toolboxDoc; - } - return lazyFunctionBinding(target, name); - } - }); - - elementProxyCache.set(container, proxy); - } - - // Update the args passed to ReactDOM.render. - args[1] = proxy; - } - - return ReactDOM.render.apply(this, args); - }; -} - -/** - * Try to access the containing toolbox XUL document, but only if all of the iframes - * in the heirarchy are XUL documents. Events dispatch differently in the case of all - * privileged XUL documents. Events that fire in an iframe propagate up to the parent - * frame. This does not happen when HTML is in the mix. Only return the toolbox if - * it matches the proper case of a XUL iframe inside of a XUL document. - * - * In addition to the XUL case, if the panel uses the toolbox's ReactDOM instance, - * this patch needs to be applied as well. This is the case for the inspector. - * - * @param {HTMLElement} node - The DOM node inside of an iframe. - * @return {XULDocument|null} The toolbox.xul document, or null. - */ -function getToolboxDocIfXulOnly(node) { - // This execution context doesn't know about XULDocuments, so don't get the toolbox. - if (typeof XULDocument !== "function") { - return null; - } - - let doc = node.ownerDocument; - const inspectorUrl = "chrome://devtools/content/inspector/inspector.xhtml"; - const netMonitorUrl = "chrome://devtools/content/netmonitor/netmonitor.xhtml"; - const webConsoleUrl = "chrome://devtools/content/webconsole/webconsole.xhtml"; - - while (doc instanceof XULDocument || - doc.location.href === inspectorUrl || - doc.location.href === netMonitorUrl || - doc.location.href === webConsoleUrl) { - const {frameElement} = doc.defaultView; - - if (!frameElement) { - // We're at the root element, and no toolbox was found. - return null; - } - - doc = frameElement.parentElement.ownerDocument; - if (doc.documentURI === "about:devtools-toolbox") { - return doc; - } - } - - return null; -} - -/** - * When accessing proxied functions, the instance becomes unbound to the method. This - * utility either passes a value through if it's not a function, or automatically binds a - * function and caches that bound function for repeated calls. - */ -function functionLazyBinder() { - const boundFunctions = {}; - - return (target, name) => { - if (typeof target[name] === "function") { - // Lazily cache function bindings. - if (boundFunctions[name]) { - return boundFunctions[name]; - } - boundFunctions[name] = target[name].bind(target); - return boundFunctions[name]; - } - return target[name]; - }; -} - -// END MONKEY PATCH -//-------------------------------------------------------------------------------------- - -ReactDOM = monkeyPatchReactDOM(ReactDOM); - var ReactDOM$2 = Object.freeze({ default: ReactDOM });