Merge pull request #3246 from spicyj/onclick

Revert "Revert #1536"

src/browser/eventPlugins/DefaultEventPluginOrder.js

@@ -30,8 +30,7 @@ var DefaultEventPluginOrder = [
   keyOf({ChangeEventPlugin: null}),
   keyOf({SelectEventPlugin: null}),
   keyOf({BeforeInputEventPlugin: null}),
-  keyOf({AnalyticsEventPlugin: null}),
-  keyOf({MobileSafariClickEventPlugin: null})
+  keyOf({AnalyticsEventPlugin: null})
 ];
 
 module.exports = DefaultEventPluginOrder;

src/browser/eventPlugins/MobileSafariClickEventPlugin.js

@@ -1,56 +0,0 @@
-/**
- * Copyright 2013-2015, Facebook, Inc.
- * All rights reserved.
- *
- * This source code is licensed under the BSD-style license found in the
- * LICENSE file in the root directory of this source tree. An additional grant
- * of patent rights can be found in the PATENTS file in the same directory.
- *
- * @providesModule MobileSafariClickEventPlugin
- * @typechecks static-only
- */
-
-'use strict';
-
-var EventConstants = require('EventConstants');
-
-var emptyFunction = require('emptyFunction');
-
-var topLevelTypes = EventConstants.topLevelTypes;
-
-/**
- * Mobile Safari does not fire properly bubble click events on non-interactive
- * elements, which means delegated click listeners do not fire. The workaround
- * for this bug involves attaching an empty click listener on the target node.
- *
- * This particular plugin works around the bug by attaching an empty click
- * listener on `touchstart` (which does fire on every element).
- */
-var MobileSafariClickEventPlugin = {
-
-  eventTypes: null,
-
-  /**
-   * @param {string} topLevelType Record from `EventConstants`.
-   * @param {DOMEventTarget} topLevelTarget The listening component root node.
-   * @param {string} topLevelTargetID ID of `topLevelTarget`.
-   * @param {object} nativeEvent Native browser event.
-   * @return {*} An accumulation of synthetic events.
-   * @see {EventPluginHub.extractEvents}
-   */
-  extractEvents: function(
-      topLevelType,
-      topLevelTarget,
-      topLevelTargetID,
-      nativeEvent) {
-    if (topLevelType === topLevelTypes.topTouchStart) {
-      var target = nativeEvent.target;
-      if (target && !target.onclick) {
-        target.onclick = emptyFunction;
-      }
-    }
-  }
-
-};
-
-module.exports = MobileSafariClickEventPlugin;

src/browser/eventPlugins/SimpleEventPlugin.js

@@ -12,8 +12,10 @@
 'use strict';
 
 var EventConstants = require('EventConstants');
+var EventListener = require('EventListener');
 var EventPluginUtils = require('EventPluginUtils');
 var EventPropagators = require('EventPropagators');
+var ReactMount = require('ReactMount');
 var SyntheticClipboardEvent = require('SyntheticClipboardEvent');
 var SyntheticEvent = require('SyntheticEvent');
 var SyntheticFocusEvent = require('SyntheticFocusEvent');
@@ -24,8 +26,8 @@ var SyntheticTouchEvent = require('SyntheticTouchEvent');
 var SyntheticUIEvent = require('SyntheticUIEvent');
 var SyntheticWheelEvent = require('SyntheticWheelEvent');
 
+var emptyFunction = require('emptyFunction');
 var getEventCharCode = require('getEventCharCode');
-
 var invariant = require('invariant');
 var keyOf = require('keyOf');
 var warning = require('warning');
@@ -289,6 +291,9 @@ for (var type in topLevelEventsToDispatchConfig) {
   topLevelEventsToDispatchConfig[type].dependencies = [type];
 }
 
+var ON_CLICK_KEY = keyOf({onClick: null});
+var onClickListeners = {};
+
 var SimpleEventPlugin = {
 
   eventTypes: eventTypes,
@@ -417,6 +422,30 @@ var SimpleEventPlugin = {
     );
     EventPropagators.accumulateTwoPhaseDispatches(event);
     return event;
+  },
+
+  didPutListener: function(id, registrationName, listener) {
+    // Mobile Safari does not fire properly bubble click events on
+    // non-interactive elements, which means delegated click listeners do not
+    // fire. The workaround for this bug involves attaching an empty click
+    // listener on the target node.
+    if (registrationName === ON_CLICK_KEY) {
+      var node = ReactMount.getNode(id);
+      if (!onClickListeners[id]) {
+        onClickListeners[id] = EventListener.listen(
+          node,
+          'click',
+          emptyFunction
+        );
+      }
+    }
+  },
+
+  willDeleteListener: function(id, registrationName) {
+    if (registrationName === ON_CLICK_KEY) {
+      onClickListeners[id].remove();
+      delete onClickListeners[id];
+    }
   }
 
 };

src/browser/ui/ReactDOMComponent.js

@@ -368,7 +368,12 @@ ReactDOMComponent.Mixin = {
           }
         }
       } else if (registrationNameModules.hasOwnProperty(propKey)) {
-        deleteListener(this._rootNodeID, propKey);
+        if (lastProps[propKey]) {
+          // Only call deleteListener if there was a listener previously or
+          // else willDeleteListener gets called when there wasn't actually a
+          // listener (e.g., onClick={null})
+          deleteListener(this._rootNodeID, propKey);
+        }
       } else if (
           DOMProperty.isStandardName[propKey] ||
           DOMProperty.isCustomAttribute(propKey)) {
@@ -412,7 +417,11 @@ ReactDOMComponent.Mixin = {
           styleUpdates = nextProp;
         }
       } else if (registrationNameModules.hasOwnProperty(propKey)) {
-        putListener(this._rootNodeID, propKey, nextProp, transaction);
+        if (nextProp) {
+          putListener(this._rootNodeID, propKey, nextProp, transaction);
+        } else if (lastProp) {
+          deleteListener(this._rootNodeID, propKey);
+        }
       } else if (
           DOMProperty.isStandardName[propKey] ||
           DOMProperty.isCustomAttribute(propKey)) {

src/browser/ui/ReactDefaultInjection.js

@@ -18,7 +18,6 @@ var DefaultEventPluginOrder = require('DefaultEventPluginOrder');
 var EnterLeaveEventPlugin = require('EnterLeaveEventPlugin');
 var ExecutionEnvironment = require('ExecutionEnvironment');
 var HTMLDOMPropertyConfig = require('HTMLDOMPropertyConfig');
-var MobileSafariClickEventPlugin = require('MobileSafariClickEventPlugin');
 var ReactBrowserComponentMixin = require('ReactBrowserComponentMixin');
 var ReactClass = require('ReactClass');
 var ReactComponentBrowserEnvironment =
@@ -84,7 +83,6 @@ function inject() {
     SimpleEventPlugin: SimpleEventPlugin,
     EnterLeaveEventPlugin: EnterLeaveEventPlugin,
     ChangeEventPlugin: ChangeEventPlugin,
-    MobileSafariClickEventPlugin: MobileSafariClickEventPlugin,
     SelectEventPlugin: SelectEventPlugin,
     BeforeInputEventPlugin: BeforeInputEventPlugin
   });

src/browser/ui/__tests__/ReactDOMComponent-test.js

@@ -427,6 +427,59 @@ describe('ReactDOMComponent', function() {
         'style={{marginRight: spacing + \'em\'}} when using JSX.'
       );
     });
+
+    it("should execute custom event plugin listening behavior", function() {
+      var React = require('React');
+      var SimpleEventPlugin = require('SimpleEventPlugin');
+
+      SimpleEventPlugin.didPutListener = mocks.getMockFunction();
+      SimpleEventPlugin.willDeleteListener = mocks.getMockFunction();
+
+      var container = document.createElement('div');
+      React.render(
+        <div onClick={() => true} />,
+        container
+      );
+
+      expect(SimpleEventPlugin.didPutListener.mock.calls.length).toBe(1);
+
+      React.unmountComponentAtNode(container);
+
+      expect(SimpleEventPlugin.willDeleteListener.mock.calls.length).toBe(1);
+    });
+
+    it("should handle null and missing properly with event hooks", function() {
+      var React = require('React');
+      var SimpleEventPlugin = require('SimpleEventPlugin');
+
+      SimpleEventPlugin.didPutListener = mocks.getMockFunction();
+      SimpleEventPlugin.willDeleteListener = mocks.getMockFunction();
+      var container = document.createElement('div');
+
+      React.render(<div onClick={null} />, container);
+      expect(SimpleEventPlugin.didPutListener.mock.calls.length).toBe(0);
+      expect(SimpleEventPlugin.willDeleteListener.mock.calls.length).toBe(0);
+
+      React.render(<div onClick={() => 'apple'} />, container);
+      expect(SimpleEventPlugin.didPutListener.mock.calls.length).toBe(1);
+      expect(SimpleEventPlugin.willDeleteListener.mock.calls.length).toBe(0);
+
+      React.render(<div onClick={() => 'banana'} />, container);
+      expect(SimpleEventPlugin.didPutListener.mock.calls.length).toBe(2);
+      expect(SimpleEventPlugin.willDeleteListener.mock.calls.length).toBe(0);
+
+      React.render(<div onClick={null} />, container);
+      expect(SimpleEventPlugin.didPutListener.mock.calls.length).toBe(2);
+      expect(SimpleEventPlugin.willDeleteListener.mock.calls.length).toBe(1);
+
+      React.render(<div />, container);
+      expect(SimpleEventPlugin.didPutListener.mock.calls.length).toBe(2);
+      expect(SimpleEventPlugin.willDeleteListener.mock.calls.length).toBe(1);
+
+      React.unmountComponentAtNode(container);
+      expect(SimpleEventPlugin.didPutListener.mock.calls.length).toBe(2);
+      expect(SimpleEventPlugin.willDeleteListener.mock.calls.length).toBe(1);
+    });
   });
 
   describe('updateComponent', function() {

src/event/EventPluginHub.js

@@ -148,7 +148,7 @@ var EventPluginHub = {
    */
   putListener: function(id, registrationName, listener) {
     invariant(
-      !listener || typeof listener === 'function',
+      typeof listener === 'function',
       'Expected %s listener to be a function, instead got type %s',
       registrationName, typeof listener
     );
@@ -156,6 +156,12 @@ var EventPluginHub = {
     var bankForRegistrationName =
       listenerBank[registrationName] || (listenerBank[registrationName] = {});
     bankForRegistrationName[id] = listener;
+
+    var PluginModule =
+      EventPluginRegistry.registrationNameModules[registrationName];
+    if (PluginModule && PluginModule.didPutListener) {
+      PluginModule.didPutListener(id, registrationName, listener);
+    }
   },
 
   /**
@@ -175,7 +181,14 @@ var EventPluginHub = {
    * @param {string} registrationName Name of listener (e.g. `onClick`).
    */
   deleteListener: function(id, registrationName) {
+    var PluginModule =
+      EventPluginRegistry.registrationNameModules[registrationName];
+    if (PluginModule && PluginModule.willDeleteListener) {
+      PluginModule.willDeleteListener(id, registrationName);
+    }
+
     var bankForRegistrationName = listenerBank[registrationName];
+    // TODO: This should never be null -- when is it?
     if (bankForRegistrationName) {
       delete bankForRegistrationName[id];
     }
@@ -188,6 +201,16 @@ var EventPluginHub = {
    */
   deleteAllListeners: function(id) {
     for (var registrationName in listenerBank) {
+      if (!listenerBank[registrationName][id]) {
+        continue;
+      }
+
+      var PluginModule =
+        EventPluginRegistry.registrationNameModules[registrationName];
+      if (PluginModule && PluginModule.willDeleteListener) {
+        PluginModule.willDeleteListener(id, registrationName);
+      }
+
       delete listenerBank[registrationName][id];
     }
   },