Skip to content

Commit

Permalink
Skip SelectEventPlugin extraction if no listeners
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Isaac Salier-Hellendag committed Apr 9, 2015
1 parent 857736d commit 47b147f
Show file tree
Hide file tree
Showing 2 changed files with 120 additions and 0 deletions.
19 changes: 19 additions & 0 deletions src/browser/eventPlugins/SelectEventPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ var activeElementID = null;
var lastSelection = null;
var mouseDown = false;

// Track whether a listener exists for this plugin. If none exist, we do
// not extract events.
var hasListener = false;
var ON_SELECT_KEY = keyOf({onSelect: null});

/**
* Get an object which is a unique representation of the current selection.
*
Expand Down Expand Up @@ -116,6 +121,8 @@ function constructSelectEvent(nativeEvent) {

return syntheticEvent;
}

return null;
}

/**
Expand Down Expand Up @@ -150,6 +157,10 @@ var SelectEventPlugin = {
topLevelTargetID,
nativeEvent) {

if (!hasListener) {
return null;
}

switch (topLevelType) {
// Track the input node that has focus.
case topLevelTypes.topFocus:
Expand Down Expand Up @@ -187,6 +198,14 @@ var SelectEventPlugin = {
case topLevelTypes.topKeyUp:
return constructSelectEvent(nativeEvent);
}

return null;
},

didPutListener: function(id, registrationName, listener) {
if (registrationName === ON_SELECT_KEY) {
hasListener = true;
}
}
};

Expand Down
101 changes: 101 additions & 0 deletions src/browser/eventPlugins/__tests__/SelectEventPlugin-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/**
* 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.
*
* @emails react-core
*/

'use strict';

var mockModules = require('mock-modules');
mockModules.mock('getActiveElement');

var EventConstants;
var React;
var ReactMount;
var ReactTestUtils;
var SelectEventPlugin;

var getActiveElement;

var topLevelTypes;

describe('SelectEventPlugin', function() {
function extract(node, topLevelEvent) {
return SelectEventPlugin.extractEvents(
topLevelEvent,
node,
ReactMount.getID(node),
{target: node}
);
}

beforeEach(function() {
mockModules.dumpCache();

EventConstants = require('EventConstants');
React = require('React');
ReactMount = require('ReactMount');
ReactTestUtils = require('ReactTestUtils');
SelectEventPlugin = require('SelectEventPlugin');

getActiveElement = require('getActiveElement');

topLevelTypes = EventConstants.topLevelTypes;
});

it('should skip extraction if no listeners are present', function() {
var WithoutSelect = React.createClass({
render: function() {
return <input type="text" />;
}
});

var rendered = ReactTestUtils.renderIntoDocument(<WithoutSelect />);
var node = React.findDOMNode(rendered);
getActiveElement.mockReturnValue(node);

var mousedown = extract(node, topLevelTypes.topMouseDown);
expect(mousedown).toBe(null);

var mouseup = extract(node, topLevelTypes.topMouseUp);
expect(mouseup).toBe(null);
});

it('should extract if an `onSelect` listener is present', function() {
var mocks = require('mocks');

var WithSelect = React.createClass({
render: function() {
return <input type="text" onSelect={this.props.onSelect} />;
}
});

var cb = mocks.getMockFunction();

var rendered = ReactTestUtils.renderIntoDocument(
<WithSelect onSelect={cb} />
);
var node = React.findDOMNode(rendered);

node.selectionStart = 0;
node.selectionEnd = 0;
getActiveElement.mockReturnValue(node);

var focus = extract(node, topLevelTypes.topFocus);
expect(focus).toBe(null);

var mousedown = extract(node, topLevelTypes.topMouseDown);
expect(mousedown).toBe(null);

var mouseup = extract(node, topLevelTypes.topMouseUp);
expect(mouseup).not.toBe(null);
expect(typeof mouseup).toBe('object');
expect(mouseup.type).toBe('select');
expect(mouseup.target).toBe(node);
});
});

0 comments on commit 47b147f

Please sign in to comment.