From b20e3afbf5487d7208eb53bfca6eb48af8fd807c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 3 Nov 2016 15:26:23 -0700 Subject: [PATCH] Move Safari onClick hack into ReactDOMComponent (#8189) This is already where we trap non-bubbling events. We also already branch on the tag type so we know if it is interactive or not. This will let us get rid of the didPutListener and willDeleteListener abstractions. I use a simple onclick = emptyFunction to avoid the need for a bookkeeping map. --- .../shared/eventPlugins/SimpleEventPlugin.js | 45 ------------------- .../dom/stack/client/ReactDOMComponent.js | 31 +++++++++++++ 2 files changed, 31 insertions(+), 45 deletions(-) diff --git a/src/renderers/dom/shared/eventPlugins/SimpleEventPlugin.js b/src/renderers/dom/shared/eventPlugins/SimpleEventPlugin.js index 5b30244cc6..76a8e529c2 100644 --- a/src/renderers/dom/shared/eventPlugins/SimpleEventPlugin.js +++ b/src/renderers/dom/shared/eventPlugins/SimpleEventPlugin.js @@ -12,9 +12,7 @@ 'use strict'; -var EventListener = require('EventListener'); var EventPropagators = require('EventPropagators'); -var ReactDOMComponentTree = require('ReactDOMComponentTree'); var SyntheticAnimationEvent = require('SyntheticAnimationEvent'); var SyntheticClipboardEvent = require('SyntheticClipboardEvent'); var SyntheticEvent = require('SyntheticEvent'); @@ -27,7 +25,6 @@ var SyntheticTransitionEvent = require('SyntheticTransitionEvent'); var SyntheticUIEvent = require('SyntheticUIEvent'); var SyntheticWheelEvent = require('SyntheticWheelEvent'); -var emptyFunction = require('emptyFunction'); var getEventCharCode = require('getEventCharCode'); var invariant = require('invariant'); @@ -141,14 +138,6 @@ var topLevelEventsToDispatchConfig: {[key: TopLevelTypes]: DispatchConfig} = {}; topLevelEventsToDispatchConfig[topEvent] = type; }); -var onClickListeners = {}; - -function getDictionaryKey(inst: ReactInstance): string { - // Prevents V8 performance issue: - // https://github.com/facebook/react/pull/7232 - return '.' + inst._rootNodeID; -} - function isInteractive(tag) { return ( tag === 'button' || tag === 'input' || @@ -304,40 +293,6 @@ var SimpleEventPlugin: PluginModule = { return event; }, - didPutListener: function( - inst: ReactInstance, - registrationName: string, - listener: () => void, - ): void { - // 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. - // http://www.quirksmode.org/blog/archives/2010/09/click_event_del.html - if (registrationName === 'onClick' && !isInteractive(inst._tag)) { - var key = getDictionaryKey(inst); - var node = ReactDOMComponentTree.getNodeFromInstance(inst); - if (!onClickListeners[key]) { - onClickListeners[key] = EventListener.listen( - node, - 'click', - emptyFunction - ); - } - } - }, - - willDeleteListener: function( - inst: ReactInstance, - registrationName: string, - ): void { - if (registrationName === 'onClick' && !isInteractive(inst._tag)) { - var key = getDictionaryKey(inst); - onClickListeners[key].remove(); - delete onClickListeners[key]; - } - }, - }; module.exports = SimpleEventPlugin; diff --git a/src/renderers/dom/stack/client/ReactDOMComponent.js b/src/renderers/dom/stack/client/ReactDOMComponent.js index 0da6954928..90e81b72c2 100644 --- a/src/renderers/dom/stack/client/ReactDOMComponent.js +++ b/src/renderers/dom/stack/client/ReactDOMComponent.js @@ -313,6 +313,20 @@ function trackInputValue() { inputValueTracking.track(this); } +function trapClickOnNonInteractiveElement() { + // 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. + // http://www.quirksmode.org/blog/archives/2010/09/click_event_del.html + // Just set it using the onclick property so that we don't have to manage any + // bookkeeping for it. Not sure if we need to clear it when the listener is + // removed. + // TODO: Only do this for the relevant Safaris maybe? + var node = getNode(this); + node.onclick = emptyFunction; +} + function trapBubbledEventsLocal() { var inst = this; // If a component renders to null or if another component fatals and causes @@ -711,6 +725,14 @@ ReactDOMComponent.Mixin = { this ); break; + default: + if (typeof props.onClick === 'function') { + transaction.getReactMountReady().enqueue( + trapClickOnNonInteractiveElement, + this + ); + } + break; } return mountImage; @@ -911,6 +933,15 @@ ReactDOMComponent.Mixin = { lastProps = ReactDOMTextarea.getHostProps(this, lastProps); nextProps = ReactDOMTextarea.getHostProps(this, nextProps); break; + default: + if (typeof lastProps.onClick !== 'function' && + typeof nextProps.onClick === 'function') { + transaction.getReactMountReady().enqueue( + trapClickOnNonInteractiveElement, + this + ); + } + break; } assertValidProps(this, nextProps);