From dbc613199bd9f7da81692d5c0941339d3ffe0a8e Mon Sep 17 00:00:00 2001 From: Josh Duck Date: Tue, 8 Oct 2013 10:26:51 -0700 Subject: [PATCH] Fix SelectEventPlugin There were 2 issues: I was reusing event outside the original event handler (activeNativeEvent). This is a bad idea. I've changed deferred dispatch to have an empty object as the nativeEvent. I didn't handle inputs without .selectionStart (e.g. file inputs). I extracted a input type check from ChangeEventPlugin and reuse it here. --- src/eventPlugins/ChangeEventPlugin.js | 34 ++------------ src/eventPlugins/SelectEventPlugin.js | 67 +++++++++++++++------------ src/utils/isTextInputElement.js | 49 ++++++++++++++++++++ 3 files changed, 89 insertions(+), 61 deletions(-) create mode 100644 src/utils/isTextInputElement.js diff --git a/src/eventPlugins/ChangeEventPlugin.js b/src/eventPlugins/ChangeEventPlugin.js index 99d5a3f018..ad8fa70f12 100644 --- a/src/eventPlugins/ChangeEventPlugin.js +++ b/src/eventPlugins/ChangeEventPlugin.js @@ -25,6 +25,7 @@ var ExecutionEnvironment = require('ExecutionEnvironment'); var SyntheticEvent = require('SyntheticEvent'); var isEventSupported = require('isEventSupported'); +var isTextInputElement = require('isTextInputElement'); var keyOf = require('keyOf'); var topLevelTypes = EventConstants.topLevelTypes; @@ -130,35 +131,6 @@ if (ExecutionEnvironment.canUseDOM) { ); } - -/** - * @see http://www.whatwg.org/specs/web-apps/current-work/multipage/the-input-element.html#input-type-attr-summary - */ -var supportedInputTypes = { - 'color': true, - 'date': true, - 'datetime': true, - 'datetime-local': true, - 'email': true, - 'month': true, - 'number': true, - 'password': true, - 'range': true, - 'search': true, - 'tel': true, - 'text': true, - 'time': true, - 'url': true, - 'week': true -}; - -function shouldUseInputEvent(elem) { - return ( - (elem.nodeName === 'INPUT' && supportedInputTypes[elem.type]) || - elem.nodeName === 'TEXTAREA' - ); -} - /** * (For old IE.) Replacement getter/setter for the `value` property that gets * set on the active element. @@ -322,7 +294,7 @@ function getTargetIDForClickEvent( * change the element's value without seeing a flicker. * * Supported elements are: - * - input (see `supportedInputTypes`) + * - input (see `isTextInputElement`) * - textarea * - select */ @@ -351,7 +323,7 @@ var ChangeEventPlugin = { } else { handleEventFunc = handleEventsForChangeEventIE8; } - } else if (shouldUseInputEvent(topLevelTarget)) { + } else if (isTextInputElement(topLevelTarget)) { if (isInputEventSupported) { getTargetIDFunc = getTargetIDForInputEvent; } else { diff --git a/src/eventPlugins/SelectEventPlugin.js b/src/eventPlugins/SelectEventPlugin.js index 2914292208..b96542e723 100644 --- a/src/eventPlugins/SelectEventPlugin.js +++ b/src/eventPlugins/SelectEventPlugin.js @@ -26,6 +26,7 @@ var SyntheticEvent = require('SyntheticEvent'); var getActiveElement = require('getActiveElement'); var isEventSupported = require('isEventSupported'); +var isTextInputElement = require('isTextInputElement'); var keyOf = require('keyOf'); var shallowEqual = require('shallowEqual'); @@ -106,30 +107,35 @@ function constructSelectEvent(nativeEvent) { if (!lastSelection || !shallowEqual(lastSelection, currentSelection)) { lastSelection = currentSelection; - return SyntheticEvent.getPooled( + var syntheticEvent = SyntheticEvent.getPooled( eventTypes.select, activeElementID, nativeEvent ); + + syntheticEvent.type = 'select'; + syntheticEvent.target = activeElement; + + EventPropagators.accumulateTwoPhaseDispatches(syntheticEvent); + + return syntheticEvent; } } /** * Handle deferred event. And manually dispatch synthetic events. */ -function handleDeferredEvent() { +function dispatchDeferredSelectEvent() { if (!activeNativeEvent) { return; } - var event = constructSelectEvent(activeNativeEvent); + var syntheticEvent = constructSelectEvent(activeNativeEvent); activeNativeEvent = null; - if (event) { - EventPropagators.accumulateTwoPhaseDispatches(event); - - // Enqueue and process the abstract event manually. - EventPluginHub.enqueueEvents(event); + // Enqueue and process the abstract event manually. + if (syntheticEvent) { + EventPluginHub.enqueueEvents(syntheticEvent); EventPluginHub.processEventQueue(); } } @@ -139,8 +145,8 @@ function handleDeferredEvent() { * across form elements. * * Supported elements are: - * - input (see `supportedInputTypes`) - * - TEXTAREA + * - input (see `isTextInputElement`) + * - textarea * - contentEditable * * This differs from native browser implementations in the following ways: @@ -167,12 +173,15 @@ var SelectEventPlugin = { nativeEvent) { switch (topLevelType) { + // Track the input node that has focus. case topLevelTypes.topFocus: - // Track the input node that has focus. - activeElement = topLevelTarget; - activeElementID = topLevelTargetID; - lastSelection = null; - mouseDown = false; + if (isTextInputElement(topLevelTarget) || + topLevelTarget.contentEditable === 'true') { + activeElement = topLevelTarget; + activeElementID = topLevelTargetID; + lastSelection = null; + mouseDown = false; + } break; case topLevelTypes.topBlur: activeElement = null; @@ -180,29 +189,27 @@ var SelectEventPlugin = { lastSelection = null; mouseDown = false; break; - case topLevelTypes.topSelectionChange: - // Chrome and IE fire non-standard event whenever selection is - // changed (and sometimes when it hasn't). - var event = constructSelectEvent(nativeEvent); - if (event) { - EventPropagators.accumulateTwoPhaseDispatches(event); - return event; - } - break; + + // Don't fire the event while the user is dragging. This matches the + // semantics of the native select event. case topLevelTypes.topMouseDown: - // Don't fire the event while the user is dragging. mouseDown = true; break; case topLevelTypes.topMouseUp: mouseDown = false; - activeNativeEvent = nativeEvent; - handleDeferredEvent.defer(); - break; + return constructSelectEvent(nativeEvent); + + // Chrome and IE fire non-standard event when selection is changed (and + // sometimes when it hasn't). + case topLevelTypes.topSelectionChange: + return constructSelectEvent(nativeEvent); + + // Firefox doesn't support selectionchange, so check selection status + // after each key entry. case topLevelTypes.topKeyDown: - // For Firefox we check seleciton after each key. if (!useSelectionChange) { activeNativeEvent = nativeEvent; - handleDeferredEvent.defer(); + setTimeout(dispatchDeferredSelectEvent, 0); } break; } diff --git a/src/utils/isTextInputElement.js b/src/utils/isTextInputElement.js new file mode 100644 index 0000000000..d7e5d5a27f --- /dev/null +++ b/src/utils/isTextInputElement.js @@ -0,0 +1,49 @@ +/** + * Copyright 2013 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * @providesModule isTextInputElement + */ + +"use strict"; + +/** + * @see http://www.whatwg.org/specs/web-apps/current-work/multipage/the-input-element.html#input-type-attr-summary + */ +var supportedInputTypes = { + 'color': true, + 'date': true, + 'datetime': true, + 'datetime-local': true, + 'email': true, + 'month': true, + 'number': true, + 'password': true, + 'range': true, + 'search': true, + 'tel': true, + 'text': true, + 'time': true, + 'url': true, + 'week': true +}; + +function isTextInputElement(elem) { + return elem && ( + (elem.nodeName === 'INPUT' && supportedInputTypes[elem.type]) || + elem.nodeName === 'TEXTAREA' + ); +} + +module.exports = isTextInputElement;