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.
facebook · Jun 4, 2018 · 43b0829 · 43b0829
1 parent 15767a8
commit 43b0829
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 43 deletions.
diff --git a/fixtures/dom/src/components/fixtures/text-inputs/BubbledInputTestCase.js 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 (
+      <Fixture>
+        <div className="control-box" onChange={this.onChange}>
+          <fieldset>
+            <legend>Controlled Text</legend>
+            <input value={this.state.value} onChange={noop} />
+            <p className="hint">Value: {JSON.stringify(this.state.value)}</p>
+          </fieldset>
+        </div>
+      </Fixture>
+    );
+  }
+}
+
+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
@@ -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 {
           <ReplaceEmailInput />
         </TestCase>
 
+        <TestCase title="Bubbled Change Events" affectedBrowsers="IE9" relatedIssues="708,11609">
+          <TestCase.Steps>
+            <li>Enter text</li>
+          </TestCase.Steps>
+
+          <TestCase.ExpectedResult>
+            Each input's value should change as expected, despite the change
+            event being attached to the parent element
+          </TestCase.ExpectedResult>
+
+          <BubbledInputTestCase type="text" />
+        </TestCase>
+
         <TestCase title="All inputs" description="General test of all inputs">
           <InputTestCase type="text" defaultValue="Text" />
           <InputTestCase type="email" defaultValue="user@example.com" />

diff --git 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