Skip SelectEventPlugin extraction if no listeners
If there are no listeners for `onSelect` yet, do not extract events. This way we can avoid issues where listeners have been set up for some event dependencies for `SelectEventPlugin`, but not all. For instance, if `topMouseDown` has been registered but not `topMouseUp`, event extraction will set the `mouseDown` flag to true but never unset it. This leads to bugs when `onSelect` is registered and should be firing during normal key behavior. Since no `topMouseUp` has yet occurred to unset the flag, `onSelect` fails to fire.
Isaac Salier-Hellendag committed
Apr 9, 2015 at 14:02 UTC
47b147f392450985bc7bd38d98a4c9e1e7188150
2 files changed
+120
src/browser/eventPlugins/SelectEventPlugin.js
+19
@@ -46,6 +46,11 @@ var activeElementID = null;
46
var lastSelection = null;
47
var mouseDown = false;
48
49
+// Track whether a listener exists for this plugin. If none exist, we do
50
+// not extract events.
51
+var hasListener = false;
52
+var ON_SELECT_KEY = keyOf({onSelect: null});
53
+
54
/**
55
* Get an object which is a unique representation of the current selection.
56
*
@@ -116,6 +121,8 @@ function constructSelectEvent(nativeEvent) {
121
122
return syntheticEvent;
123
}
124
+
125
+ return null;
126
}
127
128
/**
@@ -150,6 +157,10 @@ var SelectEventPlugin = {
157
topLevelTargetID,
158
nativeEvent) {
159
160
+ if (!hasListener) {
161
+ return null;
162
+ }
163
+
164
switch (topLevelType) {
165
// Track the input node that has focus.
166
case topLevelTypes.topFocus:
@@ -187,6 +198,14 @@ var SelectEventPlugin = {
198
case topLevelTypes.topKeyUp:
199
return constructSelectEvent(nativeEvent);
200
}
201
+
202
+ return null;
203
+ },
204
+
205
+ didPutListener: function(id, registrationName, listener) {
206
+ if (registrationName === ON_SELECT_KEY) {
207
+ hasListener = true;
208
+ }
209
}
210
};
211
src/browser/eventPlugins/__tests__/SelectEventPlugin-test.js
new
+101
@@ -0,0 +1,101 @@
1
+/**
2
+ * Copyright 2013-2015, Facebook, Inc.
3
+ * All rights reserved.
4
+ *
5
+ * This source code is licensed under the BSD-style license found in the
6
+ * LICENSE file in the root directory of this source tree. An additional grant
7
+ * of patent rights can be found in the PATENTS file in the same directory.
8
+ *
9
+ * @emails react-core
10
+ */
11
+
12
+'use strict';
13
+
14
+var mockModules = require('mock-modules');
15
+mockModules.mock('getActiveElement');
16
+
17
+var EventConstants;
18
+var React;
19
+var ReactMount;
20
+var ReactTestUtils;
21
+var SelectEventPlugin;
22
+
23
+var getActiveElement;
24
+
25
+var topLevelTypes;
26
+
27
+describe('SelectEventPlugin', function() {
28
+ function extract(node, topLevelEvent) {
29
+ return SelectEventPlugin.extractEvents(
30
+ topLevelEvent,
31
+ node,
32
+ ReactMount.getID(node),
33
+ {target: node}
34
+ );
35
+ }
36
+
37
+ beforeEach(function() {
38
+ mockModules.dumpCache();
39
+
40
+ EventConstants = require('EventConstants');
41
+ React = require('React');
42
+ ReactMount = require('ReactMount');
43
+ ReactTestUtils = require('ReactTestUtils');
44
+ SelectEventPlugin = require('SelectEventPlugin');
45
+
46
+ getActiveElement = require('getActiveElement');
47
+
48
+ topLevelTypes = EventConstants.topLevelTypes;
49
+ });
50
+
51
+ it('should skip extraction if no listeners are present', function() {
52
+ var WithoutSelect = React.createClass({
53
+ render: function() {
54
+ return <input type="text" />;
55
+ }
56
+ });
57
+
58
+ var rendered = ReactTestUtils.renderIntoDocument(<WithoutSelect />);
59
+ var node = React.findDOMNode(rendered);
60
+ getActiveElement.mockReturnValue(node);
61
+
62
+ var mousedown = extract(node, topLevelTypes.topMouseDown);
63
+ expect(mousedown).toBe(null);
64
+
65
+ var mouseup = extract(node, topLevelTypes.topMouseUp);
66
+ expect(mouseup).toBe(null);
67
+ });
68
+
69
+ it('should extract if an `onSelect` listener is present', function() {
70
+ var mocks = require('mocks');
71
+
72
+ var WithSelect = React.createClass({
73
+ render: function() {
74
+ return <input type="text" onSelect={this.props.onSelect} />;
75
+ }
76
+ });
77
+
78
+ var cb = mocks.getMockFunction();
79
+
80
+ var rendered = ReactTestUtils.renderIntoDocument(
81
+ <WithSelect onSelect={cb} />
82
+ );
83
+ var node = React.findDOMNode(rendered);
84
+
85
+ node.selectionStart = 0;
86
+ node.selectionEnd = 0;
87
+ getActiveElement.mockReturnValue(node);
88
+
89
+ var focus = extract(node, topLevelTypes.topFocus);
90
+ expect(focus).toBe(null);
91
+
92
+ var mousedown = extract(node, topLevelTypes.topMouseDown);
93
+ expect(mousedown).toBe(null);
94
+
95
+ var mouseup = extract(node, topLevelTypes.topMouseUp);
96
+ expect(mouseup).not.toBe(null);
97
+ expect(typeof mouseup).toBe('object');
98
+ expect(mouseup.type).toBe('select');
99
+ expect(mouseup.target).toBe(node);
100
+ });
101
+});