From 43b0829f8a167599d6c660703480bab8b5974c96 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Mon, 4 Jun 2018 07:50:49 -0400 Subject: [PATCH] Remove forced bubble code to prevent undo errors in IE9 Before this commit, IE9 would force a change event to fire when the onpropertychange callback fired. This threw an error when undoing text changes: React would try to update a text node that was detached from the DOM. --- .../text-inputs/BubbledInputTestCase.js | 32 ++++++++++++++ .../components/fixtures/text-inputs/index.js | 14 ++++++ .../react-dom/src/events/ChangeEventPlugin.js | 43 ------------------- 3 files changed, 46 insertions(+), 43 deletions(-) create mode 100644 fixtures/dom/src/components/fixtures/text-inputs/BubbledInputTestCase.js diff --git a/fixtures/dom/src/components/fixtures/text-inputs/BubbledInputTestCase.js b/fixtures/dom/src/components/fixtures/text-inputs/BubbledInputTestCase.js new file mode 100644 index 0000000000000..94d92efb7a5f4 --- /dev/null +++ b/fixtures/dom/src/components/fixtures/text-inputs/BubbledInputTestCase.js @@ -0,0 +1,32 @@ +import Fixture from '../../Fixture'; +const React = window.React; + +const noop = () => {}; + +class BubbledInputTestCase extends React.Component { + state = { + value: '', + }; + + onChange = event => { + this.setState({ + value: (event.srcElement || event.target).value, + }); + }; + + render() { + return ( + +
+
+ Controlled Text + +

Value: {JSON.stringify(this.state.value)}

+
+
+ + ); + } +} + +export default BubbledInputTestCase; diff --git a/fixtures/dom/src/components/fixtures/text-inputs/index.js b/fixtures/dom/src/components/fixtures/text-inputs/index.js index 54026257fd986..7a9804ae2db34 100644 --- a/fixtures/dom/src/components/fixtures/text-inputs/index.js +++ b/fixtures/dom/src/components/fixtures/text-inputs/index.js @@ -2,6 +2,7 @@ import Fixture from '../../Fixture'; import FixtureSet from '../../FixtureSet'; import TestCase from '../../TestCase'; import InputTestCase from './InputTestCase'; +import BubbledInputTestCase from './BubbledInputTestCase'; import ReplaceEmailInput from './ReplaceEmailInput'; const React = window.React; @@ -141,6 +142,19 @@ class TextInputFixtures extends React.Component { + + +
  • Enter text
    • +     + + + Each input's value should change as expected, despite the change + event being attached to the parent element + + + +     + diff --git a/packages/react-dom/src/events/ChangeEventPlugin.js b/packages/react-dom/src/events/ChangeEventPlugin.js index ce0d328d876d5..a9f3d8150a5ce 100644 --- a/packages/react-dom/src/events/ChangeEventPlugin.js +++ b/packages/react-dom/src/events/ChangeEventPlugin.js @@ -5,10 +5,8 @@ * LICENSE file in the root directory of this source tree. */ -import * as EventPluginHub from 'events/EventPluginHub'; import {accumulateTwoPhaseDispatches} from 'events/EventPropagators'; import {enqueueStateRestore} from 'events/ReactControlledComponent'; -import {batchedUpdates} from 'events/ReactGenericBatching'; import SyntheticEvent from 'events/SyntheticEvent'; import isTextInputElement from 'shared/isTextInputElement'; import ExecutionEnvironment from 'fbjs/lib/ExecutionEnvironment'; @@ -23,7 +21,6 @@ import { TOP_KEY_UP, TOP_SELECTION_CHANGE, } from './DOMTopLevelEventTypes'; -import getEventTarget from './getEventTarget'; import isEventSupported from './isEventSupported'; import {getNodeFromInstance} from '../client/ReactDOMComponentTree'; import * as inputValueTracking from '../client/inputValueTracking'; @@ -77,31 +74,6 @@ function shouldUseChangeEvent(elem) { ); } -function manualDispatchChangeEvent(nativeEvent) { - const event = createAndAccumulateChangeEvent( - activeElementInst, - nativeEvent, - getEventTarget(nativeEvent), - ); - - // If change and propertychange bubbled, we'd just bind to it like all the - // other events and have it go through ReactBrowserEventEmitter. Since it - // doesn't, we manually listen for the events and so we have to enqueue and - // process the abstract event manually. - // - // Batching is necessary here in order to ensure that all event handlers run - // before the next rerender (including event handlers attached to ancestor - // elements instead of directly on the input). Without this, controlled - // components don't work properly in conjunction with event bubbling because - // the component is rerendered and the value reverted before all the event - // handlers can run. See https://github.com/facebook/react/issues/708. - batchedUpdates(runEventInBatch, event); -} - -function runEventInBatch(event) { - EventPluginHub.runEventsInBatch(event, false); -} - function getInstIfValueChanged(targetInst) { const targetNode = getNodeFromInstance(targetInst); if (inputValueTracking.updateValueIfChanged(targetNode)) { @@ -135,7 +107,6 @@ if (ExecutionEnvironment.canUseDOM) { function startWatchingForValueChange(target, targetInst) { activeElement = target; activeElementInst = targetInst; - activeElement.attachEvent('onpropertychange', handlePropertyChange); } /** @@ -146,24 +117,10 @@ function stopWatchingForValueChange() { if (!activeElement) { return; } - activeElement.detachEvent('onpropertychange', handlePropertyChange); activeElement = null; activeElementInst = null; } -/** - * (For IE <=9) Handles a propertychange event, sending a `change` event if - * the value of the active element has changed. - */ -function handlePropertyChange(nativeEvent) { - if (nativeEvent.propertyName !== 'value') { - return; - } - if (getInstIfValueChanged(activeElementInst)) { - manualDispatchChangeEvent(nativeEvent); - } -} - function handleEventsForInputEventPolyfill(topLevelType, target, targetInst) { if (topLevelType === TOP_FOCUS) { // In IE9, propertychange fires for most input events but is buggy and